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

Fix UB in QTextStreamPrivate::putNumber()

Browse Source 
Negating a negative value may attempt to negate the minimal value for
the signed type, whose only set bit is its sign; this is UB. So don't
do that. Since the non-decimal branch of the code just prepends
locale.negativeSign(), and we're passing -1 as width parameter to the
QLocalePrivate formatters (so there's no zero-padding, whose size
would be reduced to make space for a sign), we can treat decimal the
same as all other bases. This, furthermore, simplifies the code.

In the process, I noticed (because a test only done for decimal
failed) that if QTextStream::ForceSign is set, this code-path for
negatives would prepend the minus sign before a plus sign supplied by
QLocaleData. Skip the flag to include sign, for negative input, since
we'll be including a negative sign anyway. Remove the QEXPECT_FAIL()
from the test I've added for this in a preparatory commit.

Purge one now-redundant comment and revise some others. Expand on why
we need to hack octal zero to display two zeros. Add a second test row
to keep the lonely zero-row company in the test for that.

Fixes: QTBUG-133269
Pick-to: 6.9 6.8 6.5 5.15
Change-Id: I35c9bdf34b812cff578de9b0a2570a60e6145c80
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Edward Welbourne 2025-02-04 12:09:12 +01:00 committed by Marc Mutz
parent 82ae262de2
commit 44ddb34da0
2 changed files with 13 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
src/corelib/serialization/qtextstream.cpp
View File

@ -2206,44 +2206,34 @@ QTextStream &QTextStream::operator>>(char *c)
*/ */
void QTextStreamPrivate::putNumber(qulonglong number, bool negative) void QTextStreamPrivate::putNumber(qulonglong number, bool negative)
{ {
QString result;
unsigned flags = 0; unsigned flags = 0;
const QTextStream::NumberFlags numberFlags = params.numberFlags; const QTextStream::NumberFlags numberFlags = params.numberFlags;
if (numberFlags & QTextStream::ShowBase) if (numberFlags & QTextStream::ShowBase)
flags |= QLocaleData::ShowBase; flags |= QLocaleData::ShowBase;
if (numberFlags & QTextStream::ForceSign) // ForceSign is irrelevant when we'll be including a sign in any case:
if ((numberFlags & QTextStream::ForceSign) && !negative)
flags |= QLocaleData::AlwaysShowSign; flags |= QLocaleData::AlwaysShowSign;
if (numberFlags & QTextStream::UppercaseBase) if (numberFlags & QTextStream::UppercaseBase)
flags |= QLocaleData::UppercaseBase; flags |= QLocaleData::UppercaseBase;
if (numberFlags & QTextStream::UppercaseDigits) if (numberFlags & QTextStream::UppercaseDigits)
flags |= QLocaleData::CapitalEorX; flags |= QLocaleData::CapitalEorX;
// add thousands group separators. For backward compatibility we // Group digits. For backward compatibility, we skip this for the C locale.
// don't add a group separator for C locale.
if (locale != QLocale::c() && !locale.numberOptions().testFlag(QLocale::OmitGroupSeparator)) if (locale != QLocale::c() && !locale.numberOptions().testFlag(QLocale::OmitGroupSeparator))
flags |= QLocaleData::GroupDigits; flags |= QLocaleData::GroupDigits;
const QLocaleData *dd = locale.d->m_data; const QLocaleData *dd = locale.d->m_data;
int base = params.integerBase ? params.integerBase : 10; int base = params.integerBase ? params.integerBase : 10;
if (negative && base == 10) { QString result = dd->unsLongLongToString(number, -1, base, -1, flags);
result = dd->longLongToString(-static_cast<qlonglong>(number), -1, if (negative) {
base, -1, flags);
} else if (negative) {
// Workaround for backward compatibility for writing negative
// numbers in octal and hex:
// QTextStream(result) << Qt::showbase << Qt::hex << -1 << oct << -1
// should output: -0x1 -0b1
result = dd->unsLongLongToString(number, -1, base, -1, flags);
result.prepend(locale.negativeSign()); result.prepend(locale.negativeSign());
} else { } else if (number == 0 && base == 8 && params.numberFlags & QTextStream::ShowBase
result = dd->unsLongLongToString(number, -1, base, -1, flags); && result == "0"_L1) {
// workaround for backward compatibility - in octal form with // Workaround for backward compatibility - in octal form with ShowBase
// ShowBase flag set zero should be written as '00' // flag set, zero should get its 0 prefix before its 0 value, but
if (number == 0 && base == 8 && params.numberFlags & QTextStream::ShowBase // QLocalePrivate only adds the prefix if the number doesn't start with
&& result == "0"_L1) { // a zero.
result.prepend(u'0'); result.prepend(u'0');
}
} }
putString(result, true); putString(result, true);
} }

2
tests/auto/corelib/serialization/qtextstream/tst_qtextstream.cpp
View File

@ -1125,6 +1125,7 @@ void tst_QTextStream::octTest_data()
QTest::addColumn<QByteArray>("data"); QTest::addColumn<QByteArray>("data");
QTest::newRow("0") << 0 << QByteArray("00"); QTest::newRow("0") << 0 << QByteArray("00");
QTest::newRow("40") << 40 << QByteArray("050");
} }
// ------------------------------------------------------------------------------ // ------------------------------------------------------------------------------
@ -2775,7 +2776,6 @@ void tst_QTextStream::manipulators()
QVERIFY(stream << textData); QVERIFY(stream << textData);
stream.flush(); stream.flush();
QEXPECT_FAIL("hex-negative", "Discovered while fixing QTBUG-133269", Continue);
QCOMPARE(buffer.data(), result); QCOMPARE(buffer.data(), result);
} }