Apple OS: Handle QSetting strings with embedded zero-bytes

Saving strings with embedded zero-bytes (\0) as CFStrings would
sometimes fail, and only write the part of the string leading
up to the first zero-byte, instead of all the way to the final
zero-terminator. This bug was revealed by the code-path that
falls back to storing e.g. QTime as strings, via the helper
method QSettingsPrivate::variantToString().

We now use the same approach as on platforms such as Windows
and WinRT, where the string produced by variantToString() is
checked for null-bytes, and if so, stored using a binary
representation instead of as a string. For our case that
means we fall back to CFData when detecting the null-byte.

To separate strings from regular byte arrays, new logic has
been added to variantToString() that wraps the null-byte
strings in @String(). That way we can implement a fast-path
when converting back from CFData, that doesn't go via the
slow and lossy conversion via UTF8, and the resulting QVariant
will be of type QVariant::ByteArray. The reason for using
UTF-8 as the binary representation of the string is that
in the case of storing a QByteArray("@foo") we need to
still be able to convert it back to the same byte array,
which doesn't work if the on-disk format is UTF-16.

Task-number: QTBUG-56124
Change-Id: Iab2f71cf96cf3225de48dc5e71870d74b6dde1e8
Cherry-picked: 764f5bf48cc87f4c72550b853ab93b815454cd48
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

4 files changed, 27 insertions, 16 deletions

diff --git a/src/corelib/io/qsettings.cpp b/src/corelib/io/qsettings.cpp
index 9e0e6c2769..d1ae14490c 100644
--- a/src/corelib/io/qsettings.cpp
+++ b/src/corelib/io/qsettings.cpp
@@ -413,7 +413,9 @@ QString QSettingsPrivate::variantToString(const QVariant &v)
         case QVariant::Double:
         case QVariant::KeySequence: {
             result = v.toString();
-            if (result.startsWith(QLatin1Char('@')))
+            if (result.contains(QChar::Null))
+                result = QLatin1String("@String(") + result + QLatin1Char(')');
+            else if (result.startsWith(QLatin1Char('@')))
                 result.prepend(QLatin1Char('@'));
             break;
         }
@@ -489,6 +491,8 @@ QVariant QSettingsPrivate::stringToVariant(const QString &s)
         if (s.endsWith(QLatin1Char(')'))) {
             if (s.startsWith(QLatin1String("@ByteArray("))) {
                 return QVariant(s.midRef(11, s.size() - 12).toLatin1());
+            } else if (s.startsWith(QLatin1String("@String("))) {
+                return QVariant(s.midRef(8, s.size() - 9).toString());
             } else if (s.startsWith(QLatin1String("@Variant("))
                        || s.startsWith(QLatin1String("@DateTime("))) {
 #ifndef QT_NO_DATASTREAM
diff --git a/src/corelib/io/qsettings_mac.cpp b/src/corelib/io/qsettings_mac.cpp
index e0b317b4c2..0b039f73dc 100644
--- a/src/corelib/io/qsettings_mac.cpp
+++ b/src/corelib/io/qsettings_mac.cpp
@@ -208,7 +208,14 @@ static QCFType<CFPropertyListRef> macValue(const QVariant &value)
     case QVariant::String:
     string_case:
     default:
-        result = QCFString::toCFStringRef(QSettingsPrivate::variantToString(value));
+        QString string = QSettingsPrivate::variantToString(value);
+        if (string.contains(QChar::Null)) {
+            QByteArray ba = string.toUtf8();
+            result = CFDataCreate(kCFAllocatorDefault, reinterpret_cast<const UInt8 *>(ba.data()),
+                                  CFIndex(ba.size()));
+        } else {
+            result = QCFString::toCFStringRef(string);
+        }
     }
     return result;
 }
