Fix conversions to JSON from QVariant

After reimplementing Qt JSON support on top of CBOR, there were
unintended behavior changes when converting QVariant{, List, Map} to
QJson{Value, Array, List} due to reusing the code for converting
QVariant* types to CBOR types, and from CBOR types to corresponding JSON
types. In particular, conversions from QVariant containing QByteArray to
JSON has been affected: according to RFC 7049, when converting from
CBOR to JSON, raw byte array data must be encoded in base64url when
converting to a JSON string. As a result QVariant* types containing
QByteArray data ended up base64url-encoded when converted to JSON,
instead of converting using QString::fromUtf8() as before.

There were also differences when converting QRegularExpression.

Reverted the behavior changes by adding a flag to internal methods for
converting CBOR to JSON, to distinguish whether the conversion is done
from QVariant* or CBOR types. These methods now will fall back to the old
behavior, if the conversion is done using QJson*::fromVariant*().

Additionally fixed QJsonValue::fromVariant conversion for NaN and
infinities: they should always convert to QJsonValue::Null. This works
correctly when converting from variant to QJsonArray/QJsonObject, but has
been wrong for QJsonValue.

Added more tests to verify the expected behavior.

[ChangeLog][Important Behavior Changes] Restored pre-5.15.0 behavior
when converting from QVariant* to QJson* types. Unforeseen consequences
of changes in 5.15.0 caused QByteArray data to be base64url-encoded; the
handling of QRegularExpression was also unintentionally changed. These
conversions are now reverted to the prior behavior. Additionally fixed
QJsonValue::fromVariant conversions for NaN and infinities: they should
always convert to QJsonValue::Null.

Fixes: QTBUG-84739
Change-Id: Iaee667d00e5363906eedbb67948b7b39c9d0bc78
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
bb10
Sona Kurazyan 2020-08-07 14:57:11 +02:00
parent 5b76414b43
commit 1df02b5f98
8 changed files with 230 additions and 20 deletions

View File

@ -49,6 +49,8 @@ QT_BEGIN_NAMESPACE
class QJsonArray;
class QDataStream;
namespace QJsonPrivate { class Variant; }
class QCborContainerPrivate;
class Q_CORE_EXPORT QCborArray
{
@ -273,6 +275,7 @@ private:
friend QCborValue;
friend QCborValueRef;
friend class QJsonPrivate::Variant;
explicit QCborArray(QCborContainerPrivate &dd) noexcept;
QExplicitlySharedDataPointer<QCborContainerPrivate> d;
};

View File

@ -54,6 +54,8 @@ typedef QHash<QString, QVariant> QVariantHash;
class QJsonObject;
class QDataStream;
namespace QJsonPrivate { class Variant; }
class QCborContainerPrivate;
class Q_CORE_EXPORT QCborMap
{
@ -327,6 +329,7 @@ public:
private:
friend class QCborValue;
friend class QCborValueRef;
friend class QJsonPrivate::Variant;
void detach(qsizetype reserve = 0);
explicit QCborMap(QCborContainerPrivate &dd) noexcept;

View File

@ -215,6 +215,13 @@ public:
}
};
class Variant
{
public:
static QJsonObject toJsonObject(const QVariantMap &map);
static QJsonArray toJsonArray(const QVariantList &list);
};
} // namespace QJsonPrivate
QT_END_NAMESPACE

View File

