From 79c29b47e8c2cb0995f522779007eeca52dca214 Mon Sep 17 00:00:00 2001 From: sami Date: Sat, 12 Sep 2026 21:34:21 +0400 Subject: [PATCH] core: finish the hostile-mode matrix -- 8 remaining end-to-end cases Was 7/16 of tools/testserver/README.md's mode table covered by engine_test.cpp. Adds the rest: - engine_expiring_signed_url_recovers_via_refresh_url: an expired signed URL 403s, the engine asks (paused, decision_calls >= 1) rather than failing terminally, and DownloadHandle::refresh_url() with a freshly signed URL completes it -- exercises both do_refresh_url() fixes and the probe-level referrer retry's second-403 path from the previous commit. - engine_403_without_referer_retries_with_origin: no spec.referrer set, the automatic single retry (previous commit) recovers with zero decisions asked. - engine_redirect_chain_follows_to_completion: 5 hops of a plain 302. No core-side change needed -- documents that CURLOPT_FOLLOWLOCATION/ MAXREDIRS (already on, RequestOptions::follow_redirects) cover both the probe's and every worker's own request, not just one of the two. - engine_slow_loris_stall_timeout_fires: proves curl's stall detector (CURLOPT_LOW_SPEED_LIMIT/_TIME, download_task.cpp's hardcoded 1024 B/s for 30s) actually fires rather than hanging. Needed a real fix, not just a test: every other test in this file relies on TestServer's short 1s loris dribble to keep runtime down, but 1s of trickle followed by full-speed streaming never accumulates curl's required 30 CONSECUTIVE seconds under the floor, so it would never actually abort -- a test built on the default dribble would pass by the download merely finishing a bit late, not by observing the stall timeout fire. testserver_fixture.hpp's TestServer gained an explicit-loris-seconds constructor (default ctor unchanged, still 1s) so this one test can ask for a dribble (40s) that genuinely outlasts the threshold. - engine_401_digest_then_provide_auth_completes: same shape as the existing 401-basic test: http_client.cpp already asks libcurl for CURLAUTH_ANY regardless of net::AuthScheme, so this needed no core change -- it passed on the first run and is here to prove that's true end-to-end, not just at the http_client unit level. - engine_chunked_no_length_completes_single_segment: Transfer-Encoding: chunked, no Content-Length anywhere (including HEAD). No core change needed -- takes the same size-agnostic "unknown size, one plain-GET segment" path as the existing no-range test. - utf8/legacy-content-disposition: already covered end-to-end by probe_reads_utf8_content_disposition and probe_reads_legacy_content_disposition in probe_test.cpp (probe-level, as these modes only affect the initial request) -- verified passing, no new test needed. All 20 engine_test.cpp cases and all 10 probe_test.cpp cases pass. Every testserver.py spawned while writing and running this was reaped by TestServer's destructor; verified no stragglers with `ps aux` after each run. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Q3QrF7rCt21bkAjt9BCDFQ --- core/tests/net/testserver_fixture.hpp | 12 +- core/tests/task/engine_test.cpp | 185 ++++++++++++++++++++++++++ 2 files changed, 195 insertions(+), 2 deletions(-) diff --git a/core/tests/net/testserver_fixture.hpp b/core/tests/net/testserver_fixture.hpp index 28a4bc7..6d8e0a4 100644 --- a/core/tests/net/testserver_fixture.hpp +++ b/core/tests/net/testserver_fixture.hpp @@ -28,7 +28,14 @@ namespace vdm::testing { class TestServer { public: - TestServer() { + TestServer() : TestServer(1.0) {} + + // loris_seconds overrides the dribble duration slow-loris mode uses (default matches the + // no-arg ctor's long-standing 1s). A test that needs curl's stall detector + // (CURLOPT_LOW_SPEED_TIME, hardcoded to 30s in download_task.cpp) to actually fire needs a + // dribble that outlasts that threshold, not the short one every other test relies on to + // keep runtime down. + explicit TestServer(double loris_seconds) { const char *script = VDM_TESTSERVER_PY; if (!script || !*script || ::access(script, R_OK) != 0) return; @@ -50,8 +57,9 @@ class TestServer { int devnull = ::open("/dev/null", O_WRONLY); if (devnull >= 0) ::dup2(devnull, STDERR_FILENO); + std::string loris_str = std::to_string(loris_seconds); ::execlp("python3", "python3", script, "--port", "0", "--seed", "9", "--loris-seconds", - "1", "--throttle-bps", "131072", static_cast(nullptr)); + loris_str.c_str(), "--throttle-bps", "131072", static_cast(nullptr)); ::_exit(127); } ::close(pipefd[1]); diff --git a/core/tests/task/engine_test.cpp b/core/tests/task/engine_test.cpp index 1dadb29..94a903c 100644 --- a/core/tests/task/engine_test.cpp +++ b/core/tests/task/engine_test.cpp @@ -124,6 +124,31 @@ std::string server_sha(TestServer &srv, const std::string &mode, const std::stri return out.substr(open + 1, close - open - 1); } +// Small, deliberately identical extraction to server_sha's: GET //sign/?ttl=N +// and pull the "url" field's value out of the {"url":..., "exp":...} JSON body. +std::string sign_url(TestServer &srv, const std::string &mode, const std::string &size, + int ttl_seconds) { + std::string url = + srv.url("/" + mode + "/sign/" + size + "?ttl=" + std::to_string(ttl_seconds)); + std::string cmd = "curl -s '" + url + "'"; + std::string out; + if (FILE *f = ::popen(cmd.c_str(), "r")) { + char buf[1024]; + while (std::fgets(buf, sizeof buf, f)) + out += buf; + ::pclose(f); + } + auto q = out.find("\"url\""); + if (q == std::string::npos) + return {}; + auto colon = out.find(':', q); + auto open = out.find('"', colon); + auto close = out.find('"', open + 1); + if (open == std::string::npos || close == std::string::npos) + return {}; + return out.substr(open + 1, close - open - 1); +} + DownloadSpec spec_for(TestServer &srv, const std::string &urlpath, const std::string &save) { DownloadSpec s; s.url = srv.url(urlpath); @@ -328,6 +353,30 @@ VT_TEST(engine_401_then_provide_auth_completes) { VT_CHECK_EQ(file_size(td.file("au.bin")), 1u * 1024 * 1024); } +VT_TEST(engine_401_digest_then_provide_auth_completes) { + // Same shape as engine_401_then_provide_auth_completes, but the challenge is HTTP + // Digest (qop=auth) rather than Basic. provide_auth() doesn't know or care which -- + // http_client.cpp always asks libcurl for CURLAUTH_ANY (net::AuthScheme::any) and lets + // curl negotiate against whatever WWW-Authenticate the server actually sent -- so this + // exists purely to prove that's true end-to-end, not just at the unit level. + TestServer srv; + VT_REQUIRE(srv.available()); + TmpDir td; + Recorder rec; + Engine eng; + DownloadHandle h; + auto cbs = rec.cbs(&h, "test", "test"); + h = eng.start(spec_for(srv, "/401-digest/file/1M", td.file("dg.bin")), std::move(cbs)); + rec.arm(h); + + auto r = rec.wait(); + VT_REQUIRE(r.has_value()); + VT_CHECK(rec.auth_calls.load() >= 1); + VT_CHECK_EQ(file_size(td.file("dg.bin")), 1u * 1024 * 1024); + auto got = hash_file(td.file("dg.bin"), Checksum::Algo::sha256); + VT_CHECK_EQ(got.value(), server_sha(srv, "401-digest", "1M")); +} + // --- hostile-mode matrix: the four where a bug is silent corruption, not a visible // failure (docs/04 §5 "ask, never silently corrupt" / §7's failure-policy table). --- @@ -458,6 +507,142 @@ VT_TEST(engine_content_length_mismatch_fails_honestly) { VT_CHECK_EQ(::access(td.file("clm.bin").c_str(), F_OK), -1); // never renamed into place } +// --- remaining hostile-mode matrix (tools/testserver/README.md's mode table). --- + +VT_TEST(engine_expiring_signed_url_recovers_via_refresh_url) { + // A signed URL past its ttl 403s (tools/testserver's own JSON body distinguishes + // "expired" from "bad signature", but core never parses response bodies -- CLAUDE.md + // §3 -- so both just read as a 403). The one automatic referrer retry (see + // engine_403_without_referer_retries_with_origin, below) can't fix an expired + // signature, so the second 403 asks -- via the same auto_pause_locked(..., false, + // true) "ask, don't just fail" path as wrong_status/range_bad -- rather than + // terminally failing outright, specifically so DownloadHandle::refresh_url() (its own + // contract: works "on a live or paused task", never on a terminal one) stays usable: + // the README pairs this mode with exactly that recovery. + TestServer srv; + VT_REQUIRE(srv.available()); + TmpDir td; + Recorder rec; + Engine eng; + + std::string expired = sign_url(srv, "expiring-signed-url", "64K", /*ttl=*/1); + VT_REQUIRE(!expired.empty()); + std::this_thread::sleep_for(1500ms); // let the ttl actually pass before the first request + + DownloadSpec s; + s.url = expired; + s.save_path = td.file("exp.bin"); + auto h = eng.start(std::move(s), rec.cbs()); + + for (int i = 0; i < 300 && rec.decision_calls.load() == 0; ++i) + std::this_thread::sleep_for(20ms); + VT_REQUIRE(rec.decision_calls.load() >= 1); + VT_CHECK_EQ(h.state(), EngineState::paused); + + std::string fresh = sign_url(srv, "expiring-signed-url", "64K", /*ttl=*/60); + VT_REQUIRE(!fresh.empty()); + h.refresh_url(fresh); + + auto r = rec.wait(60s); + VT_REQUIRE(r.has_value()); + VT_CHECK_EQ(file_size(td.file("exp.bin")), 64u * 1024); + auto got = hash_file(td.file("exp.bin"), Checksum::Algo::sha256); + VT_CHECK_EQ(got.value(), server_sha(srv, "expiring-signed-url", "64K")); +} + +VT_TEST(engine_403_without_referer_retries_with_origin) { + // docs/04 §7: "403 after redirect: retry once with the original referrer -- many CDNs + // require it." No spec.referrer is set here (the common case for anything not + // initiated from a browser page, e.g. `velox add `), so the first attempt 403s; + // the engine's own retry supplies the download URL's own origin as Referer, which + // this mode accepts, and the download completes with no decision ever asked. + TestServer srv; + VT_REQUIRE(srv.available()); + TmpDir td; + Recorder rec; + Engine eng; + auto h = eng.start(spec_for(srv, "/403-without-referer/file/128K", td.file("ref.bin")), + rec.cbs()); + auto r = rec.wait(30s); + VT_REQUIRE(r.has_value()); + VT_CHECK_EQ(rec.decision_calls.load(), 0); // recovered automatically, not asked + VT_CHECK_EQ(file_size(td.file("ref.bin")), 128u * 1024); + auto got = hash_file(td.file("ref.bin"), Checksum::Algo::sha256); + VT_CHECK_EQ(got.value(), server_sha(srv, "403-without-referer", "128K")); +} + +VT_TEST(engine_redirect_chain_follows_to_completion) { + // 5 hops (tools/testserver's own --redirect-depth default) of a plain 302, query + // string preserved across each. No CORE-side logic needed for this one -- libcurl's + // own CURLOPT_FOLLOWLOCATION (RequestOptions::follow_redirects, already on) and + // CURLOPT_MAXREDIRS (default 20, well over 5) do the whole thing -- this is here as + // the end-to-end check that they're actually wired through both the probe and every + // segment worker's own request, not just one of the two. + TestServer srv; + VT_REQUIRE(srv.available()); + TmpDir td; + Recorder rec; + Engine eng; + auto h = eng.start(spec_for(srv, "/redirect-chain/file/1M", td.file("rc.bin")), rec.cbs()); + auto r = rec.wait(30s); + VT_REQUIRE(r.has_value()); + VT_CHECK_EQ(file_size(td.file("rc.bin")), 1u * 1024 * 1024); + auto got = hash_file(td.file("rc.bin"), Checksum::Algo::sha256); + VT_CHECK_EQ(got.value(), server_sha(srv, "redirect-chain", "1M")); +} + +VT_TEST(engine_slow_loris_stall_timeout_fires) { + // Status line, headers, and body dribbled out one byte at a time for --loris-seconds, + // then (if the dribble hasn't already been cut off) normal streaming -- a connection + // that's technically alive (bytes ARE arriving, just far too slowly) but must not be + // allowed to hang the task forever. http_client.cpp sets CURLOPT_LOW_SPEED_LIMIT/_TIME + // (RequestOptions::low_speed_bytes_per_sec/low_speed_secs, hardcoded in + // download_task.cpp to 1024 B/s for 30s) for exactly this. + // + // Every other test in this file uses TestServer's default 1s loris dribble to keep + // runtime down, but 1s is far shorter than curl's 30s low_speed_time: a 1s trickle + // followed by full-speed streaming never accumulates 30 CONSECUTIVE seconds under the + // floor, so curl would never actually abort it -- the download would just complete + // slightly late, which would make this test pass for the wrong reason (or not exercise + // the stall timeout at all). Explicitly ask for a dribble that outlasts the 30s + // threshold so the stall timeout is the thing actually observed firing, not assumed. + TestServer srv(40.0); + VT_REQUIRE(srv.available()); + TmpDir td; + Recorder rec; + Engine eng; + auto s = spec_for(srv, "/slow-loris/file/64K", td.file("sl.bin")); + s.segments = 1; + s.max_retries = 1; + auto h = eng.start(std::move(s), rec.cbs()); + auto r = rec.wait(60s); // stall timeout fires ~30s in; must resolve, not hang to 60s + VT_REQUIRE(!r.has_value()); + VT_CHECK(is_retryable(r.error().code) || r.error().code == Error::max_retries_exhausted); +} + +VT_TEST(engine_chunked_no_length_completes_single_segment) { + // No Content-Length anywhere (HEAD gets none either, since it's the same handler path) + // -- the probe can't know total_size or prove resumability, so this should take the + // exact same "unknown size, one plain-GET segment" path as engine_non_resumable_single_ + // segment, just arriving there via a chunked body instead of a server that plainly + // refuses Range. No core-side work needed if that demotion is already size-agnostic; + // this is here to prove it, since every other test's server tells the probe the size + // up front. + TestServer srv; + VT_REQUIRE(srv.available()); + TmpDir td; + Recorder rec; + Engine eng; + auto h = eng.start(spec_for(srv, "/chunked-no-length/file/2M", td.file("ch.bin")), + rec.cbs()); + auto r = rec.wait(); + VT_REQUIRE(r.has_value()); + VT_CHECK_EQ(rec.decision_calls.load(), 0); + VT_CHECK_EQ(file_size(td.file("ch.bin")), 2u * 1024 * 1024); + auto got = hash_file(td.file("ch.bin"), Checksum::Algo::sha256); + VT_CHECK_EQ(got.value(), server_sha(srv, "chunked-no-length", "2M")); +} + // --- DAEMON-reported bug: Progress.speed_bps reads 0 for the whole life of a live // download while downloaded bytes visibly advance. DAEMON reads progress by polling // DownloadHandle::progress() (engine_port_core.hpp), not the on_progress push callback --