Describe the bug
ovms::stou32() accepts a string that is only partially a number, unlike every other converter in src/stringutils.cpp.
src/stringutils.cpp:112-129:
std::optional<uint32_t> stou32(const std::string& input) {
std::string str = input;
ovms::erase_spaces(str);
if (!str.empty() && str[0] == '-') {
return std::nullopt;
}
try {
uint64_t val = std::stoul(str); // no idx, no full-consumption check
if (val > std::numeric_limits<uint32_t>::max()) {
return std::nullopt;
}
return {static_cast<uint32_t>(val)};
} catch (...) {
return std::nullopt;
}
}
std::stoul stops at the first character it cannot use and still reports success. Its siblings in the same file all guard against that by passing &idx and comparing against the length:
stou64 — src/stringutils.cpp:143-147
stoi32 — src/stringutils.cpp:161-165
stof — src/stringutils.cpp:203-206
(stoi64 at :181-185 validates by scanning digits first, which is equivalent.) stou32 is the only one without the check, so:
12abc -> 12
3.9 -> 3
0x10 -> 0
100% -> 100
Call sites affected
-
src/grpcservermodule.cpp:79 — the GRPC_SERVERS environment variable:
const char* environmentVariableBuffer = std::getenv("GRPC_SERVERS");
if (environmentVariableBuffer) {
auto result = stou32(environmentVariableBuffer);
if (result && result.value() > 0) {
return result.value();
}
}
GRPC_SERVERS=4x silently starts 4 gRPC servers instead of being rejected and falling back to config.grpcWorkers(). A typo in a deployment environment variable is applied rather than reported.
-
src/audio/speech_to_text/s2t_servable.cpp:81 — the transcription temperature field, used as a fallback after ovms::stof has already rejected the value. That endpoint's user-visible symptom is filed separately; either fix resolves it independently and the two do not conflict.
To Reproduce
Unit level, alongside the existing StringUtils.stou32 case:
EXPECT_FALSE(ovms::stou32("12abc")); // currently returns 12
EXPECT_FALSE(ovms::stou32("3.9")); // currently returns 3
EXPECT_FALSE(ovms::stou32("0x10")); // currently returns 0
End to end:
GRPC_SERVERS=4x ovms --model_path /models/... --model_name m --port 9000
# starts 4 gRPC servers; expected: the value is rejected and grpcWorkers() is used
Expected behavior
stou32 behaves like stou64/stoi32/stof: a string that is not entirely a number returns std::nullopt.
Logs
None — the value is accepted, so nothing is logged.
Configuration
- OVMS version:
main @ fadb3314
- Any configuration; reachable via the
GRPC_SERVERS environment variable
- CPU
- N/A
- N/A
Additional context
Suggested fix, matching the siblings:
size_t idx = 0;
uint64_t val = std::stoul(str, &idx);
if (idx != str.size()) {
return std::nullopt;
}
The existing StringUtils.stou32 test covers only a negative value, overflow and the maximum, so none of it changes.
Separate, deliberately not bundled: stou32 calls erase_spaces() on its input first, so stou32("12 34") yields 1234 both before and after such a fix, whereas stou64 rejects " 100 " outright (there is an explicit test for that at src/test/stringutils_test.cpp:218). Whether stou32 should also stop erasing interior whitespace is a separate behavioural question and worth deciding on its own.
Also same class, different function, mentioned only so it is on the record: src/grpcservermodule.cpp:143 uses a bare std::stoi(value) to decide whether a gRPC channel argument is an integer, so grpc.max_receive_message_length=4MB is passed to gRPC as the integer 4 rather than as the string it is. That may well be intended, given the surrounding comment, so I have not touched it.
I have a patch for this and will open a PR shortly.
Describe the bug
ovms::stou32()accepts a string that is only partially a number, unlike every other converter insrc/stringutils.cpp.src/stringutils.cpp:112-129:std::stoulstops at the first character it cannot use and still reports success. Its siblings in the same file all guard against that by passing&idxand comparing against the length:stou64—src/stringutils.cpp:143-147stoi32—src/stringutils.cpp:161-165stof—src/stringutils.cpp:203-206(
stoi64at:181-185validates by scanning digits first, which is equivalent.)stou32is the only one without the check, so:Call sites affected
src/grpcservermodule.cpp:79— theGRPC_SERVERSenvironment variable:GRPC_SERVERS=4xsilently starts 4 gRPC servers instead of being rejected and falling back toconfig.grpcWorkers(). A typo in a deployment environment variable is applied rather than reported.src/audio/speech_to_text/s2t_servable.cpp:81— the transcriptiontemperaturefield, used as a fallback afterovms::stofhas already rejected the value. That endpoint's user-visible symptom is filed separately; either fix resolves it independently and the two do not conflict.To Reproduce
Unit level, alongside the existing
StringUtils.stou32case:End to end:
Expected behavior
stou32behaves likestou64/stoi32/stof: a string that is not entirely a number returnsstd::nullopt.Logs
None — the value is accepted, so nothing is logged.
Configuration
main@fadb3314GRPC_SERVERSenvironment variableAdditional context
Suggested fix, matching the siblings:
The existing
StringUtils.stou32test covers only a negative value, overflow and the maximum, so none of it changes.Separate, deliberately not bundled:
stou32callserase_spaces()on its input first, sostou32("12 34")yields1234both before and after such a fix, whereasstou64rejects" 100 "outright (there is an explicit test for that atsrc/test/stringutils_test.cpp:218). Whetherstou32should also stop erasing interior whitespace is a separate behavioural question and worth deciding on its own.Also same class, different function, mentioned only so it is on the record:
src/grpcservermodule.cpp:143uses a barestd::stoi(value)to decide whether a gRPC channel argument is an integer, sogrpc.max_receive_message_length=4MBis passed to gRPC as the integer4rather than as the string it is. That may well be intended, given the surrounding comment, so I have not touched it.I have a patch for this and will open a PR shortly.