@ -261,11 +261,14 @@ QJsonArray QJsonArray::fromStringList(const QStringList &list)
The QVariant values in \a list will be converted to JSON values.
\note Conversion from \l QVariant is not completely lossless. Please see
the documentation in QJsonValue::fromVariant() for more information.
\sa toVariantList(), QJsonValue::fromVariant()
*/
QJsonArray QJsonArray::fromVariantList(const QVariantList &list)
{
return QCborArray::fromVariantList(list).toJsonArray();
return QJsonPrivate::Variant::toJsonArray(list);
}
/*!

View File

@ -55,6 +55,8 @@ QT_BEGIN_NAMESPACE
using namespace QtCbor;
enum class ConversionMode { FromRaw, FromVariantToJson };
static QJsonValue fpToJson(double v)
{
return qt_is_finite(v) ? QJsonValue(v) : QJsonValue();
@ -89,7 +91,8 @@ static QString encodeByteArray(const QCborContainerPrivate *d, qsizetype idx, QC
return QString::fromLatin1(data, data.size());
}
static QString makeString(const QCborContainerPrivate *d, qsizetype idx);
static QString makeString(const QCborContainerPrivate *d, qsizetype idx,
ConversionMode mode = ConversionMode::FromRaw);
static QString maybeEncodeTag(const QCborContainerPrivate *d)
{
@ -134,7 +137,8 @@ static QString encodeTag(const QCborContainerPrivate *d)
return s;
}
static Q_NEVER_INLINE QString makeString(const QCborContainerPrivate *d, qsizetype idx)
static Q_NEVER_INLINE QString makeString(const QCborContainerPrivate *d, qsizetype idx,
ConversionMode mode)
{
const auto &e = d->elements.at(idx);
@ -146,7 +150,9 @@ static Q_NEVER_INLINE QString makeString(const QCborContainerPrivate *d, qsizety
return QString::number(e.fpvalue());
case QCborValue::ByteArray:
return encodeByteArray(d, idx, QCborTag(QCborKnownTags::ExpectedBase64url));
return mode == ConversionMode::FromVariantToJson
? d->stringAt(idx)
: encodeByteArray(d, idx, QCborTag(QCborKnownTags::ExpectedBase64url));
case QCborValue::String:
return d->stringAt(idx);
@ -190,7 +196,8 @@ static Q_NEVER_INLINE QString makeString(const QCborContainerPrivate *d, qsizety
return simpleTypeString(e.type);
}
QJsonValue qt_convertToJson(QCborContainerPrivate *d, qsizetype idx);
QJsonValue qt_convertToJson(QCborContainerPrivate *d, qsizetype idx,
ConversionMode mode = ConversionMode::FromRaw);
static QJsonValue convertExtendedTypeToJson(QCborContainerPrivate *d)
{
@ -222,35 +229,37 @@ static QJsonValue convertExtendedTypeToJson(QCborContainerPrivate *d)
}
// We need to do this because sub-objects may need conversion.
static QJsonArray convertToJsonArray(QCborContainerPrivate *d)
static QJsonArray convertToJsonArray(QCborContainerPrivate *d,
ConversionMode mode = ConversionMode::FromRaw)
{
QJsonArray a;
if (d) {
for (qsizetype idx = 0; idx < d->elements.size(); ++idx)
a.append(qt_convertToJson(d, idx));
a.append(qt_convertToJson(d, idx, mode));
}
return a;
}
// We need to do this because the keys need to be sorted and converted to strings
// and sub-objects may need recursive conversion.
static QJsonObject convertToJsonObject(QCborContainerPrivate *d)
static QJsonObject convertToJsonObject(QCborContainerPrivate *d,
ConversionMode mode = ConversionMode::FromRaw)
{
QJsonObject o;
if (d) {
for (qsizetype idx = 0; idx < d->elements.size(); idx += 2)
o.insert(makeString(d, idx), qt_convertToJson(d, idx + 1));
o.insert(makeString(d, idx), qt_convertToJson(d, idx + 1, mode));
}
return o;
}
QJsonValue qt_convertToJson(QCborContainerPrivate *d, qsizetype idx)
QJsonValue qt_convertToJson(QCborContainerPrivate *d, qsizetype idx, ConversionMode mode)
{
// encoding the container itself
if (idx == -QCborValue::Array)
return convertToJsonArray(d);
return convertToJsonArray(d, mode);
if (idx == -QCborValue::Map)
return convertToJsonObject(d);
return convertToJsonObject(d, mode);
if (idx < 0) {
// tag-like type
if (!d || d->elements.size() != 2)
@ -264,6 +273,15 @@ QJsonValue qt_convertToJson(QCborContainerPrivate *d, qsizetype idx)
case QCborValue::Integer:
return QJsonValue(e.value);
case QCborValue::ByteArray:
if (mode == ConversionMode::FromVariantToJson) {
const auto value = makeString(d, idx, mode);
return value.isEmpty() ? QJsonValue() : QJsonPrivate::Value::fromTrustedCbor(value);
}
break;
case QCborValue::RegularExpression:
if (mode == ConversionMode::FromVariantToJson)
return QJsonValue();
break;
case QCborValue::String:
case QCborValue::SimpleType:
// make string
@ -274,10 +292,10 @@ QJsonValue qt_convertToJson(QCborContainerPrivate *d, qsizetype idx)
case QCborValue::Tag:
case QCborValue::DateTime:
case QCborValue::Url:
case QCborValue::RegularExpression:
case QCborValue::Uuid:
// recurse
return qt_convertToJson(e.flags & Element::IsContainer ? e.container : nullptr, -e.type);
return qt_convertToJson(e.flags & Element::IsContainer ? e.container : nullptr, -e.type,
mode);
case QCborValue::Null:
case QCborValue::Undefined:
@ -294,7 +312,7 @@ QJsonValue qt_convertToJson(QCborContainerPrivate *d, qsizetype idx)
return fpToJson(e.fpvalue());
}
return QJsonPrivate::Value::fromTrustedCbor(makeString(d, idx));
return QJsonPrivate::Value::fromTrustedCbor(makeString(d, idx, mode));
}
/*!
@ -429,6 +447,12 @@ QJsonArray QCborArray::toJsonArray() const
return convertToJsonArray(d.data());
}
QJsonArray QJsonPrivate::Variant::toJsonArray(const QVariantList &list)
{
const auto cborArray = QCborArray::fromVariantList(list);
return convertToJsonArray(cborArray.d.data(), ConversionMode::FromVariantToJson);
}
/*!
Recursively converts every \l QCborValue value in this map to JSON using
QCborValue::toJsonValue() and creates a string key for all keys that aren't
@ -471,6 +495,12 @@ QJsonObject QCborMap::toJsonObject() const
return convertToJsonObject(d.data());
}
QJsonObject QJsonPrivate::Variant::toJsonObject(const QVariantMap &map)
{
const auto cborMap = QCborMap::fromVariantMap(map);
return convertToJsonObject(cborMap.d.data(), ConversionMode::FromVariantToJson);
}
/*!
Converts this value to a native Qt type and returns the corresponding QVariant.

View File

@ -205,11 +205,14 @@ QJsonObject &QJsonObject::operator =(const QJsonObject &other)
The keys in \a map will be used as the keys in the JSON object,
and the QVariant values will be converted to JSON values.
\note Conversion from \l QVariant is not completely lossless. Please see
the documentation in QJsonValue::fromVariant() for more information.
\sa fromVariantHash(), toVariantMap(), QJsonValue::fromVariant()
*/
QJsonObject QJsonObject::fromVariantMap(const QVariantMap &map)
{
return QCborMap::fromVariantMap(map).toJsonObject();
return QJsonPrivate::Variant::toJsonObject(map);
}
/*!
@ -231,6 +234,9 @@ QVariantMap QJsonObject::toVariantMap() const
The keys in \a hash will be used as the keys in the JSON object,
and the QVariant values will be converted to JSON values.
\note Conversion from \l QVariant is not completely lossless. Please see
the documentation in QJsonValue::fromVariant() for more information.
\sa fromVariantMap(), toVariantHash(), QJsonValue::fromVariant()
*/
QJsonObject QJsonObject::fromVariantHash(const QVariantHash &hash)