@@ -261,8 +268,17 @@ static QVariant qtValue(CFPropertyListRef cfvalue)
         return (bool)CFBooleanGetValue(static_cast<CFBooleanRef>(cfvalue));
     } else if (typeId == CFDataGetTypeID()) {
         CFDataRef cfdata = static_cast<CFDataRef>(cfvalue);
-        return QByteArray(reinterpret_cast<const char *>(CFDataGetBytePtr(cfdata)),
+        QByteArray byteArray = QByteArray(reinterpret_cast<const char *>(CFDataGetBytePtr(cfdata)),
                           CFDataGetLength(cfdata));
+
+        // Fast-path for QByteArray, so that we don't have to go
+        // though the expensive and lossy conversion via UTF-8.
+        if (!byteArray.startsWith('@'))
+            return byteArray;
+
+        const QString str = QString::fromUtf8(byteArray.constData(), byteArray.size());
+        return QSettingsPrivate::stringToVariant(str);
+
     } else if (typeId == CFDictionaryGetTypeID()) {
         CFDictionaryRef cfdict = static_cast<CFDictionaryRef>(cfvalue);
         CFTypeID arrayTypeId = CFArrayGetTypeID();
diff --git a/src/corelib/io/qsettings_win.cpp b/src/corelib/io/qsettings_win.cpp
index da0c4c3c14..f6ad182c37 100644
--- a/src/corelib/io/qsettings_win.cpp
+++ b/src/corelib/io/qsettings_win.cpp
@@ -649,15 +649,6 @@ void QWinSettingsPrivate::remove(const QString &uKey)
     }
 }
 
-static bool stringContainsNullChar(const QString &s)
-{
-    for (int i = 0; i < s.length(); ++i) {
-        if (s.at(i).unicode() == 0)
-            return true;
-    }
-    return false;
-}
-
 void QWinSettingsPrivate::set(const QString &uKey, const QVariant &value)
 {
     if (writeHandle() == 0) {
@@ -686,7 +677,7 @@ void QWinSettingsPrivate::set(const QString &uKey, const QVariant &value)
             QStringList l = variantListToStringList(value.toList());
             QStringList::const_iterator it = l.constBegin();
             for (; it != l.constEnd(); ++it) {
-                if ((*it).length() == 0 || stringContainsNullChar(*it)) {
+                if ((*it).length() == 0 || it->contains(QChar::Null)) {
                     type = REG_BINARY;
                     break;
                 }
@@ -730,7 +721,7 @@ void QWinSettingsPrivate::set(const QString &uKey, const QVariant &value)
             // If the string does not contain '\0', we can use REG_SZ, the native registry
             // string type. Otherwise we use REG_BINARY.
             QString s = variantToString(value);
-            type = stringContainsNullChar(s) ? REG_BINARY : REG_SZ;
+            type = s.contains(QChar::Null) ? REG_BINARY : REG_SZ;
             if (type == REG_BINARY) {
                 regValueBuff = QByteArray((const char*)s.utf16(), s.length() * 2);
             } else {
diff --git a/src/corelib/io/qsettings_winrt.cpp b/src/corelib/io/qsettings_winrt.cpp
index 85ef64cbd4..5ab1133ef3 100644
--- a/src/corelib/io/qsettings_winrt.cpp
+++ b/src/corelib/io/qsettings_winrt.cpp
@@ -396,7 +396,7 @@ void QWinRTSettingsPrivate::set(const QString &uKey, const QVariant &value)
         QStringList::const_iterator it = l.constBegin();
         bool containsNull = false;
         for (; it != l.constEnd(); ++it) {
-            if ((*it).length() == 0 || it->indexOf(QChar::Null) != -1) {
+            if ((*it).length() == 0 || it->contains(QChar::Null)) {
                 // We can only store as binary
                 containsNull = true;
                 break;
@@ -439,7 +439,7 @@ void QWinRTSettingsPrivate::set(const QString &uKey, const QVariant &value)
         break;
     default: {
         const QString s = variantToString(value);
-        if (s.indexOf(QChar::Null) != -1) {
+        if (s.contains(QChar::Null)) {
             hr = valueStatics->CreateUInt8Array(s.length() * 2, (BYTE*) s.utf16(), &val);
         } else {
             HStringReference ref((const wchar_t*)s.utf16(), s.size());