Skip to content

range-check uint16_t in the Any string to number fallback - #1242

Open
aysha-afrah26 wants to merge 1 commit into
BehaviorTree:masterfrom
aysha-afrah26:any-uint16-stoul-range
Open

aysha-afrah26 wants to merge 1 commit into
BehaviorTree:masterfrom
aysha-afrah26:any-uint16-stoul-range

Conversation

@aysha-afrah26

Copy link
Copy Markdown
Contributor

Any::stringToNumber in safe_any.hpp has a fallback for standard libraries that don't define __cpp_lib_to_chars, which includes Apple libc++ and so the macOS builds. In that branch uint16_t was special-cased to return std::stoul(str), so the unsigned long result was narrowed to uint16_t implicitly with no range check, and stoul also accepts a leading minus sign. On macOS Any(std::string("70000")).cast<uint16_t>() returns 4464, "65536" returns 0 and "-1" returns 65535, and Blackboard::get<uint16_t> on a string entry (a value from XML, a script, or ImportBlackboardFromJSON) silently returns the same wrapped numbers. The std::from_chars branch used on Linux and Windows rejects all three, and every other integral type in the fallback already goes through the range-checked convert<T>(). I noticed it while comparing the two branches after the locale fix in #1218 and confirmed it with a small test on macOS (Apple clang 21). Removing the special case lets uint16_t take the same stol + convert<T>() path as the other integral types, so out-of-range and negative strings now fail the cast instead of wrapping, and valid values are unchanged. It's a header-only change with no API or ABI impact, and the new Any.StringToUint16IsRangeChecked test fails on macOS before the change and passes after (full ctest is green).

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant