<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">
<br class="">
<div>
<blockquote type="cite" class="">
<div class="">On 1 Jul 2020, at 23:21, Thiago Macieira <<a href="mailto:thiago.macieira@intel.com" class="">thiago.macieira@intel.com</a>> wrote:</div>
<div class="">
<div class=""><br class="">
<br class="">
It wasn't entirely undocumented. QJsonValue::fromVariant said:<br class="">
   For all other QVariant types a conversion to a QString will be attempted.<br class="">
   If the returned string<br class="">
   is empty, a Null QJsonValue will be stored, otherwise a String value using<br class="">
   the returned QString.<br class="">
[this text is changed in <a href="https://codereview.qt-project.org/305996" class="">
https://codereview.qt-project.org/305996</a>]<br class="">
<br class="">
Since it depends on QVariant::toString(), it's implicit that the behaviour can <br class="">
change from version to version. For Qt 6.0, we're already discussing whether <br class="">
conversion between types in QVariant makes sense and toString() is one of <br class="">
those cases.<br class="">
</div>
</div>
</blockquote>
<div><br class="">
</div>
<div>Looking at this some more, the documented behaviour is:</div>
<div><br class="">
</div>
<div><span style="color: rgb(64, 66, 68); font-family: "Titillium Web", Arial, Helvetica, sans-serif; font-variant-ligatures: normal; orphans: 2; widows: 2; background-color: rgb(255, 255, 255);" class="">"For all other </span><a href="https://doc.qt.io/qt-5/qvariant.html" style="margin: 0px; padding: 0px; border: 0px; vertical-align: baseline; color: rgb(23, 168, 26); text-decoration: none; transition-duration: 0.3s; font-family: "Titillium Web", Arial, Helvetica, sans-serif; font-variant-ligatures: normal; orphans: 2; widows: 2; background-color: rgb(255, 255, 255);" class="">QVariant</a><span style="color: rgb(64, 66, 68); font-family: "Titillium Web", Arial, Helvetica, sans-serif; font-variant-ligatures: normal; orphans: 2; widows: 2; background-color: rgb(255, 255, 255);" class=""> types
 a conversion to a </span><a href="https://doc.qt.io/qt-5/qstring.html" style="margin: 0px; padding: 0px; border: 0px; vertical-align: baseline; color: rgb(23, 168, 26); text-decoration: none; transition-duration: 0.3s; font-family: "Titillium Web", Arial, Helvetica, sans-serif; font-variant-ligatures: normal; orphans: 2; widows: 2; background-color: rgb(255, 255, 255);" class="">QString</a><span style="color: rgb(64, 66, 68); font-family: "Titillium Web", Arial, Helvetica, sans-serif; font-variant-ligatures: normal; orphans: 2; widows: 2; background-color: rgb(255, 255, 255);" class=""> will
 be attempted. If the returned string is empty, a Null </span><a href="https://doc.qt.io/qt-5/qjsonvalue.html" style="margin: 0px; padding: 0px; border: 0px; vertical-align: baseline; color: rgb(23, 168, 26); text-decoration: none; transition-duration: 0.3s; font-family: "Titillium Web", Arial, Helvetica, sans-serif; font-variant-ligatures: normal; orphans: 2; widows: 2; background-color: rgb(255, 255, 255);" class="">QJsonValue</a><span style="color: rgb(64, 66, 68); font-family: "Titillium Web", Arial, Helvetica, sans-serif; font-variant-ligatures: normal; orphans: 2; widows: 2; background-color: rgb(255, 255, 255);" class=""> will
 be stored, otherwise a String value using the returned </span><a href="https://doc.qt.io/qt-5/qstring.html" style="margin: 0px; padding: 0px; border: 0px; vertical-align: baseline; color: rgb(23, 168, 26); text-decoration: none; transition-duration: 0.3s; font-family: "Titillium Web", Arial, Helvetica, sans-serif; font-variant-ligatures: normal; orphans: 2; widows: 2; background-color: rgb(255, 255, 255);" class="">QString</a><span style="color: rgb(64, 66, 68); font-family: "Titillium Web", Arial, Helvetica, sans-serif; font-variant-ligatures: normal; orphans: 2; widows: 2; background-color: rgb(255, 255, 255);" class="">."</span></div>
<div><br class="">
</div>
<div><font face="Menlo" class="">auto variant = QVariant::fromValue(QByteArray("ASCII text.\n"));</font></div>
<div>
<div><font face="Menlo" class="">auto stringForJson = variant.toString(); // "a conversion to a QString will be attempted"</font></div>
<div><font face="Menlo" class="">printf("%s", qPrintable(stringForJson));</font></div>
<div><br class="">
</div>
<div>
<div>Which for me results in printing "<b style="background-color: rgb(255, 255, 255);" class=""><font face="Menlo" class=""><span style="font-size: 12px;" class="">ASCII text.” </span></font></b></div>
<div class=""><br class="">
</div>
<div class="">
<div>The new behavior injects an extra step here:</div>
</div>
<div class=""><br class="">
</div>
</div>
<div><font face="Menlo" class="">variant = QVariant::fromValue(QByteArray("ASCII text.\n"));</font></div>
<div><font face="Menlo" class="">auto base64Encoded = variant.toByteArray().toBase64(); // <-----</font></div>
<div><font face="Menlo" class="">auto stringForJsonBase64 = QString::fromUtf8(base64Encoded);</font></div>
<div><font face="Menlo" class="">printf("%s", qPrintable(stringForJsonBase64));</font></div>
</div>
<div><br class="">
</div>
<div>Which does not match the documented behavior, so I think a revert makes sense to avoid the surprising behavior change.</div>
<div><br class="">
</div>
<div>Especially since users who encode true binary data will do their own (base64)-encoding anyways. </div>
<div><br class="">
</div>
<div>We can document that this might result in data loss and recommend pre-encoding the data. At least the user is in control then.</div>
<div><br class="">
</div>
<div>Tor Arne </div>
<div><br class="">
</div>
<blockquote type="cite" class="">
<div class="">
<div class=""><br class="">
As can be seen in the testcase, the old behaviour is silently lossy, which <br class="">
means it's dangerous. The new behaviour is surprising, but it's not lossy, as <br class="">
the original byte data is retrievable:<br class="">
<br class="">
<span class="Apple-tab-span" style="white-space:pre"></span>$ base64 -d <<<QVNDSUkgdGV4dC4K
<br class="">
<span class="Apple-tab-span" style="white-space:pre"></span>ASCII text.<br class="">
<span class="Apple-tab-span" style="white-space:pre"></span>$ base64 -d <<<UulzdW3p | od -tc<br class="">
<span class="Apple-tab-span" style="white-space:pre"></span>0000000   R 351   s   u   m 351<br class="">
<span class="Apple-tab-span" style="white-space:pre"></span>$ base64 -d <<<UsOpc3Vtw6k=<br class="">
<span class="Apple-tab-span" style="white-space:pre"></span>Résumé                                                                                                                                          <br class="">
<br class="">
(the type change is inevitable and is accepted by the user)<br class="">
<br class="">
Request For Opinion: what do we do now? See options at the beginning of the <br class="">
email.<br class="">
<br class="">
Note also this behaviour is not completely correct, because fromVariant <br class="">
converts *empty* strings to null, but it should only convert null QStrings to <br class="">
null, leaving empty strings alone (that's the QCborValue::fromVariant <br class="">
behaviour). I also don't think the implementation is self-consistent, because <br class="">
the conversion code appears more than once.<br class="">
<br class="">
[1] <a href="https://tools.ietf.org/html/rfc7049#section-4.1" class="">https://tools.ietf.org/html/rfc7049#section-4.1</a><br class="">
-- <br class="">
Thiago Macieira - thiago.macieira (AT) <a href="http://intel.com" class="">intel.com</a><br class="">
 Software Architect - Intel System Software Products<br class="">
<br class="">
<br class="">
<br class="">
_______________________________________________<br class="">
Development mailing list<br class="">
<a href="mailto:Development@qt-project.org" class="">Development@qt-project.org</a><br class="">
https://lists.qt-project.org/listinfo/development<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</body>
</html>