Files
mangaBookmark/backend/cover_test.go
T
sulthan 92eba07da7 A newly bookmarked Series acquires its Cover at creation (#59) (#68)
Closes #59.

Part of spec #55, and the ticket that fixes the reported bug #47. Architecture: `docs/adr/0007-backend-hosts-cover-bytes.md`. Does not close #47 or #55.

## What changed

A Reader bookmarks a Series nobody holds yet — the exact case in #47 — and within seconds the list shows its artwork instead of a broken image. The first Bookmark to create a Series fires `Store.OnSeriesCreated` after commit, and the new `latest.Acquirer` turns that into **one** series-page fetch that yields both the Latest Chapter and the cover URL. The bytes go through the gated cover fetcher from #57 and are stored content-addressed through #56, so the wire carries an absolute URL on this deployment's own origin — never a third-party address, and never one that 404s.

### Store

- Migration `0009_series_cover_address.sql` adds `series.cover_address`. The two facts are now split: `series.cover` is the third-party source address the bytes came from (the acquisition path's dedupe key), `series.cover_address` is the SHA-256 they are stored under. An empty `cover_address` is precisely what "no Cover yet" means, which is the distinction both the API and the UI depend on.
- `SetSeriesCover` writes the address only after the bytes are on disk, so the wire can never name an object that is not there.
- `CoverWireURL` builds `PUBLIC_BASE_URL + /covers/<sha256>` for every scanned row, and returns `""` for a blank address.
- The cover columns are gone from `Upsert`'s `INSERT` and its `DO UPDATE`. A client-supplied cover cannot reach the shared Series row on any path, not just the creation path.
- `Open` now rejects a base URL that is not an absolute `http(s)` origin: `PUBLIC_BASE_URL=bookmarks.example.com` would otherwise start cleanly and emit addresses no browser can load.

### Acquisition

- `internal/latest/acquire.go`: one fetch, gated by the poller's own `fetchableSeriesURL` (a `series_url` arrives in a client-supplied PUT body, so without the gate a token-holder chooses what the server fetches from its own network position).
- Asynchronous and log-and-drop. The Bookmark, its progress and its Latest Chapter are already committed; a Site that is down or a cover that cannot be produced disturbs none of them.
- Bounded by a two-slot semaphore. A bulk sync creating N Series would otherwise fire N simultaneous requests from one IP — the traffic shape the poller's stagger exists to avoid.
- Cancelled at shutdown (shares the poller's context) and stamps `latest_checked_at`, so the poller does not refetch the same page a tick later.
- Browser-backed Sites (kagane, novelfull) are deliberately skipped: their pages only yield a Cloudflare challenge to the TLS client, so the request would be spent for nothing. They arrive in #62.

### Wire and route

- `GET /covers/{address}` serves the bytes publicly and uncredentialed with `Cache-Control: public, max-age=604800, immutable`. The address is gated by a `^[0-9a-f]{64}$` pattern and cross-checked against a pure function of itself before any filesystem read, so no request shaped like a traversal reaches disk.
- `PUT /bookmarks/{key}` still accepts a `cover` field and discards it, permanently. Rejecting it would break every installed userscript the moment this deploys, and ADR-0004's compatibility argument depends on those scripts continuing to work. The decode site says so in place of a TODO nobody intends to keep.
- `store.CoverContentType` canonicalises comix's non-standard `image/jpg` to `image/jpeg`, so one image cannot land under two spellings. This one was found by the live smoke test, not by reading.

### Config

`PUBLIC_BASE_URL` is new and required (cover URLs must go out absolute — the userscript renders them on third-party origins, where a relative path resolves against the Site). Documented in `.env.example`, `docker-compose.yml` (`:?` so compose fails too), `DEPLOY.md` and `backend/AGENTS.md`.

## Acceptance criteria

All twelve of #59's criteria are met; the checklist on the issue is ticked with the evidence.

## Verification

- `go test ./...` green (Docker-backed Postgres suite).
- Live smoke against a real backend + Postgres: bookmarking `comix:n8we-dungeons-and-crayons` produced `"cover": "http://127.0.0.1:8099/covers/8ce74d80…"` and `"latest_chapter": "Chapter 81"` within seconds of the PUT; `curl` on that address returned `200`, `Content-Type: image/jpeg`, `Cache-Control: public, max-age=604800, immutable`, and a 280x420 JPEG. That run is what surfaced the `image/jpg` content type.
- Mutation-checked the asynchrony test: removing the `go` from `Acquire` turns `TestAcquireDoesNotBlockTheWrite` red.

## Reviewed

Both axes of `/code-review` were run against this diff before commit. Their findings that were actionable here are folded in: the concurrency bound, the shutdown tie, the `PUBLIC_BASE_URL` validation, the missing `latest_checked_at` stamp, and a test that could not fail.

## Known sequencing

A kagane/novelfull Series created between this deploy and #62 has no cover source at all: the acquisition skips those Sites and `Upsert` no longer persists the userscript-scraped address. This is #59's stated boundary rather than a defect, but it is a user-visible gap on two Sites and should order #62 accordingly.

Reviewed-on: #68
Co-authored-by: Sulthan Zaki <sultankiki05@gmail.com>
Co-committed-by: Sulthan Zaki <sultankiki05@gmail.com>
2026-08-10 04:07:53 +07:00

307 lines
10 KiB
Go

package main
import (
"context"
"crypto/sha256"
"errors"
"net/http"
"net/http/httptest"
"strings"
"sync/atomic"
"testing"
"bookmarkmanager/backend/internal/pgtest"
"bookmarkmanager/backend/internal/store"
)
// fakeCovers stands in for the headless browser. It counts calls so the test
// can prove the store spares the browser after the first navigation.
type fakeCovers struct {
body []byte
contentType string
err error
calls atomic.Int32
lastID atomic.Value
}
func (f *fakeCovers) Image(_ context.Context, imageID string) ([]byte, string, error) {
f.calls.Add(1)
f.lastID.Store(imageID)
if f.err != nil {
return nil, "", f.err
}
return f.body, f.contentType, nil
}
const testCoverID = "019fe11a-84c3-7fc3-a84b-88787374b617"
func getCover(t *testing.T, srv http.Handler, path string, cookie *http.Cookie) *httptest.ResponseRecorder {
t.Helper()
req := httptest.NewRequest(http.MethodGet, path, nil)
if cookie != nil {
req.AddCookie(cookie)
}
rr := httptest.NewRecorder()
srv.ServeHTTP(rr, req)
return rr
}
// kagane serves its covers behind a Cloudflare challenge and with
// cross-origin-resource-policy: same-origin, so the UI can only show one by
// re-serving the bytes from its own origin.
func TestKaganeCoverPersistsAndReusesStoredBytes(t *testing.T) {
cf := &fakeCovers{body: []byte("\x00webp-bytes"), contentType: "image/webp"}
cfg := testConfig()
cfg.Covers = cf
srv, st := newWebTestServer(t, cfg)
cookie := sessionCookie(t, st)
rr := getCover(t, srv, "/img/kagane/"+testCoverID, cookie)
if rr.Code != http.StatusOK {
t.Fatalf("first request: status = %d, want 200", rr.Code)
}
if got := rr.Body.String(); got != string(cf.body) {
t.Fatalf("first request: body = %q, want %q", got, cf.body)
}
// A new Handler has no process-local state from the first request. The same
// store must still answer without navigating the browser again.
srv = newRouter(st, cfg)
rr = getCover(t, srv, "/img/kagane/"+testCoverID, cookie)
if rr.Code != http.StatusOK {
t.Fatalf("stored request: status = %d, want 200", rr.Code)
}
if got := rr.Body.String(); got != string(cf.body) {
t.Fatalf("stored request: body = %q, want %q", got, cf.body)
}
if got := cf.calls.Load(); got != 1 {
t.Fatalf("fetcher called %d times, want 1 — stored bytes must survive a new handler", got)
}
if got := cf.lastID.Load(); got != testCoverID {
t.Fatalf("fetched image id = %v, want %s", got, testCoverID)
}
}
func TestKaganeCoverServesStoredBytesWithoutBrowser(t *testing.T) {
cf := &fakeCovers{err: errors.New("browser must not be called")}
cfg := testConfig()
cfg.Covers = cf
srv, st := newWebTestServer(t, cfg)
if err := st.PutKaganeCover(testCoverID, []byte("already-stored"), "image/png"); err != nil {
t.Fatalf("PutKaganeCover: %v", err)
}
rr := getCover(t, srv, "/img/kagane/"+testCoverID, sessionCookie(t, st))
if rr.Code != http.StatusOK || rr.Body.String() != "already-stored" {
t.Fatalf("stored request = (%d, %q), want (200, already-stored)", rr.Code, rr.Body.String())
}
if got := cf.calls.Load(); got != 0 {
t.Fatalf("fetcher called %d times for a stored cover, want 0", got)
}
}
func TestKaganeCoverServesPersistedBytesAfterRestart(t *testing.T) {
url := pgtest.URL(t)
coverDir := t.TempDir()
owner := store.Owner{
DiscordID: "cover-owner",
TokenHash: sha256.Sum256([]byte("cover-owner-token")),
}
first, err := store.Open(url, owner, coverDir, testCoverBaseURL)
if err != nil {
t.Fatalf("Open: %v", err)
}
if err := first.PutKaganeCover(testCoverID, []byte("survives-restart"), "image/jpeg"); err != nil {
first.Close()
t.Fatalf("PutKaganeCover: %v", err)
}
if err := first.Close(); err != nil {
t.Fatalf("close first store: %v", err)
}
second, err := store.Open(url, owner, coverDir, testCoverBaseURL)
if err != nil {
t.Fatalf("reopen: %v", err)
}
defer second.Close()
cf := &fakeCovers{err: errors.New("browser must not be called after restart")}
cfg := testConfig()
cfg.Covers = cf
rr := getCover(t, newRouter(second, cfg), "/img/kagane/"+testCoverID, sessionCookie(t, second))
if rr.Code != http.StatusOK || rr.Body.String() != "survives-restart" {
t.Fatalf("restarted request = (%d, %q), want (200, survives-restart)", rr.Code, rr.Body.String())
}
if got := cf.calls.Load(); got != 0 {
t.Fatalf("fetcher called %d times after restart, want 0", got)
}
}
// The proxy reaches a headless browser, so it is not open to the internet.
func TestKaganeCoverRequiresSession(t *testing.T) {
cf := &fakeCovers{body: []byte("x"), contentType: "image/webp"}
cfg := testConfig()
cfg.Covers = cf
srv, _ := newWebTestServer(t, cfg)
rr := getCover(t, srv, "/img/kagane/"+testCoverID, nil)
if rr.Code != http.StatusUnauthorized {
t.Fatalf("status = %d, want 401", rr.Code)
}
if got := cf.calls.Load(); got != 0 {
t.Fatalf("fetcher called %d times for an unauthenticated request, want 0", got)
}
}
func TestKaganeCoverRejectsBadInput(t *testing.T) {
cases := []struct {
name string
id string
fetch *fakeCovers
}{
{
"an id that is not a uuid never reaches the browser",
"solo-leveling",
&fakeCovers{body: []byte("x"), contentType: "image/webp"},
},
{
"a uuid-shaped id with a trailing segment is rejected whole",
testCoverID + "x",
&fakeCovers{body: []byte("x"), contentType: "image/webp"},
},
{
"a challenged fetch is a missing cover",
testCoverID,
&fakeCovers{err: errors.New("challenge held")},
},
{
"a content type outside the image set is not echoed back",
testCoverID,
&fakeCovers{body: []byte("<script>"), contentType: "text/html"},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
cfg := testConfig()
cfg.Covers = tc.fetch
srv, st := newWebTestServer(t, cfg)
rr := getCover(t, srv, "/img/kagane/"+tc.id, sessionCookie(t, st))
if rr.Code != http.StatusNotFound {
t.Fatalf("status = %d, want 404", rr.Code)
}
_, _, ok, err := st.GetKaganeCover(tc.id)
if err != nil {
t.Fatalf("GetKaganeCover after rejection: %v", err)
}
if ok {
t.Fatal("rejected cover was persisted")
}
})
}
}
// ServeMux path-cleans a traversal into a redirect before the handler runs, so
// the guarantee to pin down is that no request shaped like one ever gets bytes.
func TestKaganeCoverTraversalServesNothing(t *testing.T) {
cf := &fakeCovers{body: []byte("secret"), contentType: "image/webp"}
cfg := testConfig()
cfg.Covers = cf
srv, st := newWebTestServer(t, cfg)
rr := getCover(t, srv, "/img/kagane/../../etc/passwd", sessionCookie(t, st))
if rr.Code == http.StatusOK {
t.Fatalf("status = 200, want anything but a served body")
}
if got := cf.calls.Load(); got != 0 {
t.Fatalf("fetcher called %d times for a traversal, want 0", got)
}
}
// Without BROWSER_WS_URL there is no fetcher, and the endpoint must answer
// rather than reach for a nil one.
func TestKaganeCoverWithoutFetcher(t *testing.T) {
srv, st := newWebTestServer(t, testConfig())
rr := getCover(t, srv, "/img/kagane/"+testCoverID, sessionCookie(t, st))
if rr.Code != http.StatusNotFound {
t.Fatalf("status = %d, want 404", rr.Code)
}
}
// The acquired Cover is served from this deployment's own origin, to any
// browser rendering a third-party page — no session, no credential (ADR-0007).
func TestPublicCoverServesStoredBytesUnauthenticated(t *testing.T) {
const sourceURL = "https://cdn.asurascans.com/covers/solo.webp"
srv, st := newWebTestServer(t, testConfig())
if err := st.PutCover(sourceURL, []byte("\x00webp-bytes"), "image/webp"); err != nil {
t.Fatalf("PutCover: %v", err)
}
// The wire URL is what a client actually requests, so the path under test
// is taken from it rather than rebuilt by hand.
wire := st.CoverWireURL(store.CoverAddress(sourceURL))
path, ok := strings.CutPrefix(wire, testCoverBaseURL)
if !ok {
t.Fatalf("wire URL %q is not on the public origin %q", wire, testCoverBaseURL)
}
rr := getCover(t, srv, path, nil)
if rr.Code != http.StatusOK {
t.Fatalf("status = %d, want 200 without any credential", rr.Code)
}
if got := rr.Body.String(); got != "\x00webp-bytes" {
t.Fatalf("body = %q, want the stored bytes", got)
}
if got := rr.Header().Get("Content-Type"); got != "image/webp" {
t.Fatalf("Content-Type = %q, want the stored one", got)
}
// Content-addressed bytes never change, so a client that has them must
// never need to ask again.
if got := rr.Header().Get("Cache-Control"); !strings.Contains(got, "immutable") {
t.Fatalf("Cache-Control = %q, want an immutable cache directive", got)
}
}
func TestPublicCoverRejectsUnknownAddress(t *testing.T) {
srv, _ := newWebTestServer(t, testConfig())
cases := map[string]string{
"unknown": "/covers/" + store.CoverAddress("https://cdn.example/never-stored.jpg"),
"malformed": "/covers/not-an-address",
"traversal": "/covers/../../etc/passwd",
"empty": "/covers/",
}
for name, path := range cases {
t.Run(name, func(t *testing.T) {
if rr := getCover(t, srv, path, nil); rr.Code == http.StatusOK {
t.Fatalf("%s: status = 200, want anything but a served body", path)
}
})
}
}
// The whole point of acquiring bytes is that the UI shows them: the card's
// <img> must carry the public address, not a third-party URL and not a
// placeholder.
func TestListRendersAcquiredCover(t *testing.T) {
const sourceURL = "https://static.comix.to/039d/i/1/34/6a6742bf15736@280.jpg"
srv, st := newWebTestServer(t, testConfig())
if _, err := st.Upsert(st.OwnerID(), store.Bookmark{
Key: "comix:n8we", Site: "comix", SeriesID: "n8we", Title: "Dungeons and Crayons",
SeriesURL: "https://comix.to/title/n8we", UpdatedAt: 1000,
}); err != nil {
t.Fatalf("seed: %v", err)
}
if err := st.SetSeriesCover("comix", "n8we", sourceURL, []byte("\xff\xd8jpeg"), "image/jpeg"); err != nil {
t.Fatalf("SetSeriesCover: %v", err)
}
req := httptest.NewRequest(http.MethodGet, "/ui/list", nil)
req.AddCookie(sessionCookie(t, st))
rr := httptest.NewRecorder()
srv.ServeHTTP(rr, req)
if rr.Code != http.StatusOK {
t.Fatalf("status = %d, want 200", rr.Code)
}
want := `src="` + testCoverBaseURL + "/covers/" + store.CoverAddress(sourceURL) + `"`
if !strings.Contains(rr.Body.String(), want) {
t.Fatalf("rendered list does not contain %s", want)
}
}