From 5caa31cb214647dbe6b9da2037c1fc0da45b5e4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A5rten=20Nordheim?= Date: Mon, 19 Aug 2024 14:16:38 +0200 Subject: [PATCH] QByteArray: fix lastIndexOf for char values with set sign bit lastIndexOf will convert the `needle` to unsigned char, which is then 'upcast' to int, before we search. But the string itself was searched using signed char, meaning any values with the signed bit set would mismatch. Add tests for indexOf and lastIndexOf. Amends 4c12ae7e67abc3d9b5847f6b61177ede3ee3203b Fixes: QTBUG-128199 Change-Id: I0ce7d7d9741f21650ef6f0f012a94e00d84a0f02 Reviewed-by: Ahmad Samir Reviewed-by: Thiago Macieira (cherry picked from commit 7b1f3bdc503ea7aceacc9fa8d388d843f1d7b131) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/text/qbytearray.cpp | 6 +- .../text/qbytearray/tst_qbytearray.cpp | 92 +++++++++++++++++++ 2 files changed, 95 insertions(+), 3 deletions(-) diff --git a/src/corelib/text/qbytearray.cpp b/src/corelib/text/qbytearray.cpp index d9f0ee405a6..bf9852dcd46 100644 --- a/src/corelib/text/qbytearray.cpp +++ b/src/corelib/text/qbytearray.cpp @@ -74,10 +74,10 @@ const void *qmemrchr(const void *s, int needle, size_t size) noexcept #if QT_CONFIG(memrchr) return memrchr(s, needle, size); #endif - auto b = static_cast(s); - const char *n = b + size; + auto b = static_cast(s); + const uchar *n = b + size; while (n-- != b) { - if (*n == needle) + if (*n == uchar(needle)) return n; } return nullptr; diff --git a/tests/auto/corelib/text/qbytearray/tst_qbytearray.cpp b/tests/auto/corelib/text/qbytearray/tst_qbytearray.cpp index 5b22fecbf39..c2ea49745a5 100644 --- a/tests/auto/corelib/text/qbytearray/tst_qbytearray.cpp +++ b/tests/auto/corelib/text/qbytearray/tst_qbytearray.cpp @@ -119,6 +119,11 @@ private slots: void isUpper(); void isLower(); + void indexOf_data(); + void indexOf(); + void lastIndexOf_data(); + void lastIndexOf(); + void macTypes(); void stdString(); @@ -2588,6 +2593,93 @@ void tst_QByteArray::isLower() QVERIFY(QByteArray("`abyz{").isLower()); } +using ByteArrayOrChar = std::variant; +void tst_QByteArray::indexOf_data() +{ + qRegisterMetaType(); + QTest::addColumn("haystack"); + QTest::addColumn("needle"); + QTest::addColumn("expectedIndex"); + QTest::addColumn("from"); + + const QByteArray haystack = "abc 123 cba \x80 \x08 \x00 \x01 \x81"_ba; + + QTest::newRow("not_found_1_char_string") << haystack << ByteArrayOrChar("d"_ba) << -1 << 0; + QTest::newRow("not_found_char") << haystack << ByteArrayOrChar('d') << -1 << 0; + + QTest::newRow("not_found_string") << haystack << ByteArrayOrChar("abcd"_ba) << -1 << 0; + QTest::newRow("found_1_char_string") << haystack << ByteArrayOrChar("a"_ba) << 0 << 0; + + QTest::newRow("found_char") << haystack << ByteArrayOrChar('a') << 0 << 0; + QTest::newRow("found_string") << haystack << ByteArrayOrChar("cba"_ba) << 8 << 0; + + QTest::newRow("found_empty_string") << haystack << ByteArrayOrChar(""_ba) << 0 << 0; + + QTest::newRow("found_embedded_null") << haystack << ByteArrayOrChar("\x00"_ba) << 16 << 0; + QTest::newRow("not_found_terminating_null") << haystack << ByteArrayOrChar("\x00"_ba) << -1 << 17; + QTest::newRow("found_char_star_0x80") << haystack << ByteArrayOrChar("\x80"_ba) << 12 << 0; + QTest::newRow("found_char_0x80") << haystack << ByteArrayOrChar('\x80') << 12 << 0; + QTest::newRow("found_char_0x81") << haystack << ByteArrayOrChar('\x81') << 20 << 0; +} + +void tst_QByteArray::indexOf() +{ + QFETCH(QByteArray, haystack); + QFETCH(ByteArrayOrChar, needle); + QFETCH(int, expectedIndex); + QFETCH(int, from); + + if (auto *qba = std::get_if(&needle)) { + QCOMPARE(haystack.indexOf(*qba, from), expectedIndex); + } else { + char c = std::get(needle); + QCOMPARE(haystack.indexOf(c, from), expectedIndex); + } +} + +void tst_QByteArray::lastIndexOf_data() +{ + qRegisterMetaType(); + QTest::addColumn("haystack"); + QTest::addColumn("needle"); + QTest::addColumn("expectedIndex"); + QTest::addColumn("from"); + + const QByteArray haystack = "abc 123 cba \x80 \x08 \x00 \x01 \x81"_ba; + + QTest::newRow("not_found_1_char_string") << haystack << ByteArrayOrChar("d"_ba) << -1 << -1; + QTest::newRow("not_found_char") << haystack << ByteArrayOrChar('d') << -1 << -1; + + QTest::newRow("not_found_string") << haystack << ByteArrayOrChar("abcd"_ba) << -1 << -1; + QTest::newRow("found_1_char_string") << haystack << ByteArrayOrChar("a"_ba) << 10 << -1; + + QTest::newRow("found_char") << haystack << ByteArrayOrChar('a') << 10 << -1; + QTest::newRow("found_string") << haystack << ByteArrayOrChar("cba"_ba) << 8 << -1; + + QTest::newRow("found_empty_string") << haystack << ByteArrayOrChar(""_ba) << haystack.size() << -1; + + QTest::newRow("found_embedded_null") << haystack << ByteArrayOrChar("\x00"_ba) << 16 << -1; + QTest::newRow("not_found_leading_null") << haystack << ByteArrayOrChar("\x00"_ba) << -1 << 15; + QTest::newRow("found_char_star_0x80") << haystack << ByteArrayOrChar("\x80"_ba) << 12 << -1; + QTest::newRow("found_char_0x80") << haystack << ByteArrayOrChar('\x80') << 12 << -1; + QTest::newRow("found_char_0x81") << haystack << ByteArrayOrChar('\x81') << 20 << -1; +} + +void tst_QByteArray::lastIndexOf() +{ + QFETCH(QByteArray, haystack); + QFETCH(ByteArrayOrChar, needle); + QFETCH(int, expectedIndex); + QFETCH(int, from); + + if (auto *qba = std::get_if(&needle)) { + QCOMPARE(haystack.lastIndexOf(*qba, from), expectedIndex); + } else { + char c = std::get(needle); + QCOMPARE(haystack.lastIndexOf(c, from), expectedIndex); + } +} + void tst_QByteArray::macTypes() { #ifndef Q_OS_DARWIN