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>

7 files changed, 77 insertions, 20 deletions

diff --git a/src/corelib/serialization/qcborarray.h b/src/corelib/serialization/qcborarray.h
index 8d0aceac33..4ad8100bfe 100644
--- a/src/corelib/serialization/qcborarray.h
+++ b/src/corelib/serialization/qcborarray.h
@@ -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;
 };
diff --git a/src/corelib/serialization/qcbormap.h b/src/corelib/serialization/qcbormap.h
index 7e1a4cb485..cd1824f8e7 100644
--- a/src/corelib/serialization/qcbormap.h
+++ b/src/corelib/serialization/qcbormap.h
@@ -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;
diff --git a/src/corelib/serialization/qjson_p.h b/src/corelib/serialization/qjson_p.h
index 4589284ef3..b6f01bff1c 100644
--- a/src/corelib/serialization/qjson_p.h
+++ b/src/corelib/serialization/qjson_p.h
@@ -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
diff --git a/src/corelib/serialization/qjsonarray.cpp b/src/corelib/serialization/qjsonarray.cpp
index 5395319b7c..7e29bdea3f 100644
--- a/src/corelib/serialization/qjsonarray.cpp
+++ b/src/corelib/serialization/qjsonarray.cpp
@@ -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);
 }
 
 /*!
diff --git a/src/corelib/serialization/qjsoncbor.cpp b/src/corelib/serialization/qjsoncbor.cpp
index e0eab74a02..e836935f6f 100644
--- a/src/corelib/serialization/qjsoncbor.cpp
+++ b/src/corelib/serialization/qjsoncbor.cpp
@@ -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.
 
diff --git a/src/corelib/serialization/qjsonobject.cpp b/src/corelib/serialization/qjsonobject.cpp
index beeb3b5a54..419021a798 100644
--- a/src/corelib/serialization/qjsonobject.cpp
+++ b/src/corelib/serialization/qjsonobject.cpp
@@ -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)
diff --git a/src/corelib/serialization/qjsonvalue.cpp b/src/corelib/serialization/qjsonvalue.cpp
index 34fa84cdc1..0f922f521a 100644
--- a/src/corelib/serialization/qjsonvalue.cpp
+++ b/src/corelib/serialization/qjsonvalue.cpp
@@ -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: