From ff7675817d9e31c261ddaca81ca7f58a362d8295 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Mon, 10 Feb 2025 22:45:32 -0800 Subject: [PATCH] qEnvironmentVariableIntValue: fix off-by-one with MSVC's getenv_s This meant a string containing the octal form of INT_MIN (-020000000000) would be just too long and getenv_s() would fail. This was never caught because code that was meant to test different bases simply forgot to use the base. Amends commit bb56586e32677ee9be23bffa4f3cc9a913ef192f. I've renamed the rows to be the text being parsed, so it matches the previous rows and it makes clear what was being parsed just by reading the test's output. That also revealed a duplicate row to be removed. [ChangeLog][QtCore][QtEnvironment] Fixed a bug that caused qEnvironmentVariableIntValue() to fail to parse octal values from -020000000000 to -010000000000 with MSVC. Other compilers were not affected. Pick-to: 6.8 6.5 Change-Id: I9095d86cccd9e8001e85fffd6fbbcd6a9a1678c3 Reviewed-by: Marc Mutz (cherry picked from commit 83f2d1130aa49228a8a87547660791708735dd4b) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/global/qtenvironmentvariables.cpp | 8 +++-- .../global/qgetputenv/tst_qgetputenv.cpp | 30 ++++++++++++------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/corelib/global/qtenvironmentvariables.cpp b/src/corelib/global/qtenvironmentvariables.cpp index 58682a2406c..f564a523a57 100644 --- a/src/corelib/global/qtenvironmentvariables.cpp +++ b/src/corelib/global/qtenvironmentvariables.cpp @@ -195,13 +195,15 @@ int qEnvironmentVariableIntValue(const char *varName, bool *ok) noexcept { static const int NumBinaryDigitsPerOctalDigit = 3; static const int MaxDigitsForOctalInt = - (std::numeric_limits::digits + NumBinaryDigitsPerOctalDigit - 1) / NumBinaryDigitsPerOctalDigit; + (std::numeric_limits::digits + NumBinaryDigitsPerOctalDigit - 1) / NumBinaryDigitsPerOctalDigit + + 1 // sign + + 1; // "0" base prefix const auto locker = qt_scoped_lock(environmentMutex); size_t size; #ifdef Q_CC_MSVC // we provide a buffer that can hold any int value: - char buffer[MaxDigitsForOctalInt + 2]; // +1 for NUL +1 for optional '-' + char buffer[MaxDigitsForOctalInt + 1]; // +1 for the terminating null size_t dummy; if (getenv_s(&dummy, buffer, sizeof buffer, varName) != 0) { if (ok) @@ -211,7 +213,7 @@ int qEnvironmentVariableIntValue(const char *varName, bool *ok) noexcept size = strlen(buffer); #else const char * const buffer = ::getenv(varName); - if (!buffer || (size = strlen(buffer)) > MaxDigitsForOctalInt + 2) { + if (!buffer || (size = strlen(buffer)) > MaxDigitsForOctalInt) { if (ok) *ok = false; return 0; diff --git a/tests/auto/corelib/global/qgetputenv/tst_qgetputenv.cpp b/tests/auto/corelib/global/qgetputenv/tst_qgetputenv.cpp index 96f2ce853c5..9f9fc8f534e 100644 --- a/tests/auto/corelib/global/qgetputenv/tst_qgetputenv.cpp +++ b/tests/auto/corelib/global/qgetputenv/tst_qgetputenv.cpp @@ -163,19 +163,27 @@ void tst_QGetPutEnv::intValue_data() ROW(-1, -1, true); ROW(-010, -8, true); ROW(-000000000000000000000000000000000000000000000000001, 0, false); - ROW(2147483648, 0, false); // ROW(0xffffffff, -1, true); // could be expected, but not how QByteArray::toInt() works ROW(0xffffffff, 0, false); - const int bases[] = {10, 8, 16}; - for (size_t i = 0; i < sizeof bases / sizeof *bases; ++i) { - QTest::addRow("INT_MAX, base %d", bases[i]) - << QByteArray::number(INT_MAX) << INT_MAX << true; - QTest::addRow("INT_MAX+1, base %d", bases[i]) - << QByteArray::number(qlonglong(INT_MAX) + 1) << 0 << false; - QTest::addRow("INT_MIN, base %d", bases[i]) - << QByteArray::number(INT_MIN) << INT_MIN << true; - QTest::addRow("INT_MIN-1, base %d", bases[i]) - << QByteArray::number(qlonglong(INT_MIN) - 1) << 0 << false; + + auto addNumWithBase = [](qlonglong num, int base) { + QByteArray text; + { + QTextStream s(&text); + s.setIntegerBase(base); + s << Qt::showbase << num; + } + QTestData &row = QTest::addRow("%s", text.constData()) << text; + if (num == int(num)) + row << int(num) << true; + else + row << 0 << false; + }; + for (int base : {10, 8, 16}) { + addNumWithBase(INT_MAX, base); + addNumWithBase(qlonglong(INT_MAX) + 1, base); + addNumWithBase(INT_MIN, base); + addNumWithBase(qlonglong(INT_MIN) - 1 , base); }; }