sulthan
21615be2bd
Closes #94. ## What Two phases per the spec, in three feature commits plus two review-fix commits: **Phase one — one registry entry per Site** (`2d134fb`) The six per-site comparison points that used to live across three files collapse into one `sites` map in `backend/internal/latest/sites.go`: Latest Chapter parse, Cover parse, browser-backed list, fetcher route, host pins, and the browser payload read all become lookups into it. `browserBackedSites()` is derived from the registry (sorted, deterministic); `fetcherFor` and `fetchableSeriesURL` keep their signatures and become lookups; `BrowserFetcher.Get` dispatches through the entries' `Read`/`Done` while the tab lifecycle stays in `BrowserFetcher.run`. **Phase two — one shared Series-page read** (`f215130`) `readSeriesPage` (new `read.go`) performs the read the Poll and the Acquisition have in common: gate, route, fetch, parse Latest Chapter, parse Cover address. It returns facts only — polling and persistence policies (stamp order, cooldowns, cover policy) stay with the callers; `acquire.go` gained the comment naming the deliberate post-fetch stamp order. The poll's legacy cover heal and the no-chapter byte-count diagnostic were restored after review (`d998f87`) so the claims "the Poll keeps its own Cover policy" and "pinning is the only behavioural change" both hold. ## Behaviour - All six Sites now pin their host exactly; asura/demonic/comix previously accepted any https host. For asura this is a strict improvement: its dead old domain redirects deep links to the site root and would parse the wrong document. - Everything else is unchanged: existing parse tables, the challenge-body table and the gate table pass unmodified except the one deliberate exception — the gate table gains the three new pin cases. ## Security invariants preserved - The address gate is recognisably the same rule, now a single registry lookup: `https` + exact hostname match, all callers route through it. No fetch path was widened; asura/demonic/comix were narrowed. - The second host pin inside each browser entry's Read is retained deliberately (browser = strong SSRF primitive, `series_url` is client-supplied) and is not deduplicated against the shared gate. - Review hardening: `fetcherFor` now fails closed for unknown site strings (previously fell through to the TLS fetcher on an unreachable path), and the browser dispatch iterates a sorted list so outcomes cannot depend on map order. - The security review's log-injection finding was checked against Go's `url.Parse` and does not hold: control characters are rejected anywhere in a URL, so a client-supplied value in a log line cannot carry a newline. ## Review Reviewed on three axes (spec, standards, security) by read-only subagents over `672c16f..f1b26f4`. No blocking findings; all minor/nit findings addressed in `d998f87` and `700de20`. Verified end to end with `go test ./...` (Docker Postgres per test package) on every commit. ## Out of scope (tracked separately) - Dropping asuracomic.net (CORS allowlist, userscript match, API fixtures, live env) — separate issue, per spec. Reviewed-on: #95 Co-authored-by: Sulthan Zaki <sultankiki05@gmail.com> Co-committed-by: Sulthan Zaki <sultankiki05@gmail.com>
94 lines
5.0 KiB
Markdown
94 lines
5.0 KiB
Markdown
# ADR-0009: A Site answers fixed questions; an unusual Site owns its own fetch
|
|
|
|
Date: 2026-08-11
|
|
Status: accepted
|
|
|
|
## Decision
|
|
|
|
Per-Site knowledge lives in one registry in `backend/internal/latest/sites.go`,
|
|
keyed by the stored site string. An entry answers a fixed set of questions: the
|
|
hostname a `series_url` must carry, how to find the Latest Chapter in a body,
|
|
how to find the Cover address in a body, and — for a Site behind a JavaScript
|
|
challenge — how to read its payload from a cleared tab, how to tell that the
|
|
payload arrived, and whether the plain-TLS fetcher may take over when no
|
|
browser is configured (`Fallback`; kagane never falls back, novelfull does,
|
|
each on measured evidence).
|
|
|
|
The question set does not grow to accommodate one Site. When a Site needs
|
|
something the set cannot express, that Site gets an optional override and
|
|
performs its own fetch, leaving the other entries untouched. The override is a
|
|
per-Site escape hatch, not a stage every Site passes through, and it is added to
|
|
the registry type when the first Site needs it rather than in advance.
|
|
|
|
Two things stay outside the registry. Cover *bytes* are routed by address shape
|
|
in `fetchCoverBytes`, never by Site name, so the Poll and the Acquisition cannot
|
|
drift apart. And the host pin inside a browser entry's read is kept even though
|
|
`fetchableSeriesURL` has already pinned the same host: a headless browser is a
|
|
strong SSRF primitive and `series_url` arrives in a client-supplied PUT body, so
|
|
the second check is deliberate and must not be deduplicated.
|
|
|
|
## Why
|
|
|
|
Before the registry, the site string was compared in six places across three
|
|
files: the Latest Chapter switch (`sites.go:102`), the Cover switch
|
|
(`sites.go:263`), the browser-backed list and the fetcher choice
|
|
(`poller.go:60`, `poller.go:132`), the host pins (`poller.go:314-325`), and the
|
|
payload read (`browser.go:96-119`). Nothing tied them together, so adding a
|
|
seventh Site meant finding all six unaided, and a Site added to five of them
|
|
failed at the sixth in production rather than at compile time.
|
|
|
|
The Sites are not alike and the registry does not ask them to be. asura strips a
|
|
rotating build hash from its slug before scoping a regex; comix reads a JSON
|
|
blob embedded in server-rendered HTML; kagane's chapter list exists only in its
|
|
JSON API, which must be called from inside the page so the request carries the
|
|
clearance cookie; lightnovelworld must truncate the body at the comment thread
|
|
first. What they have in common is not behaviour, it is the questions they
|
|
answer. Arbitrary behaviour behind one entry is the point.
|
|
|
|
Making a browser Site contribute a read and a completion test, rather than
|
|
letting it drive the browser, was chosen because the tab lifecycle in
|
|
`BrowserFetcher.run` is load-bearing and shared. It holds one tab open across
|
|
re-reads, because a Cloudflare interstitial needs several seconds of live page
|
|
to solve itself and write clearance into the shared cookie jar; reading once and
|
|
closing the tab, which is what this did before 2026-08-08, never clears
|
|
anything. It also serialises the browser, binds the caller's deadline to the
|
|
tab, distinguishes a lost browser from a retryable read, and paces re-reads.
|
|
Spreading that across per-Site adapters would put one subtle, measured loop
|
|
behind six doors.
|
|
|
|
This costs the adapters little, because `chromedp.Run` takes an Action and
|
|
`chromedp.Tasks` is an Action. A Site that must click, wait on a selector, and
|
|
then evaluate expresses all of it as its read. Only a Site needing something
|
|
outside the per-tab loop — its own cadence, two tabs, a tab held between calls,
|
|
cookies set before navigation — falls outside, and that Site takes the override.
|
|
|
|
## Considered options
|
|
|
|
**Widen the shared interface whenever a Site needs something new.** Rejected:
|
|
one Site's requirement becomes a field on all seven entries, and the entries
|
|
that ignore it still have to be read and understood by anyone adding the eighth.
|
|
|
|
**Give every Site the whole fetch.** Rejected: it makes the browser lifecycle
|
|
above a per-Site concern, and pulls `chromedp` into adapters for five Sites that
|
|
never open a browser.
|
|
|
|
**A Go `interface` with a method set instead of a registry of records.**
|
|
Rejected: most Sites differ in one or two answers, and three share a single
|
|
Cover implementation, so a method set produces near-empty types. A missing
|
|
answer is a nil value caught at dispatch, which is where an unknown Site is
|
|
already handled.
|
|
|
|
## Consequences
|
|
|
|
Adding a Site is one registry entry. The existing dispatch functions —
|
|
`latestChapterFrom`, `coverFrom`, `fetchableSeriesURL` — become registry
|
|
lookups, so the table tests that drive them by site string are unchanged.
|
|
|
|
A future architecture review will see an override that only one Site uses and
|
|
read it as an inconsistency to collapse. It is not. Collapsing it means either
|
|
widening the question set for every Site or moving the shared tab lifecycle into
|
|
the adapters, and both were rejected here on the evidence above.
|
|
|
|
An unknown site string resolves to the zero entry and fails the existing
|
|
not-fetchable and no-fetcher paths, which log and skip. That is unchanged.
|