diff options
author | Tor Arne Vestbø <tor.arne.vestbo@qt.io> | 2016-10-03 18:41:30 +0200 |
---|---|---|
committer | Jani Heikkinen <jani.heikkinen@qt.io> | 2016-10-21 09:30:43 +0000 |
commit | ab0ba668642d2d4c941f57e949118327530234af (patch) | |
tree | aea76950f326b6dfb5b6eb4e662a3d5b4e2fe61c | |
parent | 50b9d8a931e859c027726a74ea0616e7b292e782 (diff) |
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>
Reviewed-by: Jani Heikkinen <jani.heikkinen@qt.io>
-rw-r--r-- | src/corelib/io/qsettings.cpp | 6 | ||||
-rw-r--r-- | src/corelib/io/qsettings_mac.cpp | 20 | ||||
-rw-r--r-- | src/corelib/io/qsettings_win.cpp | 13 | ||||
-rw-r--r-- | src/corelib/io/qsettings_winrt.cpp | 4 | ||||
-rw-r--r-- | tests/auto/corelib/io/qsettings/tst_qsettings.cpp | 50 |
5 files changed, 76 insertions, 17 deletions
diff --git a/src/corelib/io/qsettings.cpp b/src/corelib/io/qsettings.cpp index 8bdd148e28..05f6c22fc2 100644 --- a/src/corelib/io/qsettings.cpp +++ b/src/corelib/io/qsettings.cpp @@ -417,7 +417,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; } @@ -477,6 +479,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 d73cc4d298..4282fcbe2c 100644 --- a/src/corelib/io/qsettings_mac.cpp +++ b/src/corelib/io/qsettings_mac.cpp @@ -214,7 +214,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; } @@ -267,8 +274,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 05ed51e999..53e1ed360b 100644 --- a/src/corelib/io/qsettings_win.cpp +++ b/src/corelib/io/qsettings_win.cpp @@ -675,15 +675,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) { @@ -712,7 +703,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; } @@ -756,7 +747,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 708287ce5e..209b56d920 100644 --- a/src/corelib/io/qsettings_winrt.cpp +++ b/src/corelib/io/qsettings_winrt.cpp @@ -402,7 +402,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; @@ -445,7 +445,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()); diff --git a/tests/auto/corelib/io/qsettings/tst_qsettings.cpp b/tests/auto/corelib/io/qsettings/tst_qsettings.cpp index 87a801c9dd..ed298bfafa 100644 --- a/tests/auto/corelib/io/qsettings/tst_qsettings.cpp +++ b/tests/auto/corelib/io/qsettings/tst_qsettings.cpp @@ -158,6 +158,8 @@ private slots: void testByteArray(); void iniCodec(); void bom(); + void embeddedZeroByte_data(); + void embeddedZeroByte(); private: void cleanupTestFiles(); @@ -627,7 +629,6 @@ void tst_QSettings::testByteArray_data() #ifndef QT_NO_COMPRESS QTest::newRow("compressed") << qCompress(bytes); #endif - QTest::newRow("with \\0") << bytes + '\0' + bytes; } void tst_QSettings::testByteArray() @@ -678,6 +679,53 @@ void tst_QSettings::bom() QVERIFY(allkeys.contains("section2/foo2")); } +void tst_QSettings::embeddedZeroByte_data() +{ + QTest::addColumn<QVariant>("value"); + + QByteArray bytes("hello\0world", 11); + + QTest::newRow("bytearray\\0") << QVariant(bytes); + QTest::newRow("string\\0") << QVariant(QString::fromLatin1(bytes.data(), bytes.size())); + + bytes = QByteArray("@String("); + + QTest::newRow("@bytearray") << QVariant(bytes); + QTest::newRow("@string") << QVariant(QString(bytes)); + + bytes = QByteArray("@String(\0test", 13); + + QTest::newRow("@bytearray\\0") << QVariant(bytes); + QTest::newRow("@string\\0") << QVariant(QString::fromLatin1(bytes.data(), bytes.size())); +} + +void tst_QSettings::embeddedZeroByte() +{ + QFETCH(QVariant, value); + { + QSettings settings("QtProject", "tst_qsettings"); + settings.setValue(QTest::currentDataTag(), value); + } + { + QSettings settings("QtProject", "tst_qsettings"); + QVariant outValue = settings.value(QTest::currentDataTag()); + + switch (value.type()) { + case QVariant::ByteArray: + QCOMPARE(outValue.toByteArray(), value.toByteArray()); + break; + case QVariant::String: + QCOMPARE(outValue.toString(), value.toString()); + break; + default: + Q_UNREACHABLE(); + } + + if (value.toByteArray().contains(QChar::Null)) + QVERIFY(outValue.toByteArray().contains(QChar::Null)); + } +} + void tst_QSettings::testErrorHandling_data() { QTest::addColumn<int>("filePerms"); // -1 means file should not exist |