From e150bcfe4d5514b2a2960234049c514bc558adee Mon Sep 17 00:00:00 2001 From: Ivan Solovev Date: Thu, 22 Jul 2021 16:52:40 +0200 Subject: QByteArray: fix indexOf/lastIndexOf This patch fixed two bugs in indexOf/lastIndexOf: 1. The lastIndexOf(char, qsizetype) overload was crashing with an empty QByteArray. It was unconditionally calling lastIndexOfCharHelper() which assumes that this QBA is not empty. An explicit check for the empty case is added. 2. The indexOf(QByteArray, qsizetype) overload was behaving incorrectly while searching for an empty QByteArray. In this case it unconditionally returned its second parameter (from). However, from can be negative, or even exceed the size of this QByteArray. This patch handles this cases properly. As a drive-by: this patch adjusts the QByteArray::indexOf(char, qsizetype) and QByteArray::lastIndexOf(char, qsizetype) overloads to match with the QByteArrayView implementation. This is done to have similar code paths in both cases and avoid tricky bugs in future. Ideally we had to adjust the QByteArrayView implementation, but it's fully inline, so can't be changed without breaking BC. Task-number: QTBUG-91736 Pick-to: 6.2 6.1 Change-Id: Iaef2fdc5b99cce6aa342cca2d17544a1ad7ca677 Reviewed-by: Edward Welbourne Reviewed-by: Lars Knoll --- src/corelib/text/qbytearray.cpp | 15 ++++++++---- .../tst_qbytearrayapisymmetry.cpp | 27 ++++++++++++++++++---- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/src/corelib/text/qbytearray.cpp b/src/corelib/text/qbytearray.cpp index e44dead1d6..d8deb5e8cc 100644 --- a/src/corelib/text/qbytearray.cpp +++ b/src/corelib/text/qbytearray.cpp @@ -2421,12 +2421,17 @@ static inline qsizetype findCharHelper(QByteArrayView haystack, qsizetype from, qsizetype QtPrivate::findByteArray(QByteArrayView haystack, qsizetype from, QByteArrayView needle) noexcept { const auto ol = needle.size(); - if (ol == 0) - return from; + const auto l = haystack.size(); + if (ol == 0) { + if (from < 0) + return qMax(from + l, 0); + else + return from > l ? -1 : from; + } + if (ol == 1) return findCharHelper(haystack, from, needle.front()); - const auto l = haystack.size(); if (from > l || ol + from > l) return -1; @@ -2461,7 +2466,7 @@ qsizetype QtPrivate::findByteArray(QByteArrayView haystack, qsizetype from, QByt qsizetype QByteArray::indexOf(char ch, qsizetype from) const { - return static_cast(findCharHelper(*this, from, ch)); + return qToByteArrayViewIgnoringNull(*this).indexOf(ch, from); } static qsizetype lastIndexOfHelper(const char *haystack, qsizetype l, const char *needle, @@ -2556,7 +2561,7 @@ qsizetype QtPrivate::lastIndexOf(QByteArrayView haystack, qsizetype from, QByteA qsizetype QByteArray::lastIndexOf(char ch, qsizetype from) const { - return static_cast(lastIndexOfCharHelper(*this, from, ch)); + return qToByteArrayViewIgnoringNull(*this).lastIndexOf(ch, from); } static inline qsizetype countCharHelper(QByteArrayView haystack, char needle) noexcept diff --git a/tests/auto/corelib/text/qbytearrayapisymmetry/tst_qbytearrayapisymmetry.cpp b/tests/auto/corelib/text/qbytearrayapisymmetry/tst_qbytearrayapisymmetry.cpp index 8b51727417..40d7291e41 100644 --- a/tests/auto/corelib/text/qbytearrayapisymmetry/tst_qbytearrayapisymmetry.cpp +++ b/tests/auto/corelib/text/qbytearrayapisymmetry/tst_qbytearrayapisymmetry.cpp @@ -316,12 +316,20 @@ void tst_QByteArrayApiSymmetry::indexOf_data() static const char n19[] = { 0x00, 0x00, 0x01, 0x00 }; QTest::newRow("19") << QByteArray(h19, sizeof(h19)) << QByteArray(n19, sizeof(n19)) << 0 << -1; - QTest::newRow("empty") << QByteArray("") << QByteArray("x") << 0 << -1; - QTest::newRow("null") << QByteArray() << QByteArray("x") << 0 << -1; + QTest::newRow("empty from 0") << QByteArray("") << QByteArray("x") << 0 << -1; + QTest::newRow("empty from -1") << QByteArray("") << QByteArray("x") << -1 << -1; + QTest::newRow("empty from 1") << QByteArray("") << QByteArray("x") << 1 << -1; + QTest::newRow("null from 0") << QByteArray() << QByteArray("x") << 0 << -1; + QTest::newRow("null from -1") << QByteArray() << QByteArray("x") << -1 << -1; + QTest::newRow("null from 1") << QByteArray() << QByteArray("x") << 1 << -1; QTest::newRow("null-in-null") << QByteArray() << QByteArray() << 0 << 0; QTest::newRow("empty-in-null") << QByteArray() << QByteArray("") << 0 << 0; QTest::newRow("null-in-empty") << QByteArray("") << QByteArray() << 0 << 0; QTest::newRow("empty-in-empty") << QByteArray("") << QByteArray("") << 0 << 0; + QTest::newRow("empty in abc from 0") << abc << QByteArray() << 0 << 0; + QTest::newRow("empty in abc from 2") << abc << QByteArray() << 2 << 2; + QTest::newRow("empty in abc from 5") << abc << QByteArray() << 5 << -1; + QTest::newRow("empty in abc from -1") << abc << QByteArray() << -1 << 2; QByteArray veryBigHaystack(500, 'a'); veryBigHaystack += 'B'; @@ -404,12 +412,23 @@ void tst_QByteArrayApiSymmetry::lastIndexOf_data() static const char n25[] = { 0x00, 0x00, 0x01, 0x00 }; QTest::newRow("25") << QByteArray(h25, sizeof(h25)) << QByteArray(n25, sizeof(n25)) << 0 << -1; - QTest::newRow("empty") << QByteArray("") << QByteArray("x") << -1 << -1; - QTest::newRow("null") << QByteArray() << QByteArray("x") << -1 << -1; + QTest::newRow("empty from 0") << QByteArray("") << QByteArray("x") << 0 << -1; + QTest::newRow("empty from -1") << QByteArray("") << QByteArray("x") << -1 << -1; + QTest::newRow("empty from 1") << QByteArray("") << QByteArray("x") << 1 << -1; + QTest::newRow("null from 0") << QByteArray() << QByteArray("x") << 0 << -1; + QTest::newRow("null from -1") << QByteArray() << QByteArray("x") << -1 << -1; + QTest::newRow("null from 1") << QByteArray() << QByteArray("x") << 1 << -1; QTest::newRow("null-in-null") << QByteArray() << QByteArray() << -1 << 0; QTest::newRow("empty-in-null") << QByteArray() << QByteArray("") << -1 << 0; QTest::newRow("null-in-empty") << QByteArray("") << QByteArray() << -1 << 0; QTest::newRow("empty-in-empty") << QByteArray("") << QByteArray("") << -1 << 0; + QTest::newRow("empty in abc from 0") << abc << QByteArray() << 0 << 0; + QTest::newRow("empty in abc from 2") << abc << QByteArray() << 2 << 2; + QTest::newRow("empty in abc from 5") + << abc << QByteArray() << 5 << -1; // perversely enough, should be 3? + QTest::newRow("empty in abc from -1") << abc << QByteArray() << -1 << 3; + QTest::newRow("empty in abc from -5") + << abc << QByteArray() << -5 << 3; // perversely enough, should be -1? } template -- cgit v1.2.3