sami/vdm
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

core: retry once with the original referrer on a 403, at probe and worker

Browse Source 
docs/04-engine-design.md §7's failure policy table has said "403 after
redirect: retry once with the original referrer; many CDNs require it"
since it was written, and Error::forbidden's own enum comment says the
same -- but grepping download_task.cpp and http_client.cpp for 403 turned
up nothing. It was never built.

Implemented at both points a 403 can surface:

- The probe (net::Prober, a separate request path from segment workers):
  on_probe_result() now retries once via restart_probe(false), with
  effective_referrer set to the download URL's own origin (origin_of(),
  via net::split_url()), when the failure is Error::forbidden and this is
  the first retry. A second 403 asks rather than fails outright --
  auto_pause_locked(..., false, true), the same "ask, don't just fail"
  path 416/etag-mismatch already use -- specifically so DownloadHandle::
  refresh_url() stays usable afterward (its own contract requires a
  non-terminal task); this is what makes the expiring-signed-url mode's
  README-documented refresh_url() recovery actually reachable.

- Each segment worker (SegWorker::forbidden, set in seg_head() on a 403
  HEAD): the same one-shot referrer retry via retry_worker(), landing on
  auto_pause_locked() on a second 403 for the same reason.

Both paths route the retry's Referer through a new effective_referrer
field rather than spec.referrer directly, since the origin-retry must not
overwrite what the caller actually asked for -- start_worker_locked() and
restart_probe() were switched to send effective_referrer instead.

do_refresh_url() had two latent bugs surfaced by actually exercising the
expiring-signed-url recovery path end-to-end:

1. It unconditionally proceeded to resume even when the refresh probe
   itself failed -- a bad refresh URL would silently un-pause a task with
   nothing behind it. Now returns (stays paused) on !r.has_value().
2. It only handled "already probed once, just refreshing a few fields" --
   for a task whose first-ever probe never succeeded (every hostile mode
   this commit adds a test for that pauses at the initial probe, not
   mid-download), s->registered was never true, so the existing
   `if (s->registered) set_want()` never fired and nothing happened. Now
   detects !s->have_probe and calls finish_probe_locked() directly, the
   actual first-time registration/segmenter-construction path.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Q3QrF7rCt21bkAjt9BCDFQ
