Copilot commented on code in PR #3376:
URL: https://github.com/apache/brpc/pull/3376#discussion_r3566262107
##########
src/brpc/uri.cpp:
##########
@@ -168,7 +168,7 @@ int URI::SetHttpURL(const char* url) {
bool need_scheme = true;
bool need_user_info = true;
for (; true; ++p) {
- const char action = g_url_parsing_fast_action_map[(int)*p];
+ const char action = g_url_parsing_fast_action_map[(signed char)*p];
if (action == URI_PARSE_CONTINUE) {
Review Comment:
This fix addresses a platform-specific OOB read that won’t be exercised by
the default x86_64 CI. To prevent regressions in the URL parsing lookup table
path, consider adding at least one CI build/test configuration that uses
`-funsigned-char` (or an aarch64/riscv64 build), since unit tests on
signed-`char` platforms generally won’t catch the original failure mode.
##########
src/butil/containers/case_ignored_flat_map.h:
##########
@@ -29,7 +29,11 @@ namespace butil {
// note: using char caused crashes on ubuntu 20.04 aarch64 (VM on apple M1)
inline char ascii_tolower(int/*note*/ c) {
extern const signed char* const g_tolower_map;
- return g_tolower_map[c];
+ // g_tolower_map is biased by +128 and sized for a signed-char index
+ // ([-128,127]). Callers pass a `char`; on platforms where `char` is
+ // unsigned (aarch64, riscv64) a byte >= 0x80 arrives here as 128..255 and
+ // indexes past the 256-entry table. Fold back into signed-char range.
+ return g_tolower_map[(signed char)c];
Review Comment:
The unit test coverage currently exercises ascii_tolower only for inputs in
[-128, 127]. This bug triggers when ascii_tolower receives values in [128, 255]
(e.g., when a `char` argument promotes to int on unsigned-`char` platforms), so
a regression could slip by again. Consider extending the existing
flat_map_unittest to cover all 256 byte values using a safe reference (e.g.,
::tolower(static_cast<unsigned char>(i))) and compare against ascii_tolower(i).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
