From f00d322f6701580f97f38794b83b0ec13973d177 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Wed, 23 Sep 2020 21:30:30 -0700 Subject: [PATCH] QJsonObject: fix sorting after parsing from JSON text The logic was complex and missed the UTF-8 UTF-8 case. It ended up calling the UTF-8 to Latin1, resulting in an improperly-sorted container, which in turn meant keys were not found when searched. Fixes: QTBUG-86873 Pick-to: 5.15 Change-Id: I0d3ff441bec041728945fffd16379dec418637ca Reviewed-by: Ulf Hermann --- src/corelib/serialization/qjsonparser.cpp | 40 ++++----------- .../corelib/serialization/json/tst_qtjson.cpp | 51 ++++++++++++++++--- 2 files changed, 54 insertions(+), 37 deletions(-) diff --git a/src/corelib/serialization/qjsonparser.cpp b/src/corelib/serialization/qjsonparser.cpp index 116e7f6995..525cbfb3a0 100644 --- a/src/corelib/serialization/qjsonparser.cpp +++ b/src/corelib/serialization/qjsonparser.cpp @@ -379,8 +379,6 @@ error: return QCborValue(); } - - static void sortContainer(QCborContainerPrivate *container) { using Forward = QJsonPrivate::KeyIterator; @@ -403,39 +401,23 @@ static void sortContainer(QCborContainerPrivate *container) if (!bData) return 1; - // If StringIsAscii is set, we can use either the UTF-8 or the latin1 comparison - // for the string as ASCII is a subset of both. If nothing is set, that means UTF-8. - - // We are currently missing an efficient comparison between UTF-8 and UTF-16 strings. - // Therefore, we need to convert the UTF-8 string if we encounter such a case. - - if (aKey.flags & QtCbor::Element::StringIsAscii) { - if (bKey.flags & QtCbor::Element::StringIsAscii) - return QtPrivate::compareStrings(aData->asLatin1(), bData->asLatin1()); - if (bKey.flags & QtCbor::Element::StringIsUtf16) - return QtPrivate::compareStrings(aData->asLatin1(), bData->asStringView()); - - return QCborContainerPrivate::compareUtf8(aData, bData->asLatin1()); - } + // US-ASCII (StringIsAscii flag) is just a special case of UTF-8 + // string, so we can safely ignore the flag. if (aKey.flags & QtCbor::Element::StringIsUtf16) { - if (bKey.flags & QtCbor::Element::StringIsAscii) - return QtPrivate::compareStrings(aData->asStringView(), bData->asLatin1()); if (bKey.flags & QtCbor::Element::StringIsUtf16) return QtPrivate::compareStrings(aData->asStringView(), bData->asStringView()); - // Nasty case. a is UTF-16 and b is UTF-8 - return QtPrivate::compareStrings(aData->asStringView(), bData->toUtf8String()); + return -QCborContainerPrivate::compareUtf8(bData, aData->asStringView()); + } else { + if (bKey.flags & QtCbor::Element::StringIsUtf16) + return QCborContainerPrivate::compareUtf8(aData, bData->asStringView()); + + // We're missing an explicit UTF-8 to UTF-8 comparison in Qt, but + // UTF-8 to UTF-8 comparison retains simple byte ordering, so we'll + // abuse the Latin-1 comparison function. + return QtPrivate::compareStrings(aData->asLatin1(), bData->asLatin1()); } - - if (bKey.flags & QtCbor::Element::StringIsAscii) - return QCborContainerPrivate::compareUtf8(aData, bData->asLatin1()); - - // Nasty case. a is UTF-8 and b is UTF-16 - if (bKey.flags & QtCbor::Element::StringIsUtf16) - return QtPrivate::compareStrings(aData->toUtf8String(), bData->asStringView()); - - return QCborContainerPrivate::compareUtf8(aData, bData->asLatin1()); }; std::sort(Forward(container->elements.begin()), Forward(container->elements.end()), diff --git a/tests/auto/corelib/serialization/json/tst_qtjson.cpp b/tests/auto/corelib/serialization/json/tst_qtjson.cpp index ac1ef76f8c..5d4a9b15b1 100644 --- a/tests/auto/corelib/serialization/json/tst_qtjson.cpp +++ b/tests/auto/corelib/serialization/json/tst_qtjson.cpp @@ -82,6 +82,7 @@ private Q_SLOTS: void nullObject(); void constNullObject(); + void keySorting_data(); void keySorting(); void undefinedValues(); @@ -1263,21 +1264,55 @@ void tst_QtJson::constNullObject() QCOMPARE(nullObject["foo"], QJsonValue(QJsonValue::Undefined)); } +void tst_QtJson::keySorting_data() +{ + QTest::addColumn("json"); + QTest::addColumn("sortedKeys"); + + QStringList list = {"A", "B"}; + QTest::newRow("sorted-ascii-2") << R"({ "A": false, "B": true })" << list; + const char *json = "{ \"B\": true, \"A\": false }"; + QTest::newRow("unsorted-ascii-2") << json << list; + + list = QStringList{"A", "B", "C", "D", "E"}; + QTest::newRow("sorted-ascii-5") << R"({"A": 1, "B": 2, "C": 3, "D": 4, "E": 5})" << list; + QTest::newRow("unsorted-ascii-5") << R"({"A": 1, "C": 3, "D": 4, "B": 2, "E": 5})" << list; + QTest::newRow("inverse-sorted-ascii-5") << R"({"E": 5, "D": 4, "C": 3, "B": 2, "A": 1})" << list; + + list = QStringList{"á", "é", "í", "ó", "ú"}; + QTest::newRow("sorted-latin1") << R"({"á": 1, "é": 2, "í": 3, "ó": 4, "ú": 5})" << list; + QTest::newRow("unsorted-latin1") << R"({"á": 1, "í": 3, "ó": 4, "é": 2, "ú": 5})" << list; + QTest::newRow("inverse-sorted-latin1") << R"({"ú": 5, "ó": 4, "í": 3, "é": 2, "á": 1})" << list; + + QTest::newRow("sorted-escaped-latin1") << R"({"\u00e1": 1, "\u00e9": 2, "\u00ed": 3, "\u00f3": 4, "\u00fa": 5})" << list; + QTest::newRow("unsorted-escaped-latin1") << R"({"\u00e1": 1, "\u00ed": 3, "\u00f3": 4, "\u00e9": 2, "\u00fa": 5})" << list; + QTest::newRow("inverse-sorted-escaped-latin1") << R"({"\u00fa": 5, "\u00f3": 4, "\u00ed": 3, "\u00e9": 2, "\u00e1": 1})" << list; + + list = QStringList{"A", "α", "Я", "€", "测"}; + QTest::newRow("sorted") << R"({"A": 1, "α": 2, "Я": 3, "€": 4, "测": 5})" << list; + QTest::newRow("unsorted") << R"({"A": 1, "Я": 3, "€": 4, "α": 2, "测": 5})" << list; + QTest::newRow("inverse-sorted") << R"({"测": 5, "€": 4, "Я": 3, "α": 2, "A": 1})" << list; + + QTest::newRow("sorted-escaped") << R"({"A": 1, "\u03b1": 2, "\u042f": 3, "\u20ac": 4, "\u6d4b": 5})" << list; + QTest::newRow("unsorted-escaped") << R"({"A": 1, "\u042f": 3, "\u20ac": 4, "\u03b1": 2, "\u6d4b": 5})" << list; + QTest::newRow("inverse-sorted-escaped") << R"({"\u6d4b": 5, "\u20ac": 4, "\u042f": 3, "\u03b1": 2, "A": 1})" << list; +} + void tst_QtJson::keySorting() { - const char *json = "{ \"B\": true, \"A\": false }"; - QJsonDocument doc = QJsonDocument::fromJson(json); + QFETCH(QString, json); + QFETCH(QStringList, sortedKeys); + QJsonDocument doc = QJsonDocument::fromJson(json.toUtf8()); QCOMPARE(doc.isObject(), true); QJsonObject o = doc.object(); - QCOMPARE(o.size(), 2); + QCOMPARE(o.size(), sortedKeys.size()); + QCOMPARE(o.keys(), sortedKeys); QJsonObject::const_iterator it = o.constBegin(); - QCOMPARE(it.key(), QLatin1String("A")); - ++it; - QCOMPARE(it.key(), QLatin1String("B")); - - QCOMPARE(o.keys(), QStringList() << QLatin1String("A") << QLatin1String("B")); + QStringList::const_iterator it2 = sortedKeys.constBegin(); + for ( ; it != o.constEnd(); ++it, ++it2) + QCOMPARE(it.key(), *it2); } void tst_QtJson::undefinedValues()