This commit is contained in:
sami
2026-09-12 21:34:24 +04:00
co-authored by Claude Sonnet 5
parent c1c5c82f8b
commit 8e7d14ba7e
1 changed files with 127 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/src/task/download_task.cpp
+127 -9
View File
@@ -64,6 +64,19 @@ std::string lower(std::string s) {
return s;
}
// scheme://host[:port] of `url`, with no path/query/fragment -- what docs/04 §7's "403
// after redirect: retry once with the original referrer" retries with as the Referer
// header. Empty on an unparseable URL (the caller just won't get a referrer retry).
std::string origin_of(std::string_view url) {
auto s = net::split_url(url);
if (!s.valid)
return {};
std::string out = s.scheme + "://" + s.host;
if (s.port)
out += ":" + std::to_string(*s.port);
return out;
}
} // namespace
// What to do once every worker has drained (see DownloadTaskState::begin_drain_locked).
@@ -88,6 +101,7 @@ struct SegWorker {
bool needs_auth = false;
bool wrong_status = false;
bool range_bad = false;
bool forbidden = false; // 403 -- docs/04 §7's "retry once with the original referrer"
bool auth_handshake = false; // saw a 401/407 and let libcurl resend with credentials
std::string resp_etag, resp_last_modified; // captured on a wrong_status 200, for demote
std::optional<ErrorInfo> flush_error;
@@ -135,6 +149,16 @@ struct DownloadTaskState : std::enable_shared_from_this<DownloadTaskState> {
std::optional<std::uint64_t> total_size;
std::string origin_host;
// docs/04 §7's "403 after redirect: retry once with the original referrer" -- many
// CDNs 403 a bare/foreign Referer. Starts as spec.referrer (the browser's, verbatim);
// start_worker_locked() sends this, not spec.referrer directly, so a 403 retry can
// override it (to the download URL's own origin) without touching what the caller
// actually asked for. referrer_retried bounds it to exactly once per task -- a second
// 403 with a same-origin Referer already set is a real, honest failure
// (Error::forbidden), not something a referrer swap can fix.
std::string effective_referrer;
bool referrer_retried = false;
std::unique_ptr<segment::Segmenter> seg;
std::unique_ptr<io::SparseFile> file;
std::unordered_map<std::uint32_t, std::unique_ptr<SegWorker>> workers;
@@ -164,7 +188,7 @@ struct DownloadTaskState : std::enable_shared_from_this<DownloadTaskState> {
std::vector<std::function<void()>> deferred;
DownloadTaskState(TaskHost &h, TaskId i, DownloadSpec s, DownloadCallbacks c)
: host(h), id(i), spec(std::move(s)), cbs(std::move(c)) {}
: host(h), id(i), spec(std::move(s)), cbs(std::move(c)), effective_referrer(spec.referrer) {}
// --- deferred callbacks -------------------------------------------------------------
void defer(std::function<void()> fn) {
@@ -286,7 +310,7 @@ void DownloadTaskState::restart_probe(bool with_auth) {
pr.url = spec.url;
pr.headers = spec.headers;
pr.cookies = spec.cookies;
pr.referrer = spec.referrer;
pr.referrer = effective_referrer;
pr.user_agent = spec.user_agent;
pr.proxy = spec.proxy;
if (with_auth)
@@ -299,12 +323,37 @@ void DownloadTaskState::restart_probe(bool with_auth) {
}
void DownloadTaskState::on_probe_result(Result<net::ProbeResult> r) {
bool retry_probe_with_referrer = false;
{
std::unique_lock lk(mu);
if (retired.load() || is_terminal(state))
return;
if (!r.has_value()) {
fail_locked(std::move(r).error());
ErrorInfo e = std::move(r).error();
// docs/04 §7's referrer retry applies here too: a probe (HEAD, or the
// ranged-GET fallback when HEAD is refused -- probe.cpp) can be the request
// that actually gets 403'd, before any segment worker exists to retry it
// (net::Prober builds its own request from ProbeRequest::referrer, not
// through start_worker_locked() -- see restart_probe()'s use of
// effective_referrer below). Same one-shot bound via referrer_retried as the
// worker-level retry (seg_finished's w->forbidden branch) shares.
if (e.code == Error::forbidden && !referrer_retried) {
referrer_retried = true;
effective_referrer = origin_of(spec.url);
retry_probe_with_referrer = true;
} else if (e.code == Error::forbidden) {
// Already retried with the origin referrer and still 403 -- not something
// another blind retry fixes (an expired signed URL, a private resource).
// Ask rather than fail outright, the same "ask, don't just fail" shape as
// wrong_status/range_bad/the worker-level 403 branch: refresh_url() is a
// no-op once the task is terminal, and tools/testserver's expiring-signed-
// url mode (also a bare 403, indistinguishable from any other without
// parsing the body -- CLAUDE.md §3, core never does) is meant to be
// recovered exactly that way.
auto_pause_locked(std::move(e), false, true);
} else {
fail_locked(std::move(e));
}
} else {
probe = std::move(r).value();
have_probe = true;
@@ -319,6 +368,8 @@ void DownloadTaskState::on_probe_result(Result<net::ProbeResult> r) {
}
}
flush_deferred();
if (retry_probe_with_referrer)
restart_probe(false);
}
void DownloadTaskState::finish_probe_locked() {
@@ -472,7 +523,7 @@ void DownloadTaskState::start_worker_locked(std::uint32_t seg_idx) {
req.url = current_url();
req.headers = spec.headers;
req.cookies = spec.cookies;
req.referrer = spec.referrer;
req.referrer = effective_referrer;
req.user_agent = spec.user_agent;
req.proxy = spec.proxy;
req.auth = spec.auth;
@@ -552,6 +603,10 @@ net::DataAction DownloadTaskState::seg_head(std::uint32_t seg_idx, const net::Re
w->range_bad = true;
return net::DataAction::abort;
}
if (h.status == 403) {
w->forbidden = true;
return net::DataAction::abort;
}
if (h.status >= 400)
return net::DataAction::abort;
seg->set_segment_state(seg_idx, segment::SegState::downloading);
@@ -637,7 +692,7 @@ void DownloadTaskState::seg_finished(std::uint32_t seg_idx, Result<net::Transfer
// (content-length-mismatch's honest-length lie, flaky-reset's tail, a proxy RST after
// the last byte). If the segment is fully covered, that's a success.
if (seg && !cancel_requested && !pause_requested && !w->needs_auth && !w->wrong_status &&
!w->flush_error && !w->range_bad) {
!w->flush_error && !w->range_bad && !w->forbidden) {
const std::uint64_t len = seg->segment_end(seg_idx) - seg->segment_start(seg_idx) + 1;
if (len != 0 && seg->segment_completed(seg_idx) >= len) {
r = Result<net::TransferStats>(net::TransferStats{});
@@ -756,6 +811,35 @@ void DownloadTaskState::seg_finished(std::uint32_t seg_idx, Result<net::Transfer
auto_pause_locked(ErrorInfo(Error::range_not_satisfiable, "416"), false, true);
return done();
}
if (w->forbidden) {
// docs/04 §7: "403 after redirect: retry once with the original referrer -- many
// CDNs require it." Bare/foreign Referer is the common cause; origin_of() rebuilds
// it from the (possibly redirected) URL the response actually came from. Exactly
// once per task, not a backoff series -- a second 403 with a same-origin Referer
// already set isn't something another blind retry can fix (a private/expired
// resource, an expiring signed URL past its window, ...). That's not necessarily
// terminal, though: ask (same "ask, don't just fail outright" shape as
// wrong_status/range_bad above) rather than fail_locked() outright, specifically
// so DownloadHandle::refresh_url() -- do_refresh_url() is a no-op once the task is
// terminal -- stays usable for the case tools/testserver's README pairs it with:
// a caller that gets a fresh signed URL and hands it back.
release_slot();
if (!referrer_retried) {
referrer_retried = true;
effective_referrer = origin_of(current_url());
seg->set_segment_state(seg_idx, segment::SegState::stalled);
auto wp = weak_from_this();
host.schedule(std::chrono::steady_clock::now(), [wp, seg_idx] {
if (auto s = wp.lock())
s->retry_worker(seg_idx);
});
if (workers.empty())
transition(EngineState::retry_wait, std::nullopt);
} else {
auto_pause_locked(ErrorInfo(Error::forbidden, "403", w->http_status), false, true);
}
return done();
}
if (!r.has_value()) {
ErrorInfo e = std::move(r).error();
@@ -1215,6 +1299,7 @@ void DownloadTaskState::do_refresh_url(std::string url, std::vector<net::HeaderF
net::ProbeRequest pr;
pr.url = spec.url;
pr.headers = spec.headers;
pr.referrer = effective_referrer;
pr.auth = spec.auth;
pr.proxy = spec.proxy;
host.probe(std::move(pr), [wp](Result<net::ProbeResult> r) {
@@ -1224,10 +1309,43 @@ void DownloadTaskState::do_refresh_url(std::string url, std::vector<net::HeaderF
std::unique_lock lk(s->mu);
if (s->retired.load() || is_terminal(s->state))
return;
if (r.has_value()) {
s->probe.effective_url = r.value().effective_url;
s->probe.etag = r.value().etag;
s->probe.last_modified = r.value().last_modified;
if (!r.has_value()) {
lk.unlock();
s->flush_deferred();
return; // still paused; the caller can retry refresh_url() or decide()
}
if (!s->have_probe) {
// The task's *first* probe never succeeded (e.g. this session's own
// expiring-signed-url path: 403, one referrer retry, still 403 -> ask rather
// than fail outright -- see on_probe_result() -- specifically so this branch
// exists to recover it). finish_probe_locked() is what actually registers the
// task with the budget and builds its Segmenter; nothing downstream of a
// partial field copy would ever start a worker without it.
s->probe = std::move(r).value();
s->have_probe = true;
s->awaiting_auth = false;
s->awaiting_decision = false;
s->finish_probe_locked();
lk.unlock();
s->flush_deferred();
return;
}
s->probe.effective_url = r.value().effective_url;
s->probe.etag = r.value().etag;
s->probe.last_modified = r.value().last_modified;
// refresh_url()'s own contract is "on a live OR PAUSED task, without losing
// progress" -- distinct from do_decide(restart), which discards progress. A task
// can be paused here for any of three reasons (a plain user pause, awaiting_auth,
// or awaiting_decision -- e.g. this session's own 403-after-referrer-retry path,
// or the pre-existing wrong_status/range_bad ones); apply_slot_target()'s guard
// blocks on awaiting_auth/awaiting_decision specifically, so leaving either set
// would have set_want() below recompute a target that nothing ever acts on --
// the caller's new URL re-probed successfully and then the task just sat there.
// Clear both and leave `paused` the same way do_decide(restart) does.
if (s->state == EngineState::paused) {
s->awaiting_auth = false;
s->awaiting_decision = false;
s->transition(EngineState::connecting, std::nullopt);
}
if (s->registered)
s->host.budget().set_want(s->id, s->want_slots());