mirror of
https://github.com/yhirose/cpp-httplib.git
synced 2026-10-17 10:59:39 +00:00
* Add reproducer for #2431 (getaddrinfo_a use-after-free) On Linux/glibc, getaddrinfo_with_timeout() runs DNS asynchronously via getaddrinfo_a(GAI_NOWAIT) using a stack-local gaicb. When gai_suspend() hits the connection timeout, gai_cancel() is called and the function returns immediately — but gai_cancel() is non-blocking and can return EAI_NOTCANCELED, leaving the resolver worker thread alive and still referencing the destroyed stack frame. Adds three opt-in gtest cases (GetAddrInfoAsyncCancelTest.*) that exercise the cancel path repeatedly. They are gated on Linux/glibc + CPPHTTPLIB_USE_NON_BLOCKING_GETADDRINFO at compile time, and on the CPPHTTPLIB_TEST_ISSUE_2431=1 env var at runtime, so a normal `make test` run is unaffected. Also adds a dedicated CI job (issue-2431-repro) and a Docker-based local runner (test/run_issue_2431_repro.sh) that sinkhole UDP/53 so the timeout branch is taken, and run the test under ASAN/LSAN. With the bug present these runs are expected to fail; with a fix applied they should pass. Refs: https://github.com/yhirose/cpp-httplib/issues/2431 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix split build for #2431 reproducer tests The new GetAddrInfoAsyncCancelTest cases call detail::getaddrinfo_with_timeout directly. In split builds (make test_split) split.py moves the definition into httplib.cc and strips `inline`, so the symbol is not declared in the public httplib.h and test.cc fails to compile -- breaking the ubuntu/test-no-exceptions CI jobs that the PR description says should be unaffected. Add a forward declaration in test.cc, gated by the same #if as the tests themselves, so it links against the split-build symbol without changing the header-only build. * Cap issue-2431 repro job at 5 minutes The bug manifests as orphan getaddrinfo_a resolver workers that keep the runner from completing job teardown -- the previous run had all steps succeed in ~1m37s but then hung in "Cleaning up orphan processes" for ~57m before GitHub force-killed the job. A job-level timeout-minutes makes the failure signal fast and predictable: bug present -> killed at 5 min, bug fixed -> ~2 min pass. Step-level timeout isn't enough since the hang is in post-job cleanup, not the test step. * Enable ASAN detect_stack_use_after_return for #2431 repro The bug is a textbook stack-use-after-return: a stack-local struct gaicb is destroyed when getaddrinfo_with_timeout returns after gai_cancel() yields EAI_NOTCANCELED, then the still-live resolver worker thread writes back into the freed frame. ASAN's detect_stack_use_after_return is the direct detector for exactly this pattern -- enabling it lets the failure surface as a clear ASAN diagnostic during the test run instead of as an orphan-process hang at job teardown. * Revert ASAN detect_stack_use_after_return for #2431 repro The option did not detect the bug in CI -- the resolver worker write likely lands on the heap (via the gaicb's pai pointer) or happens after the test process exits, neither of which stack-use-after-return can catch. Roll back to relying on the job-level timeout: bug present -> post-cleanup hangs ~8min then job-level timeout cancels at 10min total; bug fixed -> job completes in ~2min. * Switch issue-2431 repro to a delayed loopback DNS test fixture The previous repro setup dropped UDP/53 outright, which made glibc's resolver hang forever on every lookup -- the worker never actually received a response and so never reached the buggy write-back path that #2431 is about. As a result, neither the broken HEAD nor the fix made any visible difference in CI: both produced "tests pass + post-cleanup hangs ~10min" because the orphan resolver thread is a structural property of *any* getaddrinfo path on a hung resolver, not a property of the bug. Replace the sinkhole with a small loopback test fixture (test/dns_test_fixture.py, ~50 lines, stdlib only) that answers DNS queries after a 3s delay -- longer than the test's 1s timeout. An iptables NAT rule routes the test job's lookups to the fixture without touching /etc/resolv.conf, so the rest of the runner's DNS behaviour is unaffected. With ASAN's detect_stack_use_after_return enabled, the worker's late write-back into the destroyed gaicb stack frame is now caught as a stack-use-after-return diagnostic, so the broken HEAD fails fast at the test step (clear red) and the fix turns the same job green in well under a minute. Same fixture is wired into both the GitHub Actions job and the docker-based test/run_issue_2431_repro.sh script, so local repro on macOS and CI repro on Linux exercise the identical path. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
33bc1df930
commit
d14e4fc05f
4 files changed
+409
No files matched your search
+134
@@ -1549,6 +1549,140 @@ TEST(GetAddrInfoDanglingRefTest, LongTimeout) {
|
||||
std::this_thread::sleep_for(std::chrono::seconds(8));
|
||||
}
|
||||
|
||||
#if defined(__linux__) && defined(__GLIBC__) && \
|
||||
defined(CPPHTTPLIB_USE_NON_BLOCKING_GETADDRINFO)
|
||||
|
||||
// Forward declaration: in split builds split.py strips `inline` and moves the
|
||||
// definition into httplib.cc, so detail::getaddrinfo_with_timeout is not
|
||||
// visible from the public httplib.h. Re-declaring it here lets the tests link
|
||||
// against the symbol in both header-only and split builds.
|
||||
namespace httplib {
|
||||
namespace detail {
|
||||
int getaddrinfo_with_timeout(const char *node, const char *service,
|
||||
const struct addrinfo *hints,
|
||||
struct addrinfo **res, time_t timeout_sec);
|
||||
} // namespace detail
|
||||
} // namespace httplib
|
||||
|
||||
// Reproducer for https://github.com/yhirose/cpp-httplib/issues/2431.
|
||||
//
|
||||
// On Linux/glibc, getaddrinfo_with_timeout() runs the lookup via
|
||||
// getaddrinfo_a(GAI_NOWAIT) using a stack-local `struct gaicb`. When the
|
||||
// gai_suspend() call hits the connection timeout the function calls
|
||||
// gai_cancel() and returns immediately. gai_cancel() is non-blocking and
|
||||
// can return EAI_NOTCANCELED, in which case the resolver worker thread is
|
||||
// still alive and still references the now-destroyed stack frame.
|
||||
//
|
||||
// Triggering the bug requires DNS to actually hang (UDP/53 dropped, etc.),
|
||||
// so these tests are gated on CPPHTTPLIB_TEST_ISSUE_2431=1 and are skipped
|
||||
// during normal runs. test/run_issue_2431_repro.sh sets up the environment
|
||||
// and runs them in a container.
|
||||
namespace {
|
||||
bool should_run_issue_2431_tests() {
|
||||
const char *v = getenv("CPPHTTPLIB_TEST_ISSUE_2431");
|
||||
return v && *v && std::string(v) != "0";
|
||||
}
|
||||
|
||||
std::string unique_unresolvable_host(int n) {
|
||||
// .invalid is reserved (RFC 6761) and is never served by real DNS, but
|
||||
// glibc still asks the configured nameserver — which is exactly the path
|
||||
// we want to exercise. A unique label per call avoids the resolver cache.
|
||||
auto t = std::chrono::steady_clock::now().time_since_epoch().count();
|
||||
return "h-" + std::to_string(::getpid()) + "-" + std::to_string(t) + "-" +
|
||||
std::to_string(n) + ".invalid";
|
||||
}
|
||||
} // namespace
|
||||
|
||||
TEST(GetAddrInfoAsyncCancelTest, DirectCallSingleThread) {
|
||||
if (!should_run_issue_2431_tests()) {
|
||||
GTEST_SKIP()
|
||||
<< "Set CPPHTTPLIB_TEST_ISSUE_2431=1 (and sinkhole DNS) to run";
|
||||
}
|
||||
|
||||
for (int i = 0; i < 8; ++i) {
|
||||
struct addrinfo hints;
|
||||
memset(&hints, 0, sizeof(hints));
|
||||
hints.ai_family = AF_UNSPEC;
|
||||
hints.ai_socktype = SOCK_STREAM;
|
||||
|
||||
auto host = unique_unresolvable_host(i);
|
||||
struct addrinfo *result = nullptr;
|
||||
int rc = detail::getaddrinfo_with_timeout(host.c_str(), "80", &hints,
|
||||
&result, /*timeout_sec=*/1);
|
||||
if (rc == 0 && result) { freeaddrinfo(result); }
|
||||
}
|
||||
|
||||
// Give orphaned getaddrinfo_a worker threads a chance to write into the
|
||||
// stack region they still believe holds their gaicb.
|
||||
std::this_thread::sleep_for(std::chrono::seconds(3));
|
||||
}
|
||||
|
||||
TEST(GetAddrInfoAsyncCancelTest, DirectCallMultiThread) {
|
||||
if (!should_run_issue_2431_tests()) {
|
||||
GTEST_SKIP()
|
||||
<< "Set CPPHTTPLIB_TEST_ISSUE_2431=1 (and sinkhole DNS) to run";
|
||||
}
|
||||
|
||||
std::atomic<bool> stop{false};
|
||||
std::vector<std::thread> threads;
|
||||
for (int t = 0; t < 8; ++t) {
|
||||
threads.emplace_back([t, &stop] {
|
||||
int i = 0;
|
||||
while (!stop.load(std::memory_order_relaxed)) {
|
||||
struct addrinfo hints;
|
||||
memset(&hints, 0, sizeof(hints));
|
||||
hints.ai_family = AF_UNSPEC;
|
||||
hints.ai_socktype = SOCK_STREAM;
|
||||
|
||||
auto host = unique_unresolvable_host(t * 100000 + i++);
|
||||
struct addrinfo *result = nullptr;
|
||||
int rc = detail::getaddrinfo_with_timeout(host.c_str(), "80", &hints,
|
||||
&result, /*timeout_sec=*/1);
|
||||
if (rc == 0 && result) { freeaddrinfo(result); }
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
std::this_thread::sleep_for(std::chrono::seconds(8));
|
||||
stop.store(true, std::memory_order_relaxed);
|
||||
for (auto &th : threads) {
|
||||
th.join();
|
||||
}
|
||||
std::this_thread::sleep_for(std::chrono::seconds(3));
|
||||
}
|
||||
|
||||
TEST(GetAddrInfoAsyncCancelTest, ClientGetMultiThread) {
|
||||
if (!should_run_issue_2431_tests()) {
|
||||
GTEST_SKIP()
|
||||
<< "Set CPPHTTPLIB_TEST_ISSUE_2431=1 (and sinkhole DNS) to run";
|
||||
}
|
||||
|
||||
std::atomic<bool> stop{false};
|
||||
std::vector<std::thread> threads;
|
||||
for (int t = 0; t < 8; ++t) {
|
||||
threads.emplace_back([t, &stop] {
|
||||
int i = 0;
|
||||
while (!stop.load(std::memory_order_relaxed)) {
|
||||
auto host = unique_unresolvable_host(t * 100000 + i++);
|
||||
Client cli(host, 80);
|
||||
cli.set_connection_timeout(1, 0);
|
||||
cli.set_read_timeout(1, 0);
|
||||
cli.set_write_timeout(1, 0);
|
||||
(void)cli.Get("/");
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
std::this_thread::sleep_for(std::chrono::seconds(8));
|
||||
stop.store(true, std::memory_order_relaxed);
|
||||
for (auto &th : threads) {
|
||||
th.join();
|
||||
}
|
||||
std::this_thread::sleep_for(std::chrono::seconds(3));
|
||||
}
|
||||
|
||||
#endif // __linux__ && __GLIBC__ && CPPHTTPLIB_USE_NON_BLOCKING_GETADDRINFO
|
||||
|
||||
TEST(ConnectionErrorTest, InvalidHost) {
|
||||
auto host = "-abcde.com";
|
||||
|
||||
|
||||
Reference in new issue
Block a user