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 Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
parent
483aff406a
commit
764f5bf48c
@ -416,7 +416,9 @@ QString QSettingsPrivate::variantToString(const QVariant &v)
|
|||||||
case QVariant::Double:
|
case QVariant::Double:
|
||||||
case QVariant::KeySequence: {
|
case QVariant::KeySequence: {
|
||||||
result = v.toString();
|
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('@'));
|
result.prepend(QLatin1Char('@'));
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
@ -476,6 +478,8 @@ QVariant QSettingsPrivate::stringToVariant(const QString &s)
|
|||||||
if (s.endsWith(QLatin1Char(')'))) {
|
if (s.endsWith(QLatin1Char(')'))) {
|
||||||
if (s.startsWith(QLatin1String("@ByteArray("))) {
|
if (s.startsWith(QLatin1String("@ByteArray("))) {
|
||||||
return QVariant(s.midRef(11, s.size() - 12).toLatin1());
|
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("))
|
} else if (s.startsWith(QLatin1String("@Variant("))
|
||||||
|| s.startsWith(QLatin1String("@DateTime("))) {
|
|| s.startsWith(QLatin1String("@DateTime("))) {
|
||||||
#ifndef QT_NO_DATASTREAM
|
#ifndef QT_NO_DATASTREAM
|
||||||
|
@ -214,7 +214,11 @@ static QCFType<CFPropertyListRef> macValue(const QVariant &value)
|
|||||||
case QVariant::String:
|
case QVariant::String:
|
||||||
string_case:
|
string_case:
|
||||||
default:
|
default:
|
||||||
result = QCFString::toCFStringRef(QSettingsPrivate::variantToString(value));
|
QString string = QSettingsPrivate::variantToString(value);
|
||||||
|
if (string.contains(QChar::Null))
|
||||||
|
result = string.toUtf8().toCFData();
|
||||||
|
else
|
||||||
|
result = string.toCFString();
|
||||||
}
|
}
|
||||||
return result;
|
return result;
|
||||||
}
|
}
|
||||||
@ -266,9 +270,16 @@ static QVariant qtValue(CFPropertyListRef cfvalue)
|
|||||||
} else if (typeId == CFBooleanGetTypeID()) {
|
} else if (typeId == CFBooleanGetTypeID()) {
|
||||||
return (bool)CFBooleanGetValue(static_cast<CFBooleanRef>(cfvalue));
|
return (bool)CFBooleanGetValue(static_cast<CFBooleanRef>(cfvalue));
|
||||||
} else if (typeId == CFDataGetTypeID()) {
|
} else if (typeId == CFDataGetTypeID()) {
|
||||||
CFDataRef cfdata = static_cast<CFDataRef>(cfvalue);
|
QByteArray byteArray = QByteArray::fromRawCFData(static_cast<CFDataRef>(cfvalue));
|
||||||
return 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()) {
|
} else if (typeId == CFDictionaryGetTypeID()) {
|
||||||
CFDictionaryRef cfdict = static_cast<CFDictionaryRef>(cfvalue);
|
CFDictionaryRef cfdict = static_cast<CFDictionaryRef>(cfvalue);
|
||||||
CFTypeID arrayTypeId = CFArrayGetTypeID();
|
CFTypeID arrayTypeId = CFArrayGetTypeID();
|
||||||
|
@ -667,15 +667,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)
|
void QWinSettingsPrivate::set(const QString &uKey, const QVariant &value)
|
||||||
{
|
{
|
||||||
if (writeHandle() == 0) {
|
if (writeHandle() == 0) {
|
||||||
@ -704,7 +695,7 @@ void QWinSettingsPrivate::set(const QString &uKey, const QVariant &value)
|
|||||||
QStringList l = variantListToStringList(value.toList());
|
QStringList l = variantListToStringList(value.toList());
|
||||||
QStringList::const_iterator it = l.constBegin();
|
QStringList::const_iterator it = l.constBegin();
|
||||||
for (; it != l.constEnd(); ++it) {
|
for (; it != l.constEnd(); ++it) {
|
||||||
if ((*it).length() == 0 || stringContainsNullChar(*it)) {
|
if ((*it).length() == 0 || it->contains(QChar::Null)) {
|
||||||
type = REG_BINARY;
|
type = REG_BINARY;
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
@ -748,7 +739,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
|
// If the string does not contain '\0', we can use REG_SZ, the native registry
|
||||||
// string type. Otherwise we use REG_BINARY.
|
// string type. Otherwise we use REG_BINARY.
|
||||||
QString s = variantToString(value);
|
QString s = variantToString(value);
|
||||||
type = stringContainsNullChar(s) ? REG_BINARY : REG_SZ;
|
type = s.contains(QChar::Null) ? REG_BINARY : REG_SZ;
|
||||||
if (type == REG_BINARY) {
|
if (type == REG_BINARY) {
|
||||||
regValueBuff = QByteArray((const char*)s.utf16(), s.length() * 2);
|
regValueBuff = QByteArray((const char*)s.utf16(), s.length() * 2);
|
||||||
} else {
|
} else {
|
||||||
|
@ -402,7 +402,7 @@ void QWinRTSettingsPrivate::set(const QString &uKey, const QVariant &value)
|
|||||||
QStringList::const_iterator it = l.constBegin();
|
QStringList::const_iterator it = l.constBegin();
|
||||||
bool containsNull = false;
|
bool containsNull = false;
|
||||||
for (; it != l.constEnd(); ++it) {
|
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
|
// We can only store as binary
|
||||||
containsNull = true;
|
containsNull = true;
|
||||||
break;
|
break;
|
||||||
@ -445,7 +445,7 @@ void QWinRTSettingsPrivate::set(const QString &uKey, const QVariant &value)
|
|||||||
break;
|
break;
|
||||||
default: {
|
default: {
|
||||||
const QString s = variantToString(value);
|
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);
|
hr = valueStatics->CreateUInt8Array(s.length() * 2, (BYTE*) s.utf16(), &val);
|
||||||
} else {
|
} else {
|
||||||
HStringReference ref((const wchar_t*)s.utf16(), s.size());
|
HStringReference ref((const wchar_t*)s.utf16(), s.size());
|
||||||
|
@ -174,6 +174,8 @@ private slots:
|
|||||||
void testByteArray();
|
void testByteArray();
|
||||||
void iniCodec();
|
void iniCodec();
|
||||||
void bom();
|
void bom();
|
||||||
|
void embeddedZeroByte_data();
|
||||||
|
void embeddedZeroByte();
|
||||||
|
|
||||||
private:
|
private:
|
||||||
void cleanupTestFiles();
|
void cleanupTestFiles();
|
||||||
@ -643,7 +645,6 @@ void tst_QSettings::testByteArray_data()
|
|||||||
#ifndef QT_NO_COMPRESS
|
#ifndef QT_NO_COMPRESS
|
||||||
QTest::newRow("compressed") << qCompress(bytes);
|
QTest::newRow("compressed") << qCompress(bytes);
|
||||||
#endif
|
#endif
|
||||||
QTest::newRow("with \\0") << bytes + '\0' + bytes;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
void tst_QSettings::testByteArray()
|
void tst_QSettings::testByteArray()
|
||||||
@ -694,6 +695,53 @@ void tst_QSettings::bom()
|
|||||||
QVERIFY(allkeys.contains("section2/foo2"));
|
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()
|
void tst_QSettings::testErrorHandling_data()
|
||||||
{
|
{
|
||||||
QTest::addColumn<int>("filePerms"); // -1 means file should not exist
|
QTest::addColumn<int>("filePerms"); // -1 means file should not exist
|
||||||
|
Loading…
x
Reference in New Issue
Block a user