On 1 Jul 2020, at 23:21, Thiago Macieira <thiago.macie...@intel.com<mailto:thiago.macie...@intel.com>> wrote:
It wasn't entirely undocumented. QJsonValue::fromVariant said: For all other QVariant types a conversion to a QString will be attempted. If the returned string is empty, a Null QJsonValue will be stored, otherwise a String value using the returned QString. [this text is changed in https://codereview.qt-project.org/305996] Since it depends on QVariant::toString(), it's implicit that the behaviour can change from version to version. For Qt 6.0, we're already discussing whether conversion between types in QVariant makes sense and toString() is one of those cases. Looking at this some more, the documented behaviour is: "For all other QVariant<https://doc.qt.io/qt-5/qvariant.html> types a conversion to a QString<https://doc.qt.io/qt-5/qstring.html> will be attempted. If the returned string is empty, a Null QJsonValue<https://doc.qt.io/qt-5/qjsonvalue.html> will be stored, otherwise a String value using the returned QString<https://doc.qt.io/qt-5/qstring.html>." auto variant = QVariant::fromValue(QByteArray("ASCII text.\n")); auto stringForJson = variant.toString(); // "a conversion to a QString will be attempted" printf("%s", qPrintable(stringForJson)); Which for me results in printing "ASCII text.” The new behavior injects an extra step here: variant = QVariant::fromValue(QByteArray("ASCII text.\n")); auto base64Encoded = variant.toByteArray().toBase64(); // <----- auto stringForJsonBase64 = QString::fromUtf8(base64Encoded); printf("%s", qPrintable(stringForJsonBase64)); Which does not match the documented behavior, so I think a revert makes sense to avoid the surprising behavior change. Especially since users who encode true binary data will do their own (base64)-encoding anyways. We can document that this might result in data loss and recommend pre-encoding the data. At least the user is in control then. Tor Arne As can be seen in the testcase, the old behaviour is silently lossy, which means it's dangerous. The new behaviour is surprising, but it's not lossy, as the original byte data is retrievable: $ base64 -d <<<QVNDSUkgdGV4dC4K ASCII text. $ base64 -d <<<UulzdW3p | od -tc 0000000 R 351 s u m 351 $ base64 -d <<<UsOpc3Vtw6k= Résumé (the type change is inevitable and is accepted by the user) Request For Opinion: what do we do now? See options at the beginning of the email. Note also this behaviour is not completely correct, because fromVariant converts *empty* strings to null, but it should only convert null QStrings to null, leaving empty strings alone (that's the QCborValue::fromVariant behaviour). I also don't think the implementation is self-consistent, because the conversion code appears more than once. [1] https://tools.ietf.org/html/rfc7049#section-4.1 -- Thiago Macieira - thiago.macieira (AT) intel.com<http://intel.com> Software Architect - Intel System Software Products _______________________________________________ Development mailing list Development@qt-project.org<mailto:Development@qt-project.org> https://lists.qt-project.org/listinfo/development
_______________________________________________ Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
