From 0347e11b7d7aa1983b1c950f0afc472389179433 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Mon, 5 Oct 2026 13:59:02 -0400 Subject: [PATCH 1/2] Add photo cache and /media/photos endpoint recipes.PhotoCache.CachePhoto downloads an image through the safe Fetcher's new FetchImage (JPEG/PNG/WebP/GIF content types only, 8 MB cap, uncached), decodes it (GIF first frame), refuses images under 300 px on the short side, scales to 1200 px and 400 px on the long side with golang.org/x/image/draw, encodes JPEG quality 82 under $MEALQUEST_DATA_DIR/media/photos/{id}-{size}.jpg, deduplicates by the SHA-256 of the original bytes and records a photos row. GET /media/photos/{id}-{400|1200}.jpg serves a stored size behind the session with Cache-Control "private, max-age=31536000, immutable"; anything unknown is 404. Adds golang.org/x/image v0.45.0 (approved; v0.46.0 needs Go 1.26). Maestro task: 8a432108-d3ba-4013-ae8c-11963981d641 Co-Authored-By: Claude Opus 5.5 --- cmd/server/main.go | 8 +- docs/ARCHITECTURE.md | 13 +- go.mod | 1 + go.sum | 2 + internal/recipes/fetcher.go | 80 +++++- internal/recipes/photo.go | 248 ++++++++++++++++ internal/recipes/photo_test.go | 405 +++++++++++++++++++++++++++ internal/recipes/testdata/photo.webp | Bin 0 -> 2304 bytes internal/server/login_test.go | 2 +- internal/server/media.go | 74 +++++ internal/server/media_test.go | 117 ++++++++ internal/server/origin_test.go | 2 +- internal/server/server.go | 9 +- internal/server/server_test.go | 27 +- internal/server/settings_test.go | 2 +- 15 files changed, 965 insertions(+), 25 deletions(-) create mode 100644 internal/recipes/photo.go create mode 100644 internal/recipes/photo_test.go create mode 100644 internal/recipes/testdata/photo.webp create mode 100644 internal/server/media.go create mode 100644 internal/server/media_test.go diff --git a/cmd/server/main.go b/cmd/server/main.go index 4f46d29..146e254 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -19,6 +19,7 @@ import ( "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/frontend" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/jobs" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/secretbox" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/server" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" @@ -72,6 +73,11 @@ func run(cfg config.Config, logger *slog.Logger) error { return err } + photos, err := recipes.NewPhotoCache(recipes.NewFetcher(), db, cfg.DataDir) + if err != nil { + return err + } + sessions, err := auth.NewSessionManager(db, cfg.SessionIdle, cfg.SessionMax) if err != nil { return err @@ -88,7 +94,7 @@ func run(cfg config.Config, logger *slog.Logger) error { return err } - router, err := server.NewRouter(logger, version, cfg.TrustedProxy, authFlow, sessions, settings, runner, frontend.Assets()) + router, err := server.NewRouter(logger, version, cfg.TrustedProxy, authFlow, sessions, settings, runner, photos, frontend.Assets()) if err != nil { return err } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 44418ec..1787868 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -140,7 +140,10 @@ automatic** (D-021). It requires the session cookie and, for non-GET requests, the `X-MealQuest-CSRF` header. - `/auth/*` — the OIDC flow. -- `/media/*` — cached photos. +- `/media/*` — cached photos, behind the session cookie. + `GET /media/photos/{id}-{400|1200}.jpg` serves a stored size with + `Cache-Control: private, max-age=31536000, immutable`; anything else is + 404. - `/health` — liveness. - Everything else serves the embedded PWA with SPA fallback. @@ -264,7 +267,13 @@ automatic** (D-021). and reason (fetch failure, no valid recipe, missing ingredients or image), so the caller decides per item what a failure means. - Photos are cached locally (`photos`) and processed with - `golang.org/x/image`. + `golang.org/x/image`. `recipes.PhotoCache.CachePhoto` downloads through + the Fetcher's `FetchImage` (JPEG, PNG, WebP or GIF content types, 8 MB + cap, not cached), decodes the image (a GIF's first frame), refuses one + under 300 px on its short side, scales it so its long side is 1200 px and + 400 px, and stores each as JPEG quality 82 at + `$MEALQUEST_DATA_DIR/media/photos/{id}-{size}.jpg`. Images are + deduplicated by the SHA-256 of the downloaded bytes. ## Scheduler diff --git a/go.mod b/go.mod index 39a6ce9..c5a9dce 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( github.com/coreos/go-oidc/v3 v3.21.0 github.com/go-chi/chi/v5 v5.3.1 github.com/oklog/ulid/v2 v2.1.2 + golang.org/x/image v0.45.0 golang.org/x/oauth2 v0.36.0 modernc.org/sqlite v1.59.0 ) diff --git a/go.sum b/go.sum index ba3f090..c8f3175 100644 --- a/go.sum +++ b/go.sum @@ -21,6 +21,8 @@ github.com/oklog/ulid/v2 v2.1.2/go.mod h1:rcEKHmBBKfef9DhnvX7y1HZBYxjXb0cP5ExxNs github.com/pborman/getopt v0.0.0-20170112200414-7148bc3a4c30/go.mod h1:85jBQOZwpVEaDAr341tbn15RS4fCAsIst0qp7i8ex1o= github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec h1:W09IVJc94icq4NjY3clb7Lk8O1qJ8BdBEF8z0ibU0rE= github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec/go.mod h1:qqbHyh8v60DhA7CoWK5oRCqLrMHRGoxYCSS9EjAz6Eo= +golang.org/x/image v0.45.0 h1:FMb1nTbH5H9vF55SriQHgFw5GnNL9Jg6L25BwXKzhB0= +golang.org/x/image v0.45.0/go.mod h1:n62x/7RqlwXDvGsSU4u6IUTUf6KghUZ9Bt7cG/T9Fx4= golang.org/x/mod v0.38.0 h1:MECBjubtXD7yj4HrhIUcywNaGeNVUdfVnxmPajOk4yk= golang.org/x/mod v0.38.0/go.mod h1:V6Xz0pq8TQ3dGqVQ1FVHuelZpAL0uNhSkk9ogYP3c40= golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= diff --git a/internal/recipes/fetcher.go b/internal/recipes/fetcher.go index 5f7791a..e1860ca 100644 --- a/internal/recipes/fetcher.go +++ b/internal/recipes/fetcher.go @@ -8,10 +8,12 @@ import ( "errors" "fmt" "io" + "mime" "net" "net/http" "net/netip" "net/url" + "slices" "strings" "sync" "time" @@ -23,6 +25,8 @@ const ( // MaxBodyBytes caps a fetched body. A larger page is an error, never // silently truncated: a cut-off recipe page would parse as a wrong one. MaxBodyBytes = 2 << 20 + // MaxImageBytes caps a fetched image body, as MaxBodyBytes caps a page. + MaxImageBytes = 8 << 20 // MaxRedirects is how many redirects a fetch follows; each hop is // re-checked for scheme and address. MaxRedirects = 3 @@ -51,6 +55,25 @@ var ( ErrTooLarge = errors.New("recipes: body exceeds size cap") // ErrTimeout is returned when a fetch runs past Timeout. ErrTimeout = errors.New("recipes: fetch timed out") + // ErrNotImage is returned by FetchImage for a response whose + // Content-Type is not one of ImageContentTypes. + ErrNotImage = errors.New("recipes: response is not a supported image") +) + +// ImageContentTypes are the media types FetchImage accepts. +var ImageContentTypes = []string{"image/jpeg", "image/png", "image/webp", "image/gif"} + +// fetchKind is what a request asks for: its Accept header, the body cap and +// the media types allowed, where nil allows any. +type fetchKind struct { + accept string + maxBytes int64 + contentTypes []string +} + +var ( + pageKind = fetchKind{accept: "text/html,application/xhtml+xml", maxBytes: MaxBodyBytes} + imageKind = fetchKind{accept: strings.Join(ImageContentTypes, ","), maxBytes: MaxImageBytes, contentTypes: ImageContentTypes} ) // StatusError is returned for a final response whose status is not 2xx. @@ -163,27 +186,49 @@ func (f *Fetcher) Fetch(ctx context.Context, rawURL string) (Page, error) { if page, ok := f.cached(key); ok { return page, nil } - - fetchCtx, cancel := context.WithTimeout(ctx, f.timeout) - defer cancel() - page, err := f.get(fetchCtx, key) + page, err := f.fetch(ctx, key, pageKind) if err != nil { - if ctx.Err() == nil && errors.Is(fetchCtx.Err(), context.DeadlineExceeded) { - return Page{}, fmt.Errorf("%w after %s: %s", ErrTimeout, f.timeout, key) - } return Page{}, err } f.store(key, page) return page, nil } -func (f *Fetcher) get(ctx context.Context, rawURL string) (Page, error) { +// FetchImage returns the image at rawURL under the same safeguards as +// Fetch, but with a body cap of MaxImageBytes, and only for a response +// whose Content-Type is one of ImageContentTypes. Images are not cached. +func (f *Fetcher) FetchImage(ctx context.Context, rawURL string) (Page, error) { + u, err := url.Parse(rawURL) + if err != nil { + return Page{}, fmt.Errorf("recipes: parse URL: %w", err) + } + if err := checkURL(u); err != nil { + return Page{}, err + } + return f.fetch(ctx, u.String(), imageKind) +} + +// fetch gets rawURL as kind within the Fetcher's timeout. +func (f *Fetcher) fetch(ctx context.Context, rawURL string, kind fetchKind) (Page, error) { + fetchCtx, cancel := context.WithTimeout(ctx, f.timeout) + defer cancel() + page, err := f.get(fetchCtx, rawURL, kind) + if err != nil { + if ctx.Err() == nil && errors.Is(fetchCtx.Err(), context.DeadlineExceeded) { + return Page{}, fmt.Errorf("%w after %s: %s", ErrTimeout, f.timeout, rawURL) + } + return Page{}, err + } + return page, nil +} + +func (f *Fetcher) get(ctx context.Context, rawURL string, kind fetchKind) (Page, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, rawURL, nil) if err != nil { return Page{}, fmt.Errorf("recipes: build request: %w", err) } req.Header.Set("User-Agent", UserAgent) - req.Header.Set("Accept", "text/html,application/xhtml+xml") + req.Header.Set("Accept", kind.accept) resp, err := f.client.Do(req) if err != nil { return Page{}, err @@ -193,17 +238,24 @@ func (f *Fetcher) get(ctx context.Context, rawURL string) (Page, error) { if resp.StatusCode < 200 || resp.StatusCode > 299 { return Page{}, &StatusError{URL: final, Status: resp.StatusCode} } - if resp.ContentLength > MaxBodyBytes { + contentType := resp.Header.Get("Content-Type") + if kind.contentTypes != nil { + mediaType, _, err := mime.ParseMediaType(contentType) + if err != nil || !slices.Contains(kind.contentTypes, mediaType) { + return Page{}, fmt.Errorf("%w: %s has Content-Type %q", ErrNotImage, final, contentType) + } + } + if resp.ContentLength > kind.maxBytes { return Page{}, fmt.Errorf("%w: %s declares %d bytes", ErrTooLarge, final, resp.ContentLength) } - body, err := io.ReadAll(io.LimitReader(resp.Body, MaxBodyBytes+1)) + body, err := io.ReadAll(io.LimitReader(resp.Body, kind.maxBytes+1)) if err != nil { return Page{}, fmt.Errorf("recipes: read %s: %w", final, err) } - if len(body) > MaxBodyBytes { - return Page{}, fmt.Errorf("%w: %s is over %d bytes", ErrTooLarge, final, MaxBodyBytes) + if int64(len(body)) > kind.maxBytes { + return Page{}, fmt.Errorf("%w: %s is over %d bytes", ErrTooLarge, final, kind.maxBytes) } - return Page{URL: final, ContentType: resp.Header.Get("Content-Type"), Body: body}, nil + return Page{URL: final, ContentType: contentType, Body: body}, nil } // cached returns the page cached under key if it is younger than CacheTTL. diff --git a/internal/recipes/photo.go b/internal/recipes/photo.go new file mode 100644 index 0000000..50408e4 --- /dev/null +++ b/internal/recipes/photo.go @@ -0,0 +1,248 @@ +package recipes + +import ( + "bytes" + "context" + "crypto/sha256" + "database/sql" + "encoding/hex" + "errors" + "fmt" + "image" + "image/gif" + "image/jpeg" + "image/png" + "io/fs" + "math" + "os" + "path" + "path/filepath" + "slices" + "strconv" + "sync" + + "github.com/oklog/ulid/v2" + "golang.org/x/image/draw" + "golang.org/x/image/webp" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" +) + +const ( + // PhotoDir is where cached photos are stored, relative to the data + // directory. + PhotoDir = "media/photos" + // PhotoMinShortSide is the least a photo's shorter side may measure, in + // pixels. + PhotoMinShortSide = 300 + // PhotoMaxPixels bounds the pixels a photo may decode to, so a small + // compressed file cannot claim gigabytes of memory. + PhotoMaxPixels = 40_000_000 + // PhotoQuality is the JPEG quality of every stored size. + PhotoQuality = 82 +) + +// PhotoSizes are the long-side lengths, in pixels, each photo is stored at. +var PhotoSizes = []int{1200, 400} + +var ( + // ErrPhotoTooSmall is returned for an image whose shorter side is under + // PhotoMinShortSide. + ErrPhotoTooSmall = errors.New("recipes: photo is too small") + // ErrPhotoTooLarge is returned for an image over PhotoMaxPixels. + ErrPhotoTooLarge = errors.New("recipes: photo has too many pixels") + // ErrPhotoFormat is returned for bytes that are not a JPEG, PNG, WebP + // or GIF image. + ErrPhotoFormat = errors.New("recipes: photo is not a JPEG, PNG, WebP or GIF image") +) + +// photoDecoders are the formats a photo may be in. A GIF decodes to its +// first frame. +var photoDecoders = map[string]func([]byte) (image.Image, error){ + "jpeg": func(b []byte) (image.Image, error) { return jpeg.Decode(bytes.NewReader(b)) }, + "png": func(b []byte) (image.Image, error) { return png.Decode(bytes.NewReader(b)) }, + "gif": func(b []byte) (image.Image, error) { return gif.Decode(bytes.NewReader(b)) }, + "webp": func(b []byte) (image.Image, error) { return webp.Decode(bytes.NewReader(b)) }, +} + +// PhotoCache downloads photos through a Fetcher and stores each at every +// PhotoSizes length as {id}-{size}.jpg under PhotoDir, with a photos row. +// Photos are deduplicated by the SHA-256 of the downloaded bytes. A +// PhotoCache is safe for concurrent use; photos are processed one at a time. +type PhotoCache struct { + fetcher *Fetcher + store *store.Store + // dir is the absolute photo directory. + dir string + + // mu serialises the dedupe lookup, processing and insert, so one image + // is never stored twice. + mu sync.Mutex +} + +// NewPhotoCache returns a PhotoCache storing photos under dataDir, creating +// the photo directory if it is absent. +func NewPhotoCache(fetcher *Fetcher, st *store.Store, dataDir string) (*PhotoCache, error) { + if fetcher == nil { + return nil, errors.New("recipes: nil fetcher") + } + if st == nil { + return nil, errors.New("recipes: nil store") + } + dir, err := filepath.Abs(filepath.Join(dataDir, filepath.FromSlash(PhotoDir))) + if err != nil { + return nil, fmt.Errorf("recipes: resolve photo directory: %w", err) + } + if err := os.MkdirAll(dir, 0o750); err != nil { + return nil, fmt.Errorf("recipes: create photo directory: %w", err) + } + return &PhotoCache{fetcher: fetcher, store: st, dir: dir}, nil +} + +// CachePhoto downloads the image at rawURL and returns its photos row. An +// image whose bytes are already cached returns the existing row. Otherwise +// it is decoded, refused if its shorter side is under PhotoMinShortSide, +// scaled so its long side is each of PhotoSizes, encoded as JPEG and +// recorded. The row's width and height are the original image's, and its +// local_path is the largest size's file relative to the data directory. +func (c *PhotoCache) CachePhoto(ctx context.Context, rawURL string) (db.Photo, error) { + page, err := c.fetcher.FetchImage(ctx, rawURL) + if err != nil { + return db.Photo{}, err + } + digest := sha256.Sum256(page.Body) + sum := hex.EncodeToString(digest[:]) + + c.mu.Lock() + defer c.mu.Unlock() + existing, err := db.New(c.store.Reader).GetPhotoBySHA256(ctx, sum) + if err == nil { + return existing, nil + } + if !errors.Is(err, sql.ErrNoRows) { + return db.Photo{}, fmt.Errorf("recipes: look up photo: %w", err) + } + + img, err := decodePhoto(page.Body) + if err != nil { + return db.Photo{}, fmt.Errorf("%w: %s", err, page.URL) + } + id := store.NewID() + var written []string + removeWritten := func() error { + var errs []error + for _, p := range written { + errs = append(errs, os.Remove(p)) + } + return errors.Join(errs...) + } + for _, size := range PhotoSizes { + p := c.path(id, size) + if err := writeJPEG(p, scaleLongSide(img, size)); err != nil { + return db.Photo{}, errors.Join(err, removeWritten()) + } + written = append(written, p) + } + + now := store.Now().String() + bounds := img.Bounds() + photo, err := db.New(c.store.Writer).CreatePhoto(ctx, db.CreatePhotoParams{ + ID: id, + SourceUrl: page.URL, + LocalPath: path.Join(PhotoDir, photoFileName(id, PhotoSizes[0])), + Width: int64(bounds.Dx()), + Height: int64(bounds.Dy()), + Sha256: sum, + FetchedAt: now, + CreatedAt: now, + UpdatedAt: now, + }) + if err != nil { + return db.Photo{}, errors.Join(fmt.Errorf("recipes: record photo: %w", err), removeWritten()) + } + return photo, nil +} + +// OpenPhoto opens the stored file of photo id at size. An id that is not a +// canonical ULID, a size not in PhotoSizes, or a photo that is not stored +// is an error matching fs.ErrNotExist. +func (c *PhotoCache) OpenPhoto(id string, size int) (*os.File, error) { + parsed, err := ulid.ParseStrict(id) + if err != nil || parsed.String() != id || !slices.Contains(PhotoSizes, size) { + return nil, fmt.Errorf("recipes: photo %q at size %d: %w", id, size, fs.ErrNotExist) + } + return os.Open(c.path(id, size)) +} + +func (c *PhotoCache) path(id string, size int) string { + return filepath.Join(c.dir, photoFileName(id, size)) +} + +func photoFileName(id string, size int) string { + return id + "-" + strconv.Itoa(size) + ".jpg" +} + +// decodePhoto decodes a JPEG, PNG, WebP or GIF image, checking its size +// from the header before decoding the pixels. +func decodePhoto(b []byte) (image.Image, error) { + cfg, format, err := image.DecodeConfig(bytes.NewReader(b)) + if err != nil { + return nil, fmt.Errorf("%w: %w", ErrPhotoFormat, err) + } + decode, ok := photoDecoders[format] + if !ok { + return nil, fmt.Errorf("%w: format %q", ErrPhotoFormat, format) + } + if int64(cfg.Width)*int64(cfg.Height) > PhotoMaxPixels { + return nil, fmt.Errorf("%w: %dx%d", ErrPhotoTooLarge, cfg.Width, cfg.Height) + } + if min(cfg.Width, cfg.Height) < PhotoMinShortSide { + return nil, fmt.Errorf("%w: %dx%d, shorter side under %d px", ErrPhotoTooSmall, cfg.Width, cfg.Height, PhotoMinShortSide) + } + img, err := decode(b) + if err != nil { + return nil, fmt.Errorf("%w: %w", ErrPhotoFormat, err) + } + return img, nil +} + +// scaleLongSide returns img scaled so its longer side is long pixels, +// keeping its aspect ratio. Transparent areas become white, since JPEG has +// no alpha. +func scaleLongSide(img image.Image, long int) *image.RGBA { + b := img.Bounds() + w, h := b.Dx(), b.Dy() + dw, dh := long, long + if w >= h { + dh = max(1, int(math.Round(float64(h)*float64(long)/float64(w)))) + } else { + dw = max(1, int(math.Round(float64(w)*float64(long)/float64(h)))) + } + dst := image.NewRGBA(image.Rect(0, 0, dw, dh)) + draw.Draw(dst, dst.Bounds(), image.White, image.Point{}, draw.Src) + draw.CatmullRom.Scale(dst, dst.Bounds(), img, b, draw.Over, nil) + return dst +} + +// writeJPEG encodes img at PhotoQuality into a temporary file beside path +// and renames it into place, so a reader never sees a partial file. +func writeJPEG(path string, img image.Image) error { + f, err := os.CreateTemp(filepath.Dir(path), ".photo-*.tmp") + if err != nil { + return fmt.Errorf("recipes: create photo file: %w", err) + } + tmp := f.Name() + err = jpeg.Encode(f, img, &jpeg.Options{Quality: PhotoQuality}) + if err == nil { + err = f.Sync() + } + err = errors.Join(err, f.Close()) + if err == nil { + err = os.Rename(tmp, path) + } + if err != nil { + return errors.Join(fmt.Errorf("recipes: write photo %s: %w", filepath.Base(path), err), os.Remove(tmp)) + } + return nil +} diff --git a/internal/recipes/photo_test.go b/internal/recipes/photo_test.go new file mode 100644 index 0000000..f852a1e --- /dev/null +++ b/internal/recipes/photo_test.go @@ -0,0 +1,405 @@ +package recipes + +import ( + "bytes" + "context" + "errors" + "image" + "image/color" + "image/gif" + "image/jpeg" + "image/png" + "io/fs" + "net/http" + "net/url" + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations" +) + +// photoServer serves each path's body with its Content-Type and counts +// requests. +type photoServer struct { + files map[string]photoFile + requests atomic.Int32 +} + +type photoFile struct { + contentType string + body []byte +} + +func (s *photoServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { + s.requests.Add(1) + f, ok := s.files[r.URL.Path] + if !ok { + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", f.contentType) + w.Write(f.body) +} + +// newTestPhotoCache returns a PhotoCache over files, a migrated temp +// database and a temp data directory. +func newTestPhotoCache(t *testing.T, files map[string]photoFile) (*PhotoCache, *store.Store, string, *photoServer) { + t.Helper() + srv := &photoServer{files: files} + env := newTestEnv(t, srv, publicAnswers()) + installClock(env.f) + dataDir := t.TempDir() + st, err := store.Open(t.Context(), dataDir) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := st.Close(); err != nil { + t.Error(err) + } + }) + if err := store.Migrate(t.Context(), st.Writer, migrations.FS); err != nil { + t.Fatal(err) + } + cache, err := NewPhotoCache(env.f, st, dataDir) + if err != nil { + t.Fatal(err) + } + return cache, st, dataDir, srv +} + +// gradient is a w×h opaque image. +func gradient(w, h int) *image.RGBA { + img := image.NewRGBA(image.Rect(0, 0, w, h)) + for y := range h { + for x := range w { + img.Set(x, y, color.RGBA{uint8(x * 255 / w), uint8(y * 255 / h), 128, 255}) + } + } + return img +} + +func encodeJPEG(t *testing.T, img image.Image) []byte { + t.Helper() + var b bytes.Buffer + if err := jpeg.Encode(&b, img, nil); err != nil { + t.Fatal(err) + } + return b.Bytes() +} + +func encodePNG(t *testing.T, img image.Image) []byte { + t.Helper() + var b bytes.Buffer + if err := png.Encode(&b, img); err != nil { + t.Fatal(err) + } + return b.Bytes() +} + +// twoFrameGIF is a w×h animated GIF whose first frame is red and second +// blue. +func twoFrameGIF(t *testing.T, w, h int) []byte { + t.Helper() + frame := func(c color.Color) *image.Paletted { + p := image.NewPaletted(image.Rect(0, 0, w, h), color.Palette{c}) + return p + } + var b bytes.Buffer + anim := &gif.GIF{ + Image: []*image.Paletted{frame(color.RGBA{255, 0, 0, 255}), frame(color.RGBA{0, 0, 255, 255})}, + Delay: []int{10, 10}, + } + if err := gif.EncodeAll(&b, anim); err != nil { + t.Fatal(err) + } + return b.Bytes() +} + +func readTestdata(t *testing.T, name string) []byte { + t.Helper() + b, err := os.ReadFile(filepath.Join("testdata", name)) + if err != nil { + t.Fatal(err) + } + return b +} + +// storedSize decodes the stored JPEG of id at size and returns it. +func storedSize(t *testing.T, dataDir, id string, size int) image.Image { + t.Helper() + b, err := os.ReadFile(filepath.Join(dataDir, "media", "photos", photoFileName(id, size))) + if err != nil { + t.Fatalf("read stored photo: %v", err) + } + img, format, err := image.Decode(bytes.NewReader(b)) + if err != nil || format != "jpeg" { + t.Fatalf("stored photo is %q, %v; want a JPEG", format, err) + } + return img +} + +func TestCachePhotoFormatsAndSizes(t *testing.T) { + t.Parallel() + files := map[string]photoFile{ + "/landscape.jpg": {"image/jpeg", encodeJPEG(t, gradient(800, 600))}, + "/portrait.png": {"image/png", encodePNG(t, gradient(600, 900))}, + "/photo.webp": {"image/webp", readTestdata(t, "photo.webp")}, + "/anim.gif": {"image/gif; charset=binary", twoFrameGIF(t, 300, 450)}, + "/square.png": {"image/png", encodePNG(t, gradient(1500, 1500))}, + } + cache, _, dataDir, _ := newTestPhotoCache(t, files) + + tests := []struct { + path string + width, height int64 + // sizes maps each stored size to its expected dimensions. + sizes map[int]image.Point + }{ + {"/landscape.jpg", 800, 600, map[int]image.Point{1200: {1200, 900}, 400: {400, 300}}}, + {"/portrait.png", 600, 900, map[int]image.Point{1200: {800, 1200}, 400: {267, 400}}}, + {"/photo.webp", 640, 480, map[int]image.Point{1200: {1200, 900}, 400: {400, 300}}}, + {"/anim.gif", 300, 450, map[int]image.Point{1200: {800, 1200}, 400: {267, 400}}}, + {"/square.png", 1500, 1500, map[int]image.Point{1200: {1200, 1200}, 400: {400, 400}}}, + } + for _, tc := range tests { + photo, err := cache.CachePhoto(t.Context(), "https://example.com"+tc.path) + if err != nil { + t.Fatalf("CachePhoto %s: %v", tc.path, err) + } + if photo.Width != tc.width || photo.Height != tc.height { + t.Errorf("%s: row is %dx%d, want %dx%d", tc.path, photo.Width, photo.Height, tc.width, tc.height) + } + if photo.SourceUrl != "https://example.com"+tc.path || len(photo.Sha256) != 64 { + t.Errorf("%s: row = %+v", tc.path, photo) + } + if want := "media/photos/" + photo.ID + "-1200.jpg"; photo.LocalPath != want { + t.Errorf("%s: local_path = %q, want %q", tc.path, photo.LocalPath, want) + } + for size, want := range tc.sizes { + img := storedSize(t, dataDir, photo.ID, size) + if got := img.Bounds().Size(); got != want { + t.Errorf("%s at %d: stored %v, want %v", tc.path, size, got, want) + } + } + } +} + +func TestCachePhotoGIFUsesFirstFrame(t *testing.T) { + t.Parallel() + cache, _, dataDir, _ := newTestPhotoCache(t, map[string]photoFile{ + "/anim.gif": {"image/gif", twoFrameGIF(t, 400, 400)}, + }) + photo, err := cache.CachePhoto(t.Context(), "https://example.com/anim.gif") + if err != nil { + t.Fatalf("CachePhoto: %v", err) + } + r, g, b, _ := storedSize(t, dataDir, photo.ID, 400).At(200, 200).RGBA() + if r>>8 < 200 || g>>8 > 50 || b>>8 > 50 { + t.Errorf("centre pixel = (%d,%d,%d), want the first frame's red", r>>8, g>>8, b>>8) + } +} + +func TestCachePhotoFillsTransparencyWhite(t *testing.T) { + t.Parallel() + cache, _, dataDir, _ := newTestPhotoCache(t, map[string]photoFile{ + "/clear.png": {"image/png", encodePNG(t, image.NewNRGBA(image.Rect(0, 0, 400, 400)))}, + }) + photo, err := cache.CachePhoto(t.Context(), "https://example.com/clear.png") + if err != nil { + t.Fatalf("CachePhoto: %v", err) + } + r, g, b, _ := storedSize(t, dataDir, photo.ID, 400).At(200, 200).RGBA() + if r>>8 < 250 || g>>8 < 250 || b>>8 < 250 { + t.Errorf("transparent pixel stored as (%d,%d,%d), want white", r>>8, g>>8, b>>8) + } +} + +func countPhotos(t *testing.T, st *store.Store) int { + t.Helper() + var n int + if err := st.Reader.QueryRowContext(context.Background(), "SELECT count(*) FROM photos").Scan(&n); err != nil { + t.Fatal(err) + } + return n +} + +func photoFiles(t *testing.T, dataDir string) []string { + t.Helper() + entries, err := os.ReadDir(filepath.Join(dataDir, "media", "photos")) + if err != nil { + t.Fatal(err) + } + var names []string + for _, e := range entries { + names = append(names, e.Name()) + } + return names +} + +func TestCachePhotoRejectsTooSmall(t *testing.T) { + t.Parallel() + cache, st, dataDir, _ := newTestPhotoCache(t, map[string]photoFile{ + "/narrow.png": {"image/png", encodePNG(t, gradient(299, 1000))}, + "/short.jpg": {"image/jpeg", encodeJPEG(t, gradient(1000, 299))}, + "/edge.png": {"image/png", encodePNG(t, gradient(300, 1000))}, + }) + for _, p := range []string{"/narrow.png", "/short.jpg"} { + if _, err := cache.CachePhoto(t.Context(), "https://example.com"+p); !errors.Is(err, ErrPhotoTooSmall) { + t.Errorf("CachePhoto %s error = %v, want ErrPhotoTooSmall", p, err) + } + } + if n := countPhotos(t, st); n != 0 { + t.Errorf("%d photos recorded for refused images", n) + } + if names := photoFiles(t, dataDir); len(names) != 0 { + t.Errorf("files stored for refused images: %v", names) + } + if _, err := cache.CachePhoto(t.Context(), "https://example.com/edge.png"); err != nil { + t.Errorf("CachePhoto with a 300 px short side: %v", err) + } +} + +func TestCachePhotoRejectsNonImages(t *testing.T) { + t.Parallel() + cache, st, _, _ := newTestPhotoCache(t, map[string]photoFile{ + "/page.html": {"text/html", encodePNG(t, gradient(400, 400))}, + "/fake.jpg": {"image/jpeg", []byte("not a picture")}, + }) + if _, err := cache.CachePhoto(t.Context(), "https://example.com/page.html"); !errors.Is(err, ErrNotImage) { + t.Errorf("text/html error = %v, want ErrNotImage", err) + } + if _, err := cache.CachePhoto(t.Context(), "https://example.com/fake.jpg"); !errors.Is(err, ErrPhotoFormat) { + t.Errorf("undecodable image error = %v, want ErrPhotoFormat", err) + } + if n := countPhotos(t, st); n != 0 { + t.Errorf("%d photos recorded", n) + } +} + +func TestCachePhotoDeduplicatesBySHA256(t *testing.T) { + t.Parallel() + body := encodeJPEG(t, gradient(640, 480)) + cache, st, dataDir, srv := newTestPhotoCache(t, map[string]photoFile{ + "/a.jpg": {"image/jpeg", body}, + "/b.jpg": {"image/jpeg", body}, + "/other.jpg": {"image/jpeg", encodeJPEG(t, gradient(480, 640))}, + }) + first, err := cache.CachePhoto(t.Context(), "https://example.com/a.jpg") + if err != nil { + t.Fatalf("CachePhoto a: %v", err) + } + again, err := cache.CachePhoto(t.Context(), "https://example.com/b.jpg") + if err != nil { + t.Fatalf("CachePhoto b: %v", err) + } + if again != first { + t.Errorf("same bytes at another URL = %+v, want the first row %+v", again, first) + } + if n := srv.requests.Load(); n != 2 { + t.Errorf("%d downloads, want 2: each URL is fetched to hash it", n) + } + other, err := cache.CachePhoto(t.Context(), "https://example.com/other.jpg") + if err != nil { + t.Fatalf("CachePhoto other: %v", err) + } + if other.ID == first.ID || other.Sha256 == first.Sha256 { + t.Errorf("different bytes share a photo: %+v and %+v", first, other) + } + if n := countPhotos(t, st); n != 2 { + t.Errorf("%d photos recorded, want 2", n) + } + if names := photoFiles(t, dataDir); len(names) != 4 { + t.Errorf("stored files %v, want two sizes of two photos", names) + } +} + +func TestOpenPhoto(t *testing.T) { + t.Parallel() + cache, _, _, _ := newTestPhotoCache(t, map[string]photoFile{ + "/a.png": {"image/png", encodePNG(t, gradient(400, 300))}, + }) + photo, err := cache.CachePhoto(t.Context(), "https://example.com/a.png") + if err != nil { + t.Fatalf("CachePhoto: %v", err) + } + for _, size := range PhotoSizes { + f, err := cache.OpenPhoto(photo.ID, size) + if err != nil { + t.Fatalf("OpenPhoto %d: %v", size, err) + } + f.Close() + } + for _, tc := range []struct { + id string + size int + }{ + {photo.ID, 800}, + {store.NewID(), 400}, + {strings.ToLower(photo.ID), 400}, + {"../" + photo.ID, 400}, + {"", 400}, + } { + if _, err := cache.OpenPhoto(tc.id, tc.size); !errors.Is(err, fs.ErrNotExist) { + t.Errorf("OpenPhoto(%q, %d) error = %v, want fs.ErrNotExist", tc.id, tc.size, err) + } + } +} + +func TestFetchImageSizeCap(t *testing.T) { + t.Parallel() + env := newTestEnv(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + size := MaxImageBytes + if r.URL.Path != "/exact" { + size++ + } + w.Header().Set("Content-Type", "image/jpeg") + w.Write(bytes.Repeat([]byte("a"), size)) + }), publicAnswers()) + installClock(env.f) + + page, err := env.f.FetchImage(t.Context(), "https://example.com/exact") + if err != nil { + t.Fatalf("FetchImage at the cap: %v", err) + } + if len(page.Body) != MaxImageBytes { + t.Errorf("body is %d bytes, want %d", len(page.Body), MaxImageBytes) + } + if _, err := env.f.FetchImage(t.Context(), "https://example.com/over"); !errors.Is(err, ErrTooLarge) { + t.Errorf("FetchImage over the cap error = %v, want ErrTooLarge", err) + } +} + +func TestFetchImageContentTypes(t *testing.T) { + t.Parallel() + var accept atomic.Value + env := newTestEnv(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + accept.Store(r.Header.Get("Accept")) + w.Header().Set("Content-Type", r.URL.Query().Get("type")) + w.Write([]byte("x")) + }), publicAnswers()) + installClock(env.f) + typeURL := func(ct string) string { return "https://example.com/?type=" + url.QueryEscape(ct) } + + for _, ct := range []string{"image/jpeg", "image/png", "image/webp", "image/gif", "IMAGE/PNG; q=1"} { + if _, err := env.f.FetchImage(t.Context(), typeURL(ct)); err != nil { + t.Errorf("FetchImage %q: %v", ct, err) + } + } + if got := accept.Load(); got != "image/jpeg,image/png,image/webp,image/gif" { + t.Errorf("Accept = %q", got) + } + for _, ct := range []string{"text/html", "image/svg+xml", "image/avif", "application/octet-stream", ""} { + if _, err := env.f.FetchImage(t.Context(), typeURL(ct)); !errors.Is(err, ErrNotImage) { + t.Errorf("FetchImage %q error = %v, want ErrNotImage", ct, err) + } + } + // Pages still fetch whatever the Content-Type. + if _, err := env.f.Fetch(t.Context(), typeURL("text/plain")); err != nil { + t.Errorf("Fetch: %v", err) + } +} diff --git a/internal/recipes/testdata/photo.webp b/internal/recipes/testdata/photo.webp new file mode 100644 index 0000000000000000000000000000000000000000..285ba6ed6489eea061c2128b7a2d1fd9dfd8dd13 GIT binary patch literal 2304 zcmZ8fc{~$r9G_#ka+Z*zEF_eYqa1V92-8?XQjWbfVj8b&uWNWUA$f}wF<0h_TxH~b z-LH5ZQ_D5AF~^ddX0M}qtN)(Q^L>8b=X?CDFF+ux+yH>>c_Y|m*qOsKTj!E1fTwIq zVeDgU;9g`2Tv0wd6VJ^CILPCM8b759U@_n40w|qcO|08T>QuQ5>R5%r;h`g4DQ}M6 z=AdMu1xh_jL=hjf%Avx8&`S2g6c3U>v2zeWCtl%rz)H{tOkWRb# zZvz>vUnXB|W9MMo*Mir?+68+co3hn!tU_PYTXDVt{KA8l#u&+(C5Umec6(*L%>&P{ z0y)_2^qM76eN5^V{d2zCHwt97%g{ie{aOA=)I$HI??0wx=8kY3h~Lqo^lg0mu_@lf zUUP_dM>yGO3+muoNy~iZhq=4M97tCR`OX7lXXu^3IPwcb4TNt^h>4@oI+)N+ZKSs$NbtzKDsc$;NGr@j*P8U*(I_%ZE(ai`3!r zR)N{K_M+{RlV6Z;J)sR}Ar9!9jmrw4qVA2^F;%HI%8yze5KF&@<$57Q(PbLW6&tR*i$7CaW!bU8(o9!}%+{D@bsWJn}Zy$lC^%!PdKy|2uu4 zKtSePA6Y$@7LZe$AMeC_V_4ACZx}Tc*c4=fdmry%^r*KUYQQhJhve?x?+$F0n2ud? z=mtKDq)RwkxabA1=!1kE7y`ML)e*-ab|XfY$5-@5GHZ~f&ogFeBt?d|;*9+0L)hS$ z6vUe^VIb2_=XPoJ-N|&%*t%XrHLY$aZa*=NP8)QnbBwXzA_@$(iz0yQ`PRrBDz8^x z-dVYKB@47T)7p%jbu-g8{Fr=;ZhHI5AwP?tK1FENb+P7aK=(jW>pM1(3@>r zxn0WCL~M)G&A~FM=Rir2TAIN?W_M|PvvXxtba8m`y15Rs#Kni%HA|?O{m|v8lGB8_ zB|Wj=EX8hRabdxuqi7UfgB@2i%)KTcSU%wTk{i&ha+Yz=W5Yb6;n_ZhO><}12G2*m z@I04+2ai+lH&oFi$3368f$JL5=CqOWPQ2M)L zHd&hsH>meAI#*j1(SjI9=YuDd5(tZk@--YLg*3+(%n0eROJ-NcI_`VNv>EyRJj%0) zp%BR`$N@T?LoF>6o73{a{|q^QV&STi1<;Z91t1UW8$jHBI8{O&7-FpWeL zbcv>Te1wp}T-yU_&Ba88@}toB%`MrB7u#dAi`cgkDD4w5{;Si z_ME!St^}}`n=X=vWX<)rMS5g9=RjUVLJz@Cen{vkBG0Wt-@-D(K6^4d^tl?zqmh7g zT@~rnfv2k8cZ2bZ%iit=X_l5IANy>X7~B$oCsO&9jHyRP)(oZY`4`b()Qq5f#}Vlb z?LMh%{hZ~#nEOuY>z>jVQ!yCW#!&}zE7waMfo1zW`b@+<52+{}Mr@)7UJ_cCWd1d^ z7>fHy@f4%30Q!466Ghv__$@&;tpNnoS;9-;%LKBoH3!F|lFlUssX zDUJ#z#IA|Q$`LX^ZHEYBGmF_iSbeiOFBsa5k`Etr{j~vQ5ExV`ju$m#1I5 z@)^Zp8H*#nQO>gBw}iE_)Il00Ghg;Xn>wH5(cg={p+F-yQe$c&+&T83)7jQ)mF)$t z|9$zl{wd@q+*@YS^m*4+lfA!DLy3*XX#<8RtLvr0y!}-=(hI2J0DK|0~A5N-2{&K2;1JA8F8*OnWLc@*Yc(t32 iSkURusuv|&)kY~}{Z%odfo+<{xU3RSMo$p{fd2r$6O|qS literal 0 HcmV?d00001 diff --git a/internal/server/login_test.go b/internal/server/login_test.go index 88b60a5..16e83d7 100644 --- a/internal/server/login_test.go +++ b/internal/server/login_test.go @@ -83,7 +83,7 @@ func newLoginRouter(t *testing.T, issuer string) http.Handler { t.Fatal(err) } r, err := NewRouter(logger, "dev", trusted, flow, sessions, newTestSettings(t), - newTestRunner(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) } diff --git a/internal/server/media.go b/internal/server/media.go new file mode 100644 index 0000000..609ec06 --- /dev/null +++ b/internal/server/media.go @@ -0,0 +1,74 @@ +package server + +import ( + "errors" + "io/fs" + "log/slog" + "net/http" + "os" + "strconv" + "strings" + + "github.com/go-chi/chi/v5" +) + +// photoCacheControl lets the browser keep a photo for a year without +// revalidating: a photo's files never change once stored, and a new image +// gets a new id. +const photoCacheControl = "private, max-age=31536000, immutable" + +// PhotoFiles opens a cached photo's file at one of its stored sizes. An +// unknown photo or size is an error matching fs.ErrNotExist. +type PhotoFiles interface { + OpenPhoto(id string, size int) (*os.File, error) +} + +// getPhoto answers GET /media/photos/{id}-{size}.jpg with the stored JPEG. +func getPhoto(logger *slog.Logger, photos PhotoFiles) http.HandlerFunc { + notFound := apiErrorHandler(logger, http.StatusNotFound, "not_found", "not found") + return func(w http.ResponseWriter, r *http.Request) { + id, size, ok := parsePhotoName(chi.URLParam(r, "name")) + if !ok { + notFound(w, r) + return + } + f, err := photos.OpenPhoto(id, size) + if errors.Is(err, fs.ErrNotExist) { + notFound(w, r) + return + } + if err != nil { + logger.ErrorContext(r.Context(), "open photo", "id", id, "size", size, "error", err) + writeError(w, r, logger, http.StatusInternalServerError, "internal", "internal error") + return + } + defer f.Close() + info, err := f.Stat() + if err != nil { + logger.ErrorContext(r.Context(), "stat photo", "id", id, "size", size, "error", err) + writeError(w, r, logger, http.StatusInternalServerError, "internal", "internal error") + return + } + w.Header().Set("Content-Type", "image/jpeg") + w.Header().Set("Cache-Control", photoCacheControl) + http.ServeContent(w, r, "", info.ModTime(), f) + } +} + +// parsePhotoName splits "{id}-{size}.jpg" into its id and size, refusing +// a size not written in its canonical decimal form. +func parsePhotoName(name string) (id string, size int, ok bool) { + stem, ok := strings.CutSuffix(name, ".jpg") + if !ok { + return "", 0, false + } + i := strings.LastIndexByte(stem, '-') + if i < 0 { + return "", 0, false + } + size, err := strconv.Atoi(stem[i+1:]) + if err != nil || strconv.Itoa(size) != stem[i+1:] { + return "", 0, false + } + return stem[:i], size, true +} diff --git a/internal/server/media_test.go b/internal/server/media_test.go new file mode 100644 index 0000000..1785d0f --- /dev/null +++ b/internal/server/media_test.go @@ -0,0 +1,117 @@ +package server + +import ( + "io" + "log/slog" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + "testing/fstest" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" +) + +// newMediaRouter returns a router over a photo cache whose data directory +// holds both sizes of one stored photo, and that photo's id. Each size's +// file holds its size as text. +func newMediaRouter(t *testing.T) (http.Handler, string) { + t.Helper() + dataDir := t.TempDir() + photos, err := recipes.NewPhotoCache(recipes.NewFetcher(), newTestStore(t), dataDir) + if err != nil { + t.Fatal(err) + } + id := store.NewID() + for _, size := range []string{"400", "1200"} { + p := filepath.Join(dataDir, "media", "photos", id+"-"+size+".jpg") + if err := os.WriteFile(p, []byte("jpeg "+size), 0o600); err != nil { + t.Fatal(err) + } + } + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + r, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestRunner(t), + photos, fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + if err != nil { + t.Fatal(err) + } + return r, id +} + +func TestGetPhotoServesEachSize(t *testing.T) { + t.Parallel() + router, id := newMediaRouter(t) + for _, size := range []string{"400", "1200"} { + req := httptest.NewRequest(http.MethodGet, "/media/photos/"+id+"-"+size+".jpg", nil) + req.AddCookie(validSessionCookie) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("size %s: status = %d, want 200", size, rec.Code) + } + if got := rec.Body.String(); got != "jpeg "+size { + t.Errorf("size %s: body = %q", size, got) + } + if got := rec.Header().Get("Content-Type"); got != "image/jpeg" { + t.Errorf("size %s: Content-Type = %q, want image/jpeg", size, got) + } + if got := rec.Header().Get("Cache-Control"); got != "private, max-age=31536000, immutable" { + t.Errorf("size %s: Cache-Control = %q", size, got) + } + } +} + +func TestGetPhotoRequiresSession(t *testing.T) { + t.Parallel() + router, id := newMediaRouter(t) + req := httptest.NewRequest(http.MethodGet, "/media/photos/"+id+"-400.jpg", nil) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusUnauthorized { + t.Fatalf("status = %d, want 401", rec.Code) + } + if got := rec.Body.String(); got != unauthenticatedJSON { + t.Errorf("body = %s, want %s", got, unauthenticatedJSON) + } + if got := rec.Header().Get("Cache-Control"); got != "" { + t.Errorf("Cache-Control = %q on a refused request", got) + } +} + +func TestGetPhotoUnknownNotFound(t *testing.T) { + t.Parallel() + router, id := newMediaRouter(t) + for _, name := range []string{ + store.NewID() + "-400.jpg", // no such photo + id + "-800.jpg", // size not stored + id + "-0400.jpg", + id + "-+400.jpg", + id + "-400.png", + id + ".jpg", + id + "-400", + "..%2F" + id + "-400.jpg", + "not-an-id-400.jpg", + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + req := httptest.NewRequest(http.MethodGet, "/media/photos/"+name, nil) + req.AddCookie(validSessionCookie) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + + if rec.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404", rec.Code) + } + if got := rec.Body.String(); got != notFoundJSON { + t.Errorf("body = %s, want %s", got, notFoundJSON) + } + if got := rec.Header().Get("Cache-Control"); got != "" { + t.Errorf("Cache-Control = %q on a 404", got) + } + }) + } +} diff --git a/internal/server/origin_test.go b/internal/server/origin_test.go index b686164..bc235b9 100644 --- a/internal/server/origin_test.go +++ b/internal/server/origin_test.go @@ -45,7 +45,7 @@ func newOriginRouter(t *testing.T, logs io.Writer) http.Handler { } logger := slog.New(forwarded.LogHandler(slog.NewJSONHandler(logs, nil))) r, err := NewRouter(logger, "dev", trusted, probeAuth{}, stubSessions{}, newTestSettings(t), - newTestRunner(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) } diff --git a/internal/server/server.go b/internal/server/server.go index b1f3049..8439dd0 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -30,16 +30,20 @@ type RouteMounter interface { // only from a peer in trusted (see resolveOrigin). version is reported by // GET /health; auth mounts the /auth routes; sessions guards /api/v1 and // /media; settings serves /api/v1/settings/household; runner serves -// POST /api/v1/jobs and GET /api/v1/jobs/{id}; assets is the web build +// POST /api/v1/jobs and GET /api/v1/jobs/{id}; photos serves +// GET /media/photos/{id}-{size}.jpg; assets is the web build // served for every other GET. /health, /auth and the web build are public. // Requests to /api/v1 other than GET also need the session's CSRF token in CSRFHeader. -func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, auth RouteMounter, sessions SessionLookup, settings *household.SettingsService, runner *jobs.Runner, assets fs.FS) (http.Handler, error) { +func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, auth RouteMounter, sessions SessionLookup, settings *household.SettingsService, runner *jobs.Runner, photos PhotoFiles, assets fs.FS) (http.Handler, error) { if settings == nil { return nil, errors.New("server: nil household settings service") } if runner == nil { return nil, errors.New("server: nil job runner") } + if photos == nil { + return nil, errors.New("server: nil photo files") + } notFound := apiErrorHandler(logger, http.StatusNotFound, "not_found", "not found") methodNotAllowed := apiErrorHandler(logger, http.StatusMethodNotAllowed, "method_not_allowed", "method not allowed") @@ -70,6 +74,7 @@ func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, a apiV1.Patch("/settings/household", patchHouseholdSettings(logger, settings)) apiV1.Post("/jobs", postJob(logger, runner)) apiV1.Get("/jobs/{id}", getJob(logger, runner)) + media.Get("/photos/{name}", getPhoto(logger, photos)) r.Route("/api", func(r chi.Router) { r.MethodNotAllowed(methodNotAllowed) r.NotFound(notFound) diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 136d9f8..9710b4c 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -16,6 +16,7 @@ import ( "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/jobs" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations" ) @@ -38,7 +39,7 @@ func newTestRouterWith(t *testing.T, version string, settings *household.Setting "index.html": {Data: []byte(fixtureIndex)}, "assets/index-AbC123.js": {Data: []byte(fixtureAsset)}, } - r, err := NewRouter(logger, version, nil, stubAuth{}, stubSessions{}, settings, runner, assets) + r, err := NewRouter(logger, version, nil, stubAuth{}, stubSessions{}, settings, runner, newTestPhotos(t), assets) if err != nil { t.Fatal(err) } @@ -112,11 +113,31 @@ func newTestRunner(t *testing.T) *jobs.Runner { return runner } +// newTestPhotos returns a photo cache over a migrated temp database and an +// empty temp data directory. +func newTestPhotos(t *testing.T) *recipes.PhotoCache { + t.Helper() + photos, err := recipes.NewPhotoCache(recipes.NewFetcher(), newTestStore(t), t.TempDir()) + if err != nil { + t.Fatal(err) + } + return photos +} + +func TestNewRouterRequiresPhotoFiles(t *testing.T) { + t.Parallel() + + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestRunner(t), nil, fstest.MapFS{"index.html": {}}); err == nil { + t.Fatal("NewRouter accepted nil photo files") + } +} + func TestNewRouterRequiresSettings(t *testing.T) { t.Parallel() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, nil, newTestRunner(t), fstest.MapFS{"index.html": {}}); err == nil { + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, nil, newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { t.Fatal("NewRouter accepted a nil settings service") } } @@ -125,7 +146,7 @@ func TestNewRouterRequiresJobRunner(t *testing.T) { t.Parallel() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), nil, fstest.MapFS{"index.html": {}}); err == nil { + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), nil, newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { t.Fatal("NewRouter accepted a nil job runner") } } diff --git a/internal/server/settings_test.go b/internal/server/settings_test.go index e9da996..a0fe87c 100644 --- a/internal/server/settings_test.go +++ b/internal/server/settings_test.go @@ -216,7 +216,7 @@ func TestHouseholdSettingsWithRealSession(t *testing.T) { } logger := slog.New(slog.NewTextHandler(io.Discard, nil)) router, err := NewRouter(logger, "dev", nil, stubAuth{}, sessions, household.NewSettingsService(st), - newTestRunner(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) } -- 2.55.0 From 70f545152affd5dfeff3176492b76a6ffcc621b4 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Mon, 5 Oct 2026 14:10:54 -0400 Subject: [PATCH 2/2] Validate decoded photo size and require a photos row to serve decodePhoto checked the GIF logical screen size, but gif.Decode returns the first frame, which may be smaller; a 400x400 screen with a 1x1 first frame passed and was scaled. The short-side check now measures the decoded image's bounds. The pixel cap still comes from the header, before decoding. OpenPhoto served any file whose name was a canonical ULID and stored size, with no photos row required. It now looks the id up with the new GetPhoto query and reports an id with no row as fs.ErrNotExist, so GET /media/photos/{id}-{size}.jpg is 404 for it even when a file exists. OpenPhoto takes the request context for the lookup. The media test fixture records a photos row for its served photo and adds an orphan photo whose files exist without a row; both its sizes must be 404. Maestro task: 8a432108-d3ba-4013-ae8c-11963981d641 Co-Authored-By: Claude Opus 5.5 --- docs/ARCHITECTURE.md | 7 ++--- internal/db/photos.sql.go | 22 +++++++++++++++ internal/db/querier.go | 1 + internal/recipes/photo.go | 22 +++++++++------ internal/recipes/photo_test.go | 39 ++++++++++++++++++++++++--- internal/server/media.go | 5 ++-- internal/server/media_test.go | 49 ++++++++++++++++++++++++---------- migrations/queries/photos.sql | 4 +++ 8 files changed, 119 insertions(+), 30 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 1787868..206a29a 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -141,7 +141,8 @@ automatic** (D-021). `X-MealQuest-CSRF` header. - `/auth/*` — the OIDC flow. - `/media/*` — cached photos, behind the session cookie. - `GET /media/photos/{id}-{400|1200}.jpg` serves a stored size with + `GET /media/photos/{id}-{400|1200}.jpg` serves a stored size of a photo + with a `photos` row, with `Cache-Control: private, max-age=31536000, immutable`; anything else is 404. - `/health` — liveness. @@ -269,8 +270,8 @@ automatic** (D-021). - Photos are cached locally (`photos`) and processed with `golang.org/x/image`. `recipes.PhotoCache.CachePhoto` downloads through the Fetcher's `FetchImage` (JPEG, PNG, WebP or GIF content types, 8 MB - cap, not cached), decodes the image (a GIF's first frame), refuses one - under 300 px on its short side, scales it so its long side is 1200 px and + cap, not cached), decodes the image (a GIF's first frame), refuses a + decoded image under 300 px on its short side, scales it so its long side is 1200 px and 400 px, and stores each as JPEG quality 82 at `$MEALQUEST_DATA_DIR/media/photos/{id}-{size}.jpg`. Images are deduplicated by the SHA-256 of the downloaded bytes. diff --git a/internal/db/photos.sql.go b/internal/db/photos.sql.go index 884ba4d..9f799a7 100644 --- a/internal/db/photos.sql.go +++ b/internal/db/photos.sql.go @@ -59,6 +59,28 @@ func (q *Queries) CreatePhoto(ctx context.Context, arg CreatePhotoParams) (Photo return i, err } +const getPhoto = `-- name: GetPhoto :one +SELECT id, source_url, local_path, width, height, sha256, fetched_at, created_at, updated_at FROM photos +WHERE id = ? +` + +func (q *Queries) GetPhoto(ctx context.Context, id string) (Photo, error) { + row := q.db.QueryRowContext(ctx, getPhoto, id) + var i Photo + err := row.Scan( + &i.ID, + &i.SourceUrl, + &i.LocalPath, + &i.Width, + &i.Height, + &i.Sha256, + &i.FetchedAt, + &i.CreatedAt, + &i.UpdatedAt, + ) + return i, err +} + const getPhotoBySHA256 = `-- name: GetPhotoBySHA256 :one SELECT id, source_url, local_path, width, height, sha256, fetched_at, created_at, updated_at FROM photos WHERE sha256 = ? diff --git a/internal/db/querier.go b/internal/db/querier.go index 825889c..4afff6a 100644 --- a/internal/db/querier.go +++ b/internal/db/querier.go @@ -43,6 +43,7 @@ type Querier interface { // Returns sql.ErrNoRows until the row exists. GetIntegrationsGoogle(ctx context.Context) (IntegrationsGoogle, error) GetJob(ctx context.Context, id string) (Job, error) + GetPhoto(ctx context.Context, id string) (Photo, error) // Finds stored image bytes by content hash. The schema does not make sha256 // unique, so the earliest-created (lowest id) match is returned. GetPhotoBySHA256(ctx context.Context, sha256 string) (Photo, error) diff --git a/internal/recipes/photo.go b/internal/recipes/photo.go index 50408e4..107c00d 100644 --- a/internal/recipes/photo.go +++ b/internal/recipes/photo.go @@ -165,13 +165,18 @@ func (c *PhotoCache) CachePhoto(ctx context.Context, rawURL string) (db.Photo, e } // OpenPhoto opens the stored file of photo id at size. An id that is not a -// canonical ULID, a size not in PhotoSizes, or a photo that is not stored -// is an error matching fs.ErrNotExist. -func (c *PhotoCache) OpenPhoto(id string, size int) (*os.File, error) { +// canonical ULID, a size not in PhotoSizes, an id with no photos row, or a +// photo whose file is not stored is an error matching fs.ErrNotExist. +func (c *PhotoCache) OpenPhoto(ctx context.Context, id string, size int) (*os.File, error) { parsed, err := ulid.ParseStrict(id) if err != nil || parsed.String() != id || !slices.Contains(PhotoSizes, size) { return nil, fmt.Errorf("recipes: photo %q at size %d: %w", id, size, fs.ErrNotExist) } + if _, err := db.New(c.store.Reader).GetPhoto(ctx, id); errors.Is(err, sql.ErrNoRows) { + return nil, fmt.Errorf("recipes: photo %q: %w", id, fs.ErrNotExist) + } else if err != nil { + return nil, fmt.Errorf("recipes: look up photo %q: %w", id, err) + } return os.Open(c.path(id, size)) } @@ -183,8 +188,9 @@ func photoFileName(id string, size int) string { return id + "-" + strconv.Itoa(size) + ".jpg" } -// decodePhoto decodes a JPEG, PNG, WebP or GIF image, checking its size -// from the header before decoding the pixels. +// decodePhoto decodes a JPEG, PNG, WebP or GIF image, checking its pixel +// count from the header before decoding the pixels, and the shorter side of +// the decoded image: a GIF's first frame may be smaller than its screen. func decodePhoto(b []byte) (image.Image, error) { cfg, format, err := image.DecodeConfig(bytes.NewReader(b)) if err != nil { @@ -197,13 +203,13 @@ func decodePhoto(b []byte) (image.Image, error) { if int64(cfg.Width)*int64(cfg.Height) > PhotoMaxPixels { return nil, fmt.Errorf("%w: %dx%d", ErrPhotoTooLarge, cfg.Width, cfg.Height) } - if min(cfg.Width, cfg.Height) < PhotoMinShortSide { - return nil, fmt.Errorf("%w: %dx%d, shorter side under %d px", ErrPhotoTooSmall, cfg.Width, cfg.Height, PhotoMinShortSide) - } img, err := decode(b) if err != nil { return nil, fmt.Errorf("%w: %w", ErrPhotoFormat, err) } + if size := img.Bounds().Size(); min(size.X, size.Y) < PhotoMinShortSide { + return nil, fmt.Errorf("%w: %dx%d, shorter side under %d px", ErrPhotoTooSmall, size.X, size.Y, PhotoMinShortSide) + } return img, nil } diff --git a/internal/recipes/photo_test.go b/internal/recipes/photo_test.go index f852a1e..a8c08bd 100644 --- a/internal/recipes/photo_test.go +++ b/internal/recipes/photo_test.go @@ -204,6 +204,33 @@ func TestCachePhotoGIFUsesFirstFrame(t *testing.T) { } } +func TestCachePhotoRejectsSmallGIFFirstFrame(t *testing.T) { + t.Parallel() + // A 400x400 screen whose first frame is 1x1: the frame is what would be + // stored, so it is what must measure PhotoMinShortSide. + var b bytes.Buffer + anim := &gif.GIF{ + Image: []*image.Paletted{image.NewPaletted(image.Rect(0, 0, 1, 1), color.Palette{color.Black})}, + Delay: []int{10}, + Config: image.Config{ColorModel: color.Palette{color.Black}, Width: 400, Height: 400}, + } + if err := gif.EncodeAll(&b, anim); err != nil { + t.Fatal(err) + } + cache, st, dataDir, _ := newTestPhotoCache(t, map[string]photoFile{ + "/speck.gif": {"image/gif", b.Bytes()}, + }) + if _, err := cache.CachePhoto(t.Context(), "https://example.com/speck.gif"); !errors.Is(err, ErrPhotoTooSmall) { + t.Errorf("CachePhoto error = %v, want ErrPhotoTooSmall", err) + } + if n := countPhotos(t, st); n != 0 { + t.Errorf("%d photos recorded for a refused image", n) + } + if names := photoFiles(t, dataDir); len(names) != 0 { + t.Errorf("files stored for a refused image: %v", names) + } +} + func TestCachePhotoFillsTransparencyWhite(t *testing.T) { t.Parallel() cache, _, dataDir, _ := newTestPhotoCache(t, map[string]photoFile{ @@ -320,7 +347,7 @@ func TestCachePhotoDeduplicatesBySHA256(t *testing.T) { func TestOpenPhoto(t *testing.T) { t.Parallel() - cache, _, _, _ := newTestPhotoCache(t, map[string]photoFile{ + cache, _, dataDir, _ := newTestPhotoCache(t, map[string]photoFile{ "/a.png": {"image/png", encodePNG(t, gradient(400, 300))}, }) photo, err := cache.CachePhoto(t.Context(), "https://example.com/a.png") @@ -328,23 +355,29 @@ func TestOpenPhoto(t *testing.T) { t.Fatalf("CachePhoto: %v", err) } for _, size := range PhotoSizes { - f, err := cache.OpenPhoto(photo.ID, size) + f, err := cache.OpenPhoto(t.Context(), photo.ID, size) if err != nil { t.Fatalf("OpenPhoto %d: %v", size, err) } f.Close() } + // A file with no photos row is not a stored photo. + orphan := store.NewID() + if err := os.WriteFile(filepath.Join(dataDir, "media", "photos", orphan+"-400.jpg"), []byte("jpeg"), 0o600); err != nil { + t.Fatal(err) + } for _, tc := range []struct { id string size int }{ {photo.ID, 800}, {store.NewID(), 400}, + {orphan, 400}, {strings.ToLower(photo.ID), 400}, {"../" + photo.ID, 400}, {"", 400}, } { - if _, err := cache.OpenPhoto(tc.id, tc.size); !errors.Is(err, fs.ErrNotExist) { + if _, err := cache.OpenPhoto(t.Context(), tc.id, tc.size); !errors.Is(err, fs.ErrNotExist) { t.Errorf("OpenPhoto(%q, %d) error = %v, want fs.ErrNotExist", tc.id, tc.size, err) } } diff --git a/internal/server/media.go b/internal/server/media.go index 609ec06..68fee0d 100644 --- a/internal/server/media.go +++ b/internal/server/media.go @@ -1,6 +1,7 @@ package server import ( + "context" "errors" "io/fs" "log/slog" @@ -20,7 +21,7 @@ const photoCacheControl = "private, max-age=31536000, immutable" // PhotoFiles opens a cached photo's file at one of its stored sizes. An // unknown photo or size is an error matching fs.ErrNotExist. type PhotoFiles interface { - OpenPhoto(id string, size int) (*os.File, error) + OpenPhoto(ctx context.Context, id string, size int) (*os.File, error) } // getPhoto answers GET /media/photos/{id}-{size}.jpg with the stored JPEG. @@ -32,7 +33,7 @@ func getPhoto(logger *slog.Logger, photos PhotoFiles) http.HandlerFunc { notFound(w, r) return } - f, err := photos.OpenPhoto(id, size) + f, err := photos.OpenPhoto(r.Context(), id, size) if errors.Is(err, fs.ErrNotExist) { notFound(w, r) return diff --git a/internal/server/media_test.go b/internal/server/media_test.go index 1785d0f..936d556 100644 --- a/internal/server/media_test.go +++ b/internal/server/media_test.go @@ -10,39 +10,58 @@ import ( "testing" "testing/fstest" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" ) // newMediaRouter returns a router over a photo cache whose data directory -// holds both sizes of one stored photo, and that photo's id. Each size's -// file holds its size as text. -func newMediaRouter(t *testing.T) (http.Handler, string) { +// holds both sizes of two photos, the id of the one with a photos row, and +// the id of the orphan without one. Each size's file holds its size as +// text. +func newMediaRouter(t *testing.T) (r http.Handler, id, orphan string) { t.Helper() dataDir := t.TempDir() - photos, err := recipes.NewPhotoCache(recipes.NewFetcher(), newTestStore(t), dataDir) + st := newTestStore(t) + photos, err := recipes.NewPhotoCache(recipes.NewFetcher(), st, dataDir) if err != nil { t.Fatal(err) } - id := store.NewID() - for _, size := range []string{"400", "1200"} { - p := filepath.Join(dataDir, "media", "photos", id+"-"+size+".jpg") - if err := os.WriteFile(p, []byte("jpeg "+size), 0o600); err != nil { - t.Fatal(err) + id, orphan = store.NewID(), store.NewID() + for _, stem := range []string{id, orphan} { + for _, size := range []string{"400", "1200"} { + p := filepath.Join(dataDir, "media", "photos", stem+"-"+size+".jpg") + if err := os.WriteFile(p, []byte("jpeg "+size), 0o600); err != nil { + t.Fatal(err) + } } } + now := store.Now().String() + if _, err := db.New(st.Writer).CreatePhoto(t.Context(), db.CreatePhotoParams{ + ID: id, + SourceUrl: "https://example.com/a.jpg", + LocalPath: "media/photos/" + id + "-1200.jpg", + Width: 1600, + Height: 1200, + Sha256: "sha", + FetchedAt: now, + CreatedAt: now, + UpdatedAt: now, + }); err != nil { + t.Fatal(err) + } logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - r, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestRunner(t), + r, err = NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestRunner(t), photos, fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) } - return r, id + return r, id, orphan } func TestGetPhotoServesEachSize(t *testing.T) { t.Parallel() - router, id := newMediaRouter(t) + router, id, _ := newMediaRouter(t) for _, size := range []string{"400", "1200"} { req := httptest.NewRequest(http.MethodGet, "/media/photos/"+id+"-"+size+".jpg", nil) req.AddCookie(validSessionCookie) @@ -66,7 +85,7 @@ func TestGetPhotoServesEachSize(t *testing.T) { func TestGetPhotoRequiresSession(t *testing.T) { t.Parallel() - router, id := newMediaRouter(t) + router, id, _ := newMediaRouter(t) req := httptest.NewRequest(http.MethodGet, "/media/photos/"+id+"-400.jpg", nil) rec := httptest.NewRecorder() router.ServeHTTP(rec, req) @@ -84,10 +103,12 @@ func TestGetPhotoRequiresSession(t *testing.T) { func TestGetPhotoUnknownNotFound(t *testing.T) { t.Parallel() - router, id := newMediaRouter(t) + router, id, orphan := newMediaRouter(t) for _, name := range []string{ store.NewID() + "-400.jpg", // no such photo + orphan + "-400.jpg", // file stored, no photos row id + "-800.jpg", // size not stored + orphan + "-1200.jpg", id + "-0400.jpg", id + "-+400.jpg", id + "-400.png", diff --git a/migrations/queries/photos.sql b/migrations/queries/photos.sql index 983d5ab..79a62eb 100644 --- a/migrations/queries/photos.sql +++ b/migrations/queries/photos.sql @@ -15,3 +15,7 @@ SELECT * FROM photos WHERE sha256 = ? ORDER BY id LIMIT 1; + +-- name: GetPhoto :one +SELECT * FROM photos +WHERE id = ?; -- 2.55.0