View File

@ -351,7 +351,6 @@ void QJsonValue::swap(QJsonValue &other) noexcept
error cases as e.g. accessing a non existing key in a QJsonObject.
*/
/*!
Converts \a variant to a QJsonValue and returns it.
@ -461,7 +460,11 @@ void QJsonValue::swap(QJsonValue &other) noexcept
For other types not listed above, a conversion to string will be attempted,
usually but not always by calling QVariant::toString(). If the conversion
fails the value is replaced by a null JSON value. Note that
QVariant::toString() is also lossy for the majority of types.
QVariant::toString() is also lossy for the majority of types. For example,
if the passed QVariant is representing raw byte array data, it is recommended
to pre-encode it to \l {https://www.ietf.org/rfc/rfc4648.txt}{Base64} (or
another lossless encoding), otherwise a lossy conversion using QString::fromUtf8()
will be used.
Please note that the conversions via QVariant::toString() are subject to
change at any time. Both QVariant and QJsonValue may be extended in the
@ -488,8 +491,10 @@ QJsonValue QJsonValue::fromVariant(const QVariant &variant)
return QJsonValue(variant.toLongLong());
Q_FALLTHROUGH();
case QMetaType::Float:
case QMetaType::Double:
return QJsonValue(variant.toDouble());
case QMetaType::Double: {
double v = variant.toDouble();
return qt_is_finite(v) ? QJsonValue(v) : QJsonValue();
}
case QMetaType::QString:
return QJsonValue(variant.toString());
case QMetaType::QStringList:

View File

@ -163,6 +163,10 @@ private Q_SLOTS:
void streamVariantSerialization();
void escapeSurrogateCodePoints_data();
void escapeSurrogateCodePoints();
void fromToVariantConversions_data();
void fromToVariantConversions();
private:
QString testDataDir;
};
@ -3301,5 +3305,154 @@ void tst_QtJson::escapeSurrogateCodePoints()
QVERIFY(buffer.contains(escStr));
}
void tst_QtJson::fromToVariantConversions_data()
{
QTest::addColumn<QVariant>("variant");
QTest::addColumn<QJsonValue>("json");
QTest::addColumn<QVariant>("jsonToVariant");
QByteArray utf8("\xC4\x90\xC4\x81\xC5\xA3\xC3\xA2"); // Đāţâ
QDateTime dt = QDateTime::currentDateTimeUtc();
QUuid uuid = QUuid::createUuid();
constexpr qlonglong maxInt = std::numeric_limits<qlonglong>::max();
constexpr qlonglong minInt = std::numeric_limits<qlonglong>::min();
constexpr double maxDouble = std::numeric_limits<double>::max();
constexpr double minDouble = std::numeric_limits<double>::min();
QTest::newRow("default") << QVariant() << QJsonValue(QJsonValue::Null)
<< QVariant::fromValue(nullptr);
QTest::newRow("nullptr") << QVariant::fromValue(nullptr) << QJsonValue(QJsonValue::Null)
<< QVariant::fromValue(nullptr);
QTest::newRow("bool") << QVariant(true) << QJsonValue(true) << QVariant(true);
QTest::newRow("int pos") << QVariant(123) << QJsonValue(123) << QVariant(qlonglong(123));
QTest::newRow("int neg") << QVariant(-123) << QJsonValue(-123) << QVariant(qlonglong(-123));
QTest::newRow("int big pos") << QVariant((1ll << 55) +1) << QJsonValue((1ll << 55) + 1)
<< QVariant(qlonglong((1ll << 55) + 1));
QTest::newRow("int big neg") << QVariant(-(1ll << 55) + 1) << QJsonValue(-(1ll << 55) + 1)
<< QVariant(qlonglong(-(1ll << 55) + 1));
QTest::newRow("int max") << QVariant(maxInt) << QJsonValue(maxInt) << QVariant(maxInt);
QTest::newRow("int min") << QVariant(minInt) << QJsonValue(minInt) << QVariant(minInt);
QTest::newRow("double pos") << QVariant(123.) << QJsonValue(123.) << QVariant(qlonglong(123.));
QTest::newRow("double neg") << QVariant(-123.) << QJsonValue(-123.)
<< QVariant(qlonglong(-123.));
QTest::newRow("double big") << QVariant(maxDouble - 1000) << QJsonValue(maxDouble - 1000)
<< QVariant(maxDouble - 1000);
QTest::newRow("double max") << QVariant(maxDouble) << QJsonValue(maxDouble)
<< QVariant(maxDouble);
QTest::newRow("double min") << QVariant(minDouble) << QJsonValue(minDouble)
<< QVariant(minDouble);
QTest::newRow("double big neg") << QVariant(1000 - maxDouble) << QJsonValue(1000 - maxDouble)
<< QVariant(1000 - maxDouble);
QTest::newRow("double max neg") << QVariant(-maxDouble) << QJsonValue(-maxDouble)
<< QVariant(-maxDouble);
QTest::newRow("double min neg") << QVariant(-minDouble) << QJsonValue(-minDouble)
<< QVariant(-minDouble);
QTest::newRow("string null") << QVariant(QString()) << QJsonValue(QString())
<< QVariant(QString());
QTest::newRow("string empty") << QVariant(QString("")) << QJsonValue(QString(""))
<< QVariant(QString(""));
QTest::newRow("string ascii") << QVariant(QString("Data")) << QJsonValue(QString("Data"))
<< QVariant(QString("Data"));
QTest::newRow("string utf8") << QVariant(QString(utf8)) << QJsonValue(QString(utf8))
<< QVariant(QString(utf8));
QTest::newRow("bytearray null") << QVariant(QByteArray()) << QJsonValue(QJsonValue::Null)
<< QVariant::fromValue(nullptr);
QTest::newRow("bytearray empty") << QVariant(QByteArray()) << QJsonValue(QJsonValue::Null)
<< QVariant::fromValue(nullptr);
QTest::newRow("bytearray ascii") << QVariant(QByteArray("Data")) << QJsonValue(QString("Data"))
<< QVariant(QString("Data"));
QTest::newRow("bytearray utf8") << QVariant(utf8) << QJsonValue(QString(utf8))
<< QVariant(QString(utf8));
QTest::newRow("datetime") << QVariant(dt) << QJsonValue(dt.toString(Qt::ISODateWithMs))
<< QVariant(dt.toString(Qt::ISODateWithMs));
QTest::newRow("url") << QVariant(QUrl("http://example.com/{q}"))
<< QJsonValue("http://example.com/%7Bq%7D")
<< QVariant(QString("http://example.com/%7Bq%7D"));
QTest::newRow("uuid") << QVariant(QUuid(uuid))
<< QJsonValue(uuid.toString(QUuid::WithoutBraces))
<< QVariant(uuid.toString(QUuid::WithoutBraces));
QTest::newRow("regexp") << QVariant(QRegularExpression(".")) << QJsonValue(QJsonValue::Null)
<< QVariant::fromValue(nullptr);
QTest::newRow("inf") << QVariant(qInf()) << QJsonValue(QJsonValue::Null)
<< QVariant::fromValue(nullptr);
QTest::newRow("-inf") << QVariant(-qInf()) << QJsonValue(QJsonValue::Null)
<< QVariant::fromValue(nullptr);
QTest::newRow("NaN") << QVariant(qQNaN()) << QJsonValue(QJsonValue::Null)
<< QVariant::fromValue(nullptr);
}
void tst_QtJson::fromToVariantConversions()
{
QFETCH(QVariant, variant);
QFETCH(QJsonValue, json);
QFETCH(QVariant, jsonToVariant);
QVariant variantFromJson(json);
QVariant variantFromJsonArray(QJsonArray { json });
QVariant variantFromJsonObject(QVariantMap { { "foo", variant } });
QJsonObject object { QPair<QString, QJsonValue>("foo", json) };
// QJsonValue <> QVariant
{
QCOMPARE(QJsonValue::fromVariant(variant), json);
// test the same for QVariant from QJsonValue/QJsonArray/QJsonObject
QCOMPARE(QJsonValue::fromVariant(variantFromJson), json);
QCOMPARE(QJsonValue::fromVariant(variantFromJsonArray), QJsonArray { json });
QCOMPARE(QJsonValue::fromVariant(variantFromJsonObject), object);
// QJsonValue to variant
QCOMPARE(json.toVariant(), jsonToVariant);
QCOMPARE(json.toVariant().userType(), jsonToVariant.userType());
// variant to QJsonValue
QCOMPARE(QVariant(json).toJsonValue(), json);
}
// QJsonArray <> QVariantList
{
QCOMPARE(QJsonArray::fromVariantList(QVariantList { variant }), QJsonArray { json });
// test the same for QVariantList from QJsonValue/QJsonArray/QJsonObject
QCOMPARE(QJsonArray::fromVariantList(QVariantList { variantFromJson }),
QJsonArray { json });
QCOMPARE(QJsonArray::fromVariantList(QVariantList { variantFromJsonArray }),
QJsonArray {{ QJsonArray { json } }});
QCOMPARE(QJsonArray::fromVariantList(QVariantList { variantFromJsonObject }),
QJsonArray { object });
// QJsonArray to variant
QCOMPARE(QJsonArray { json }.toVariantList(), QVariantList { jsonToVariant });
// variant to QJsonArray
QCOMPARE(QVariant(QJsonArray { json }).toJsonArray(), QJsonArray { json });
}
// QJsonObject <> QVariantMap
{
QCOMPARE(QJsonObject::fromVariantMap(QVariantMap { { "foo", variant } }), object);
// test the same for QVariantMap from QJsonValue/QJsonArray/QJsonObject
QCOMPARE(QJsonObject::fromVariantMap(QVariantMap { { "foo", variantFromJson } }), object);
QJsonObject nestedArray { QPair<QString, QJsonArray>("bar", QJsonArray { json }) };
QJsonObject nestedObject { QPair<QString, QJsonObject>("bar", object) };
QCOMPARE(QJsonObject::fromVariantMap(QVariantMap { { "bar", variantFromJsonArray } }),
nestedArray);
QCOMPARE(QJsonObject::fromVariantMap(QVariantMap { { "bar", variantFromJsonObject } }),
nestedObject);
// QJsonObject to variant
QCOMPARE(object.toVariantMap(), QVariantMap({ { "foo", jsonToVariant } }));
// variant to QJsonObject
QCOMPARE(QVariant(object).toJsonObject(), object);
}
}
QTEST_MAIN(tst_QtJson)
#include "tst_qtjson.moc"