Skip to content

fix(link): skip validation for links that are already expired - #411

Open
Lefted wants to merge 1 commit into
sirrobot01:mainfrom
Lefted:fix/skip-validation-for-expired-links
Open

Lefted wants to merge 1 commit into
sirrobot01:mainfrom
Lefted:fix/skip-validation-for-expired-links

Conversation

@Lefted

@Lefted Lefted commented Sep 8, 2026

Copy link
Copy Markdown

Problem

Download links are cached in two places and neither cache looks at
DownloadLink.ExpiresAt:

  1. pkg/debrid/account/account.goAccount.GetDownloadLink returns whatever is
    stored under the sliced file link. The only check on the way out is Valid(),
    which just verifies the URL parses. A cached entry therefore outlives the
    download URL it holds, indefinitely, until the process restarts or something
    else deletes it.
  2. pkg/manager/link/service.gofetchAndValidate takes the link from
    fetchLink and validates it (HEAD) without ever considering its age. Only
    after validation fails in a retryable way does invalidateAndRefetch (Retry link validation on read_pxy_timeout and other retryable error #384)
    drop the link and request a fresh one.

So a link the code already knows is expired still goes through the full
validation retry ladder — ~60 s with default retry/backoff settings — before it
is refetched. During streaming the reader blocks for that whole minute, once per
file: on playback start when the mount's cache doesn't already hold the head of
the file, and on seeks past the cached region.

Reproduction

Provider whose CDN rejects an expired download token (TorBox returns a bodyless
400 for an unknown/expired download UUID; a bad token gives 401, so this is
specifically "this UUID is gone"):

  1. Fetch a download link for a file — it lands in the account cache and in
    linkService.validated.
  2. Wait past its ExpiresAt (default auto_expire_links_after is 3d; the
    midnight linkService.Clear() job also drops the validated entry, so the
    next read takes the un-memoized path).
  3. Read the file.

Observed: fetchLink returns the stale URL from the account cache, validateLink
HEADs it, the CDN answers 400, the retry ladder runs to exhaustion (~60 s), and
only then is a fresh link fetched. The refetched link works, which confirms
nothing was wrong except the age of the cached one.

Fix

  • types.DownloadLink.Expired()ExpiresAt in the past, and non-zero. A zero
    ExpiresAt means the provider didn't tell us a lifetime, and those links keep
    their current behaviour exactly.
  • Account.GetDownloadLink evicts an expired entry on read and falls through to
    the fetcher, instead of handing back a URL it knows is dead.
  • fetchAndValidate checks expiry immediately after fetchLink and, when the
    link is expired, goes straight to invalidateAndRefetch; the fresh link then
    continues through the normal path and is validated once. No extra HEAD, no
    ladder, no recursion.

Every provider that sets ExpiresAt derives it from auto_expire_links_after
(alldebrid, debridlink, premiumize, realdebrid, torbox), each falling back to 48h
when the config value is empty or unparseable, so ExpiresAt is never
accidentally zero-valued-into-the-past. Code paths that build a DownloadLink
without an expiry are unaffected.

Deliberately not included

pkg/manager/workers.go schedules linkService.Clear() at 00:00 CET and clears
only the validation memo — Account.ClearDownloadLinks() exists and is unused,
which is the other half of why a stale link could survive the night. Adding the
account clear to that job would work, but with expiry enforced on read it mostly
discards links that are still perfectly good, and it needs the manager to reach
into every provider's account manager. Happy to add it (or to drop the scheduled
clear entirely, which the read-side check makes redundant) if you'd prefer that
in the same PR.

Tests

New pkg/debrid/account/account_test.go (the package had no tests):

  • a cached entry whose ExpiresAt is in the future is served without calling the
    fetcher;
  • an entry past ExpiresAt is evicted, the fetcher is called exactly once, and
    the fresh link replaces it in the cache;
  • an entry with a zero ExpiresAt is still served from cache regardless of age
    (the no-op guarantee for providers that don't expose a lifetime);
  • table test for Expired() itself.

go build ./..., go vet ./... clean. go test ./... passes except for two
pre-existing failures unrelated to this change (pkg/storage
TestDowngradeRoundTripsThroughVersion3 and the two pkg/share tests), which
fail identically on an unmodified checkout of main on this machine.

Download links are cached twice: in the per-account link cache
(`pkg/debrid/account`, keyed by the file link) and in the link service's
`validated` memo. Neither cache looks at `DownloadLink.ExpiresAt`:

- `Account.GetDownloadLink` returns whatever is stored. `Valid()` only
  checks that the URL parses, so an entry outlives the URL it holds.
- `fetchAndValidate` validates the link it got back before anything
  considers its age. On a provider whose CDN answers 4xx for an expired
  download token, that means a link known to be expired still goes
  through the full validation retry ladder, ~60 s with default settings,
  before being refetched. During playback the reader blocks for that
  whole minute, once per file.

Fix both ends:

- evict an expired entry on read in the account cache, so the next
  `GetDownloadLink` fetches a fresh URL instead of handing back a dead
  one;
- in `fetchAndValidate`, check expiry right after the fetch and go
  straight to `invalidateAndRefetch`, then validate the fresh link once
  through the normal path.

`ExpiresAt` is only set where a provider actually knows the lifetime
(all five providers derive it from `auto_expire_links_after`, falling
back to 48h). Links that leave it zero are never considered expired, so
this is a no-op for them.

Not touched: the midnight `linkService.Clear()` job in
`pkg/manager/workers.go` still clears only the validation memo. With
expiry now enforced on read, a wholesale clear of the account caches
would just discard links that are still good; the scheduled clear can go
away separately.

Tests: `pkg/debrid/account` gains coverage for the three cache-read
cases (valid entry served from cache, expired entry evicted and
refetched, entry without an expiry left alone) plus the `Expired()`
predicate.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant