1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

QString: fix append() wrt. raw data

Browse Source 
When appending to an empty string, we optimize and
copy the internal pointer. But if the other string was
created with fromRawData this might be temporary data on the stack/heap
and might be de-allocated or overwritten before the string
is used or is forced to make a deep-copy. This would lead to incorrect
data being used.

This is easy to overlook if you plan to append multiple strings
together, potentially supplied through an argument. Upon appending a
second string it would make a full copy, but there might not be a
guarantee for that. So, it's hard for users to avoid this pitfall!

Fixes: QTBUG-115752
Change-Id: Ia9aa5f463121c2ce2e0e8eee8a6c8612b7297f2b
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit 4660a230d527a9cffda41999103aba6ff5c2ecd5)
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
(cherry picked from commit d4a6c81081db24812a043c44d0d3d683b53e7d80)
Reviewed-by: Daniel Smith <Daniel.Smith@qt.io>
This commit is contained in:
Mårten Nordheim 2023-08-14 18:48:31 +02:00 committed by Daniel Smith
parent e8fb761f75
commit bb4e6d908f
3 changed files with 36 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/corelib/text/qstring.cpp
View File

@ -3131,7 +3131,10 @@ QString &QString::append(const QString &str)
{
if (!str.isNull()) {
if (isNull()) {
operator=(str);
if (Q_UNLIKELY(!str.d.isMutable()))
assign(str); // fromRawData, so we do a deep copy
else
operator=(str);
} else if (str.size()) {
append(str.constData(), str.size());
}

15
tests/auto/corelib/text/qbytearray/tst_qbytearray.cpp
View File

@ -51,6 +51,7 @@ private slots:
void prependExtended_data();
void prependExtended();
void append();
void appendFromRawData();
void appendExtended_data();
void appendExtended();
void assign();
@ -911,6 +912,20 @@ void tst_QByteArray::append()
}
}
void tst_QByteArray::appendFromRawData()
{
char rawData[] = "Hello World!";
QByteArray ba = QByteArray::fromRawData(rawData, std::size(rawData) - 1);
QByteArray copy;
copy.append(ba);
QCOMPARE(copy, ba);
// We make an _actual_ copy, because appending a byte array
// created with fromRawData() might be optimized to copy the DataPointer,
// which means we may point to temporary stack data.
QCOMPARE_NE((void *)copy.constData(), (void *)ba.constData());
}
void tst_QByteArray::appendExtended_data()
{
prependExtended_data();

17
tests/auto/corelib/text/qstring/tst_qstring.cpp
View File

@ -469,6 +469,8 @@ private slots:
void append_bytearray_special_cases();
#endif
void appendFromRawData();
void operator_pluseq_qstring() { operator_pluseq_impl<QString>(); }
void operator_pluseq_qstring_data() { operator_pluseq_data(); }
void operator_pluseq_qstringview() { operator_pluseq_impl<QStringView, QString &(QString::*)(QStringView)>(); }
@ -3394,6 +3396,21 @@ void tst_QString::append_bytearray_special_cases()
}
#endif // !defined(QT_RESTRICTED_CAST_FROM_ASCII) && !defined(QT_NO_CAST_FROM_ASCII)
void tst_QString::appendFromRawData()
{
const char16_t utf[] = u"Hello World!";
auto *rawData = reinterpret_cast<const QChar *>(utf);
QString str = QString::fromRawData(rawData, std::size(utf) - 1);
QString copy;
copy.append(str);
QCOMPARE(copy, str);
// We make an _actual_ copy, because appending a byte array
// created with fromRawData() might be optimized to copy the DataPointer,
// which means we may point to temporary stack data.
QCOMPARE_NE((void *)copy.constData(), (void *)str.constData());
}
void tst_QString::assign()
{
// QString &assign(QAnyStringView)