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

Fix parsing of most significant group of digits

Browse Source 
The first-group minimum size limit is only relevant when there's
*only* one separator: a smaller group is allowed when there is a later
separator. QLocaleData::numberToCLocale() was, however, rejecting a
short first group without regard to what came later. Expanded two
grouping tests, that previously checked we *generate* correct strings,
to round-trip the strings and verify we get the right answer back.
Some of those duly failed. Fixed the parsing to check the right
condition on seeing a group separator.

That let valid groups get accepted. In the process, added some
test-cases that we do correctly reject invalid forms. Made sure the
fix also gets those right.

Pick-to: 6.9 6.8 6.5
Task-number: QTBUG-134913
Change-Id: Ifa5351adba4eec232917a343d4a73fb4656d1d32
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
This commit is contained in:
Edward Welbourne 2025-03-19 09:48:47 +01:00
parent f1d48552e4
commit 754666dbe8
2 changed files with 65 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/corelib/text/qlocale.cpp
View File

@ -4625,6 +4625,7 @@ bool QLocaleData::numberToCLocale(QStringView s, QLocale::NumberOptions number_o
bool wantDigits = true;
// Digit-grouping details (all modes):
bool needHigherGroup = false; // Set when first group is too short to be the only one
qsizetype digitsInGroup = 0;
const QLocaleData::GroupSizes grouping = groupSizes();
const auto badLeastGroup = [&]() {
@ -4635,6 +4636,9 @@ bool QLocaleData::numberToCLocale(QStringView s, QLocale::NumberOptions number_o
// so objecting now would break existing code.
if (stage == Grouped) {
Q_ASSERT(!number_options.testFlag(QLocale::RejectGroupSeparator));
// First group was invalid if it was short and we've not seen a separator since:
if (needHigherGroup)
return true;
// Were there enough digits since the last group separator?
if (digitsInGroup != grouping.least)
return true;
@ -4680,16 +4684,23 @@ bool QLocaleData::numberToCLocale(QStringView s, QLocale::NumberOptions number_o
switch (stage) {
case Whole:
// Check size of most significant group
if (grouping.first > digitsInGroup
|| digitsInGroup >= grouping.least + grouping.first) {
if (digitsInGroup == 0
|| digitsInGroup > qMax(grouping.first, grouping.higher)) {
return false;
}
Q_ASSERT(!needHigherGroup);
// First group is only allowed fewer than grouping.first digits
// if it's followed by a grouping.higher group, i.e. there's a
// later group separator:
if (grouping.first > digitsInGroup)
needHigherGroup = true;
stage = Grouped;
break;
case Grouped:
// Check size of group between two separators:
if (digitsInGroup != grouping.higher)
return false;
needHigherGroup = false; // We just found it, if needed.
break;
// Only allow group chars within the whole-number part:
case Fraction:

55
tests/auto/corelib/text/qlocale/tst_qlocale.cpp
View File

@ -4662,8 +4662,10 @@ void tst_QLocale::systemGrouping_data()
// that return invalid group sizes to test that we fallbakc to CLDR data.
QTest::newRow("en-ES") // {2,3,3}
<< u"en-ES"_s << u","_s << u"0"_s << 1234 << u"1234"_s;
QTest::newRow("en-ES-grouped") // {2,3,3}
QTest::newRow("en-ES-grouped")
<< u"en-ES"_s << u","_s << u"0"_s << 12345 << u"12,345"_s;
QTest::newRow("en-ES-long")
<< u"en-ES"_s << u","_s << u"0"_s << 1234567 << u"1,234,567"_s;
QTest::newRow("en-BD") // {1,2,3}
<< u"en-BD"_s << u","_s << u"0"_s << 123456789 << u"12,34,56,789"_s;
QTest::newRow("en-BT") // {1,2,3}
@ -4680,6 +4682,7 @@ void tst_QLocale::systemGrouping_data()
QChar::highSurrogate(zeroVal + 3), QChar::lowSurrogate(zeroVal + 3),
QChar::highSurrogate(zeroVal + 4), QChar::lowSurrogate(zeroVal + 4),
QChar::highSurrogate(zeroVal + 5), QChar::lowSurrogate(zeroVal + 5),
QChar::highSurrogate(zeroVal + 6), QChar::lowSurrogate(zeroVal + 6),
};
const QChar separator(QLatin1Char(',')); // Separator for the Chakma locale
const QString
@ -4689,16 +4692,22 @@ void tst_QLocale::systemGrouping_data()
two = QString::fromRawData(data + 4, 2),
three = QString::fromRawData(data + 6, 2),
four = QString::fromRawData(data + 8, 2),
five = QString::fromRawData(data + 10, 2);
five = QString::fromRawData(data + 10, 2),
six = QString::fromRawData(data + 12, 2);
QString fourDigit = one + two + three + four;
QString fiveDigit = one + two + separator + three + four + five;
// Leading group can have single digit as long as there's a later separator:
QString sixDigit = one + separator + two + three + separator + four + five + six;
QTest::newRow("Chakma-short") // {2,2,3}
<< u"ccp"_s << QString(separator)
<< zero << 1234 << fourDigit;
QTest::newRow("Chakma") // {2,2,3}
QTest::newRow("Chakma")
<< u"ccp"_s << QString(separator)
<< zero << 12345 << fiveDigit;
QTest::newRow("Chakma-long")
<< u"ccp"_s << QString(separator) << zero
<< 123456 << sixDigit;
}
void tst_QLocale::systemGrouping()
@ -4714,7 +4723,12 @@ void tst_QLocale::systemGrouping()
QLocale sys = QLocale::system();
QCOMPARE(sys.groupSeparator(), separator);
QCOMPARE(sys.zeroDigit(), zeroDigit);
QCOMPARE(sys.toString(number), formattedString);
bool ok;
int count = sys.toInt(formattedString, &ok);
QVERIFY2(ok, "Integer didn't round-trip");
QCOMPARE(count, number);
}
}
@ -4945,6 +4959,11 @@ void tst_QLocale::numberGrouping()
QCOMPARE(u"%L1"_s.arg(double(number), 0, 'f', 0), string);
}
}
// Check round-trip via toInt():
bool ok;
int actual = locale.toInt(string, &ok);
QVERIFY(ok);
QCOMPARE(actual, number);
}
void tst_QLocale::numberGroupingIndia()
@ -4991,6 +5010,13 @@ void tst_QLocale::numberGroupingIndia()
QCOMPARE(indian.toString(uInteger32), strResult32);
QCOMPARE(indian.toUInt(strResult32), uInteger32);
bool ok = false;
QCOMPARE(indian.toInt(u"1,23,45,678"_s, &ok), 12345678);
QVERIFY(ok);
// Malformed (bad grouping):
QCOMPARE(indian.toInt(u"123,45,678"_s, &ok), 0);
QVERIFY(!ok);
// 63-bit:
const QString strResult64("60,05,00,40,03,00,20,01,000");
const qint64 int64 = Q_INT64_C(6005004003002001000);
@ -5063,6 +5089,29 @@ void tst_QLocale::numberFormatChakma()
QCOMPARE(chakma.toString(integer32), strResult32);
QCOMPARE(chakma.toInt(strResult32), integer32);
bool ok = false; // Lakh is the Hindi name for 1,00,000
const QString goodLakh = one + separator + two + three + separator + four + five + six;
QCOMPARE(chakma.toInt(goodLakh, &ok), 123456);
QVERIFY(ok);
const QString longThousand = one + two + three + separator + four + five + six;
QCOMPARE(chakma.toInt(longThousand, &ok), 0);
QVERIFY(!ok);
const QString goodCrore // Crore is Hindi for 1,00,00,000
= one + separator + two + three + separator + zero + zero + separator + four + five + six;
QCOMPARE(chakma.toInt(goodCrore, &ok), 12300456);
QVERIFY(ok);
// Officially should be grouped, but we tolerate a complete lack of grouping
// for backwards-compatibility reasons.
const QString badLakh
= one + two + three + four + five + six;
QCOMPARE(chakma.toInt(badLakh, &ok), 123456);
QVERIFY(ok);
// However, even one group separator requires all to be correctly placed:
const QString longLakh
= one + two + three + separator + zero + zero + separator + four + five + six;
QCOMPARE(chakma.toInt(longLakh, &ok), 0);
QVERIFY(!ok);
const uint uInteger32 = 2030405010u;
QCOMPARE(chakma.toString(uInteger32), strResult32);
QCOMPARE(chakma.toUInt(strResult32), uInteger32);