Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Download links are cached in two places and neither cache looks at
DownloadLink.ExpiresAt:pkg/debrid/account/account.go—Account.GetDownloadLinkreturns whatever isstored 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.
pkg/manager/link/service.go—fetchAndValidatetakes the link fromfetchLinkand validates it (HEAD) without ever considering its age. Onlyafter 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
400for an unknown/expired download UUID; a bad token gives401, so this isspecifically "this UUID is gone"):
linkService.validated.ExpiresAt(defaultauto_expire_links_afteris 3d; themidnight
linkService.Clear()job also drops thevalidatedentry, so thenext read takes the un-memoized path).
Observed:
fetchLinkreturns the stale URL from the account cache,validateLinkHEADs 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()—ExpiresAtin the past, and non-zero. A zeroExpiresAtmeans the provider didn't tell us a lifetime, and those links keeptheir current behaviour exactly.
Account.GetDownloadLinkevicts an expired entry on read and falls through tothe fetcher, instead of handing back a URL it knows is dead.
fetchAndValidatechecks expiry immediately afterfetchLinkand, when thelink is expired, goes straight to
invalidateAndRefetch; the fresh link thencontinues through the normal path and is validated once. No extra HEAD, no
ladder, no recursion.
Every provider that sets
ExpiresAtderives it fromauto_expire_links_after(alldebrid, debridlink, premiumize, realdebrid, torbox), each falling back to 48h
when the config value is empty or unparseable, so
ExpiresAtis neveraccidentally zero-valued-into-the-past. Code paths that build a
DownloadLinkwithout an expiry are unaffected.
Deliberately not included
pkg/manager/workers.goscheduleslinkService.Clear()at 00:00 CET and clearsonly 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):ExpiresAtis in the future is served without calling thefetcher;
ExpiresAtis evicted, the fetcher is called exactly once, andthe fresh link replaces it in the cache;
ExpiresAtis still served from cache regardless of age(the no-op guarantee for providers that don't expose a lifetime);
Expired()itself.go build ./...,go vet ./...clean.go test ./...passes except for twopre-existing failures unrelated to this change (
pkg/storageTestDowngradeRoundTripsThroughVersion3and the twopkg/sharetests), whichfail identically on an unmodified checkout of
mainon this machine.