Skip to content

Commit bc08387

Browse files
etrclaude
andcommitted
CI: fix sanitizer link, Windows env, lint, and CodeQL snprintf finding
Four independent CI failure classes on the v2.0 matrix: - tsan/msan/lsan link error: http_request_unescape_arena_test.cpp defines global operator new/delete to count heap allocs; the tsan/msan/lsan runtimes ship their own strong definitions, so the test collided at link time (multiple definition). Guard the overrides out on those lanes via -DLHS_SANITIZER_OWNS_OPERATOR_NEW (set in verify-build.yml) plus __has_feature/__SANITIZE_THREAD__ fallbacks. With overrides absent the counter stays 0, so the zero-global-alloc pins still hold and the correctness/lifetime pins still exercise the real arena path. - Windows (mingw gcc-16): <stdlib.h> no longer declares POSIX setenv/unsetenv. Route the three debug_dump_request_body_* tests through _putenv on _WIN32 (VAR= removes the variable under MSVCRT). - cpplint gate (29 errors, surfaced now that the lint script runs): fixed runtime/int, indent_namespace, blank_line, header_guard, IWYU (real includes where legal, inline NOLINT in mid-class fragment headers), TODO username, and try-clause newline formatting. - CodeQL high-severity cpp/overflowing-snprintf in peer_address.cpp: the IPv6 canonicaliser advanced `pos` by snprintf's return value, which CodeQL traced into the next call's `sizeof(buf) - pos` size argument (potential size_t underflow, CWE-190). A group is <=4 hex digits so it never truncates with buf[40], but add the canonical bound guard so pos is provably in-range. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NpysYDDJac63yz2mZKKiDf
1 parent 36a25b0 commit bc08387

16 files changed

Lines changed: 111 additions & 31 deletions

.github/workflows/verify-build.yml

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -992,11 +992,14 @@ jobs:
992992
if [ "$BUILD_TYPE" = "msan" ]; then
993993
LIBCXX_MSAN="${{ github.workspace }}/libcxx-msan" ;
994994
export CFLAGS="-fsanitize=memory -fsanitize-memory-track-origins -fno-omit-frame-pointer -g -O1" ;
995-
export CXXFLAGS="-stdlib=libc++ -nostdinc++ -isystem ${LIBCXX_MSAN}/include/c++/v1 -fsanitize=memory -fsanitize-memory-track-origins -fno-omit-frame-pointer -g -O1" ;
995+
export CXXFLAGS="-stdlib=libc++ -nostdinc++ -isystem ${LIBCXX_MSAN}/include/c++/v1 -fsanitize=memory -fsanitize-memory-track-origins -fno-omit-frame-pointer -g -O1 -DLHS_SANITIZER_OWNS_OPERATOR_NEW" ;
996996
export LDFLAGS="-stdlib=libc++ -fsanitize=memory -L${LIBCXX_MSAN}/lib -Wl,-rpath,${LIBCXX_MSAN}/lib -lc++abi" ;
997997
fi
998-
if [ "$BUILD_TYPE" = "lsan" ]; then export CFLAGS='-fsanitize=leak'; export CXXFLAGS='-fsanitize=leak'; export LDFLAGS='-fsanitize=leak'; fi
999-
if [ "$BUILD_TYPE" = "tsan" ]; then export CFLAGS='-fsanitize=thread'; export CXXFLAGS='-fsanitize=thread'; export LDFLAGS='-fsanitize=thread'; fi
998+
# tsan/msan/lsan runtimes define their own global operator new/delete;
999+
# -DLHS_SANITIZER_OWNS_OPERATOR_NEW makes the arena alloc-pin test drop
1000+
# its overrides so it links (see test/unit/http_request_unescape_arena_test.cpp).
1001+
if [ "$BUILD_TYPE" = "lsan" ]; then export CFLAGS='-fsanitize=leak'; export CXXFLAGS='-fsanitize=leak -DLHS_SANITIZER_OWNS_OPERATOR_NEW'; export LDFLAGS='-fsanitize=leak'; fi
1002+
if [ "$BUILD_TYPE" = "tsan" ]; then export CFLAGS='-fsanitize=thread'; export CXXFLAGS='-fsanitize=thread -DLHS_SANITIZER_OWNS_OPERATOR_NEW'; export LDFLAGS='-fsanitize=thread'; fi
10001003
if [ "$BUILD_TYPE" = "ubsan" ]; then export CFLAGS='-fsanitize=undefined'; export CXXFLAGS='-fsanitize=undefined'; export LDFLAGS='-fsanitize=undefined'; fi
10011004
10021005
# Additional flags on mac. They need to stay in step as env variables don't propagate across steps.

src/detail/ip_representation.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ void ip_representation::parse_ipv4(const std::string& ip) {
141141
clear_bit(mask, static_cast<unsigned int>(12 + i));
142142
continue;
143143
}
144-
const long piece = strtol(parts[i].c_str(), nullptr, 10);
144+
const int64_t piece = strtol(parts[i].c_str(), nullptr, 10);
145145
if (piece < 0 || piece > 255) {
146146
throw std::invalid_argument("IP is badly formatted. 255 is max value for ip part.");
147147
}
@@ -206,7 +206,7 @@ void ip_representation::parse_nested_ipv4(const std::vector<std::string>& parts,
206206
clear_bit(mask, static_cast<unsigned int>(y + ii));
207207
continue;
208208
}
209-
const long subpart = strtol(subparts[ii].c_str(), nullptr, 10);
209+
const int64_t subpart = strtol(subparts[ii].c_str(), nullptr, 10);
210210
if (subpart < 0 || subpart > 255) {
211211
throw std::invalid_argument("IP is badly formatted. 255 is max value for ip part.");
212212
}
@@ -243,12 +243,12 @@ void ip_representation::apply_ipv6_part(std::vector<std::string>& parts, unsigne
243243
const std::string hi_str = part.substr(0, 2);
244244
const std::string lo_str = part.substr(2, 2);
245245
char* endp = nullptr;
246-
const long hi = strtol(hi_str.c_str(), &endp, 16);
246+
const int64_t hi = strtol(hi_str.c_str(), &endp, 16);
247247
if (*endp != '\0') {
248248
throw std::invalid_argument(
249249
"IP is badly formatted. IPV6 part contains a non-hex character.");
250250
}
251-
const long lo = strtol(lo_str.c_str(), &endp, 16);
251+
const int64_t lo = strtol(lo_str.c_str(), &endp, 16);
252252
if (*endp != '\0') {
253253
throw std::invalid_argument(
254254
"IP is badly formatted. IPV6 part contains a non-hex character.");

src/httpserver/detail/lambda_resource.hpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,8 +75,7 @@ namespace detail {
7575
// variant arm was dead code (no writer stored a lambda directly — every
7676
// on_*/route path funnels through lambda_resource), so it was removed.
7777
// The typedef relocates here, where its remaining uses are.
78-
using lambda_handler =
79-
std::function<::httpserver::http_response(const ::httpserver::http_request&)>;
78+
using lambda_handler = std::function<::httpserver::http_response(const ::httpserver::http_request&)>;
8079

8180
// Tiny adapter that wraps a single lambda_handler as an http_resource
8281
// virtual override. We keep one slot per method enum and dispatch in

src/httpserver/detail/webserver_impl_dispatch.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,7 @@ const detail::route_entry* find_v2_entry_by_path_(
362362
// Returns {shim, is_fresh}: is_fresh is true when a brand-new
363363
// lambda_resource shim was created (no entry previously existed at
364364
// this path), false when an existing shim was reused.
365-
std::pair<std::shared_ptr<detail::lambda_resource>, bool>
365+
std::pair<std::shared_ptr<detail::lambda_resource>, bool> // NOLINT(build/include_what_you_use)
366366
prepare_or_create_lambda_shim(const detail::http_endpoint& idx,
367367
method_set methods);
368368
void commit_handlers_to_shim(detail::lambda_resource& shim,

src/httpserver/http_request.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,7 @@ class http_request {
284284
// NOT part of the public ABI; only visible when the library itself
285285
// is being compiled (gated by HTTPSERVER_COMPILATION).
286286
MHD_Connection* underlying_connection_for_testing() const noexcept;
287+
287288
private:
288289
#endif // HTTPSERVER_COMPILATION
289290

src/httpserver/webserver_routes.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ template <typename T,
112112
std::is_base_of_v<http_resource, T>>>
113113
void register_prefix(const std::string& path, std::unique_ptr<T> res) {
114114
register_prefix(path,
115-
std::shared_ptr<http_resource>(std::move(res)));
115+
std::shared_ptr<http_resource>(std::move(res))); // NOLINT(build/include_what_you_use)
116116
}
117117
/**
118118
* Register a resource for a prefix URL match (shared_ptr overload).
@@ -125,7 +125,7 @@ void register_prefix(const std::string& path, std::unique_ptr<T> res) {
125125
* a reference.
126126
**/
127127
void register_prefix(const std::string& path,
128-
std::shared_ptr<http_resource> res);
128+
std::shared_ptr<http_resource> res); // NOLINT(build/include_what_you_use)
129129

130130
/**
131131
* Register a lambda handler for HTTP GET on @p path.

src/peer_address.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,14 @@ std::string emit_canonical(const ipv6_groups& g, const zero_run& collapse) {
126126
}
127127
int n = std::snprintf(buf + pos, sizeof(buf) - pos, "%x",
128128
static_cast<unsigned>(g[i]));
129+
// snprintf returns the length it WOULD have written (excl. NUL).
130+
// A group is at most 4 hex digits, so with buf[40] this branch is
131+
// never taken; the explicit guard keeps `pos` provably within the
132+
// buffer so the `sizeof(buf) - pos` size argument can never
133+
// underflow into a huge size_t (cpp/overflowing-snprintf, CWE-190).
134+
if (n < 0 || static_cast<std::size_t>(n) >= sizeof(buf) - pos) {
135+
return std::string(buf, pos);
136+
}
129137
pos += static_cast<std::size_t>(n);
130138
++i;
131139
// Emit a ':' separator if more groups remain AND the next slot

test/bench_sizeof_http_resource.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ using httpserver::v1_baseline::V1_STD_MAP_STRING_BOOL_SIZEOF;
8888
// TASK-058's Allow cache needed, this assertion breaks at compile
8989
// time and the growth gets reviewed.
9090
//
91-
// TODO: unlike V1_GET_HEADERS_NS_PER_CALL in bench_get_headers.cpp (which
91+
// TODO(etr): unlike V1_GET_HEADERS_NS_PER_CALL in bench_get_headers.cpp (which
9292
// carries a per-stdlib defensive static_assert against an accidental
9393
// cross-stdlib swap, e.g. a libstdc++ value silently reused on libc++),
9494
// V1_HTTP_RESOURCE_SIZEOF / V1_STD_MAP_STRING_BOOL_SIZEOF below have no

test/integ/authentication.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include <memory>
3535
#include <optional>
3636
#include <string>
37+
#include <utility>
3738

3839
#include "./httpserver.hpp"
3940
#include "./littletest.hpp"
@@ -320,6 +321,7 @@ class digest_ha1_resource : public http_resource {
320321
}
321322
return http_response::string("SUCCESS");
322323
}
324+
323325
private:
324326
http_utils::digest_algorithm algo_;
325327
const unsigned char* ha1_;

test/integ/debug_dump_request_body_set_test.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,11 @@ LT_END_AUTO_TEST(dump_body_to_stdout_when_env_set)
196196
LT_BEGIN_AUTO_TEST_ENV()
197197
// Opt in BEFORE the test bodies run (and BEFORE the magic-static
198198
// cache in the body pipeline is initialised on first dispatch).
199+
#ifdef _WIN32
200+
// MinGW's <stdlib.h> does not always declare POSIX setenv.
201+
_putenv("LIBHTTPSERVER_DEBUG_DUMP_REQUEST_BODY=1");
202+
#else
199203
::setenv("LIBHTTPSERVER_DEBUG_DUMP_REQUEST_BODY", "1", 1);
204+
#endif
200205
AUTORUN_TESTS()
201206
LT_END_AUTO_TEST_ENV()

0 commit comments

Comments
 (0)