From 72e405b9079707c9c033864aa66d20221b54bb06 Mon Sep 17 00:00:00 2001 From: Fabian Kosmale Date: Thu, 18 Apr 2024 10:25:21 +0200 Subject: [PATCH] QStringConverterICU: Pass correct pointer to callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pass the pointer to the current state, not a pointer to a pointer to it. [ChangeLog][QtCore][QStringConverter] Fixed a bug involving moved QStringEncoder/QStringDecoder objects accessing invalid state. Amends 122270d6bea164e6df4357f4d4d77aacfa430470. Done-with: Marc Mutz Change-Id: I70d4dc00e3e0db6cad964579662bcf6d185a4c34 Reviewed-by: Fabian Kosmale Reviewed-by: Mårten Nordheim (cherry picked from commit 39bbfce9b675c9085ef49c9b9c52c146eca55e4a) Reviewed-by: Qt Cherry-pick Bot (cherry picked from commit 7c4e1357e49baebdd2d20710fccb5604cbb36c0d) --- src/corelib/text/qstringconverter.cpp | 4 +- .../qstringconverter/tst_qstringconverter.cpp | 76 +++++++++++-------- 2 files changed, 46 insertions(+), 34 deletions(-) diff --git a/src/corelib/text/qstringconverter.cpp b/src/corelib/text/qstringconverter.cpp index 6ca65ba6ad8..043b8f54151 100644 --- a/src/corelib/text/qstringconverter.cpp +++ b/src/corelib/text/qstringconverter.cpp @@ -1966,7 +1966,7 @@ struct QStringConverterICU : QStringConverter const void *context; ucnv_getToUCallBack(icu_conv, &action, &context); if (context != state) - ucnv_setToUCallBack(icu_conv, action, &state, nullptr, nullptr, &err); + ucnv_setToUCallBack(icu_conv, action, state, nullptr, nullptr, &err); ucnv_toUnicode(icu_conv, &target, targetLimit, &source, sourceLimit, nullptr, flush, &err); // We did reserve enough space: @@ -1999,7 +1999,7 @@ struct QStringConverterICU : QStringConverter const void *context; ucnv_getFromUCallBack(icu_conv, &action, &context); if (context != state) - ucnv_setFromUCallBack(icu_conv, action, &state, nullptr, nullptr, &err); + ucnv_setFromUCallBack(icu_conv, action, state, nullptr, nullptr, &err); ucnv_fromUnicode(icu_conv, &target, targetLimit, &source, sourceLimit, nullptr, flush, &err); // We did reserve enough space: diff --git a/tests/auto/corelib/text/qstringconverter/tst_qstringconverter.cpp b/tests/auto/corelib/text/qstringconverter/tst_qstringconverter.cpp index f37935f6655..ebe6280385a 100644 --- a/tests/auto/corelib/text/qstringconverter/tst_qstringconverter.cpp +++ b/tests/auto/corelib/text/qstringconverter/tst_qstringconverter.cpp @@ -540,11 +540,10 @@ void tst_QStringConverter::charByCharConsistency_data() void tst_QStringConverter::charByCharConsistency() { - QFETCH(QStringView, source); - QFETCH(QByteArray, codec); + QFETCH(const QStringView, source); + QFETCH(const QByteArray, codec); - { - QStringEncoder encoder(codec); + const auto check = [&](QStringEncoder encoder){ if (!encoder.isValid()) QSKIP("Unsupported codec"); @@ -555,19 +554,28 @@ void tst_QStringConverter::charByCharConsistency() stepByStepConverted += encoder.encode(codeUnit); } QCOMPARE(stepByStepConverted, fullyConverted); - } + }; + + check(QStringEncoder(codec)); + if (QTest::currentTestResolved()) return; + + check(QStringEncoder(codec, QStringConverter::Flag::ConvertInvalidToNull)); + if (QTest::currentTestResolved()) return; + + // moved codecs also work: { - QStringEncoder encoder(codec, QStringConverter::Flag::ConvertInvalidToNull); - - QByteArray fullyConverted = encoder.encode(source); - encoder.resetState(); - QByteArray stepByStepConverted; - for (const auto& codeUnit: source) { - stepByStepConverted += encoder.encode(codeUnit); - } - QCOMPARE(stepByStepConverted, fullyConverted); + QStringEncoder dec(codec); + check(std::move(dec)); } + if (QTest::currentTestResolved()) return; + + { + QStringEncoder dec(codec, QStringConverter::Flag::ConvertInvalidToNull); + check(std::move(dec)); + } + if (QTest::currentTestResolved()) return; + } void tst_QStringConverter::byteByByteConsistency_data() @@ -584,11 +592,10 @@ void tst_QStringConverter::byteByByteConsistency_data() void tst_QStringConverter::byteByByteConsistency() { - QFETCH(QByteArray, source); - QFETCH(QByteArray, codec); + QFETCH(const QByteArray, source); + QFETCH(const QByteArray, codec); - { - QStringDecoder decoder(codec); + const auto check = [&](QStringDecoder decoder) { if (!decoder.isValid()) QSKIP("Unsupported codec"); @@ -601,23 +608,28 @@ void tst_QStringConverter::byteByByteConsistency() stepByStepConverted += decoder.decode(singleChar); } QCOMPARE(stepByStepConverted, fullyConverted); - } + }; + + check(QStringDecoder(codec)); + if (QTest::currentTestResolved()) return; + + check(QStringDecoder(codec, QStringConverter::Flag::ConvertInvalidToNull)); + if (QTest::currentTestResolved()) return; + + // moved codecs also work: { - QStringDecoder decoder(codec, QStringConverter::Flag::ConvertInvalidToNull); - if (!decoder.isValid()) - QSKIP("Unsupported codec"); - - QString fullyConverted = decoder.decode(source); - decoder.resetState(); - QString stepByStepConverted; - for (const auto& byte: source) { - QByteArray singleChar; - singleChar.append(byte); - stepByStepConverted += decoder.decode(singleChar); - } - QCOMPARE(stepByStepConverted, fullyConverted); + QStringDecoder dec(codec); + check(std::move(dec)); } + if (QTest::currentTestResolved()) return; + + { + QStringDecoder dec(codec, QStringConverter::Flag::ConvertInvalidToNull); + check(std::move(dec)); + } + if (QTest::currentTestResolved()) return; + } void tst_QStringConverter::statefulPieceWise()