diff --git a/TODO.md b/TODO.md index e75260a..2290071 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,17 @@ P2: security: per-origin rate limiting # Completed Steps +- 2026-10-21 one image request type instead of five (closes #307): the new + package `internal/imagerequest`, which imports no other package of pixa, holds + `imagerequest.Request` with its two parts, `Source` (host, path and query) and + `Transform` (size, format, quality and fit), the `Format` and `Fit` types, the + checks, the parsing of the `/v1/image/` path and the variant key. + `imgcache.ParsedURL`, `imgcache.ImageRequest`, `signature.Request`, + `imageprocessor.Request`, the second `Size`, format and fit types and the + conversions between them are gone. The image processor takes a `Transform` + only, and `encurl` keeps a private struct with the token's short CBOR field + names, so tokens made before still decode. Every URL of both image routes + answers as before. - 2026-10-21 the Gitea workflow checks out the repo and runs `script/cibuild`, nothing else (closes #203): its `script/docker-smoke` step is removed, as the canonical `REPO_POLICIES.md` says, since `make check` covers what CI needs to diff --git a/internal/encurl/encurl.go b/internal/encurl/encurl.go index 13a5772..e754ec9 100644 --- a/internal/encurl/encurl.go +++ b/internal/encurl/encurl.go @@ -7,15 +7,15 @@ import ( "github.com/fxamacker/cbor/v2" - "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/seal" ) // Default values for optional fields. const ( DefaultQuality = 85 - DefaultFormat = imgcache.FormatJXL - DefaultFitMode = imgcache.FitCover + DefaultFormat = imagerequest.FormatJXL + DefaultFitMode = imagerequest.FitCover // HKDF salt for URL encryption key derivation urlKeySalt = "pixa-urlenc-v1" @@ -28,19 +28,19 @@ var ( ErrDecryptFailed = errors.New("failed to decrypt URL") ) -// Payload contains all image request parameters in a compact format. +// payload contains all image request parameters in a compact format. // Field names are short to minimize encoded size. // Fields with omitempty use sane defaults when absent. -type Payload struct { - SourceHost string `cbor:"h"` // required - SourcePath string `cbor:"p"` // required - SourceQuery string `cbor:"q,omitempty"` // optional - Width int `cbor:"w,omitempty"` // 0 = original - Height int `cbor:"ht,omitempty"` // 0 = original - Format imgcache.ImageFormat `cbor:"f,omitempty"` // default: jxl - Quality int `cbor:"ql,omitempty"` // default: 85 - FitMode imgcache.FitMode `cbor:"fm,omitempty"` // default: cover - ExpiresAt int64 `cbor:"e,omitempty"` // 0 = never expires +type payload struct { + SourceHost string `cbor:"h"` // required + SourcePath string `cbor:"p"` // required + SourceQuery string `cbor:"q,omitempty"` // optional + Width int `cbor:"w,omitempty"` // 0 = original + Height int `cbor:"ht,omitempty"` // 0 = original + Format imagerequest.Format `cbor:"f,omitempty"` // default: jxl + Quality int `cbor:"ql,omitempty"` // default: 85 + FitMode imagerequest.Fit `cbor:"fm,omitempty"` // default: cover + ExpiresAt int64 `cbor:"e,omitempty"` // 0 = never expires } // Generator creates and parses encrypted URL tokens. @@ -59,9 +59,27 @@ func NewGenerator(signingKey string) (*Generator, error) { return &Generator{key: key}, nil } -// Generate creates an encrypted URL token from the payload. -// The token is CBOR-encoded, encrypted with NaCl secretbox, and base64url-encoded. -func (g *Generator) Generate(p *Payload) (string, error) { +// Generate creates an encrypted URL token from the request's source, +// transform and expiry, each as the request holds it; an empty format, a +// quality of 0 or an empty fit mode is left out of the token, and a zero +// Expires makes a token that never expires. The token is CBOR-encoded, +// encrypted with NaCl secretbox, and base64url-encoded. +func (g *Generator) Generate(req *imagerequest.Request) (string, error) { + p := &payload{ + SourceHost: req.Source.Host, + SourcePath: req.Source.Path, + SourceQuery: req.Source.Query, + Width: req.Transform.Size.Width, + Height: req.Transform.Size.Height, + Format: req.Transform.Format, + Quality: req.Transform.Quality, + FitMode: req.Transform.FitMode, + } + + if !req.Expires.IsZero() { + p.ExpiresAt = req.Expires.Unix() + } + // CBOR encode the payload data, err := cbor.Marshal(p) if err != nil { @@ -72,9 +90,11 @@ func (g *Generator) Generate(p *Payload) (string, error) { return seal.Encrypt(g.key, data) } -// Parse decrypts and validates an encrypted URL token. +// Parse decrypts and validates an encrypted URL token and returns the +// request it holds, with default values for omitted optional fields. A token +// that never expires gives the zero Expires. // Returns ErrExpired if the token has expired, or other errors for invalid tokens. -func (g *Generator) Parse(token string) (*Payload, error) { +func (g *Generator) Parse(token string) (*imagerequest.Request, error) { // Decrypt data, err := seal.Decrypt(g.key, token) if err != nil { @@ -87,7 +107,7 @@ func (g *Generator) Parse(token string) (*Payload, error) { } // CBOR decode - var p Payload + var p payload err = cbor.Unmarshal(data, &p) if err != nil { @@ -99,13 +119,6 @@ func (g *Generator) Parse(token string) (*Payload, error) { return nil, ErrExpired } - return &p, nil -} - -// ToImageRequest converts the payload to an ImageRequest. -// Applies default values for omitted optional fields. An ExpiresAt of 0, a URL -// that never expires, gives the zero Expires. -func (p *Payload) ToImageRequest() *imgcache.ImageRequest { format := p.Format if format == "" { format = DefaultFormat @@ -126,44 +139,18 @@ func (p *Payload) ToImageRequest() *imgcache.ImageRequest { expires = time.Unix(p.ExpiresAt, 0) } - return &imgcache.ImageRequest{ - SourceHost: p.SourceHost, - SourcePath: p.SourcePath, - SourceQuery: p.SourceQuery, - Size: imgcache.Size{ - Width: p.Width, - Height: p.Height, + return &imagerequest.Request{ + Source: imagerequest.Source{ + Host: p.SourceHost, + Path: p.SourcePath, + Query: p.SourceQuery, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: p.Width, Height: p.Height}, + Format: format, + Quality: quality, + FitMode: fitMode, }, - Format: format, - Quality: quality, - FitMode: fitMode, Expires: expires, - } -} - -// FromImageRequest creates a Payload from an ImageRequest with the given expiration. -func FromImageRequest(req *imgcache.ImageRequest, expiresAt time.Time) *Payload { - p := &Payload{ - SourceHost: req.SourceHost, - SourcePath: req.SourcePath, - SourceQuery: req.SourceQuery, - Width: req.Size.Width, - Height: req.Size.Height, - ExpiresAt: expiresAt.Unix(), - } - - // Only set non-default values to benefit from omitempty - if req.Format != "" && req.Format != DefaultFormat { - p.Format = req.Format - } - - if req.Quality != 0 && req.Quality != DefaultQuality { - p.Quality = req.Quality - } - - if req.FitMode != "" && req.FitMode != DefaultFitMode { - p.FitMode = req.FitMode - } - - return p + }, nil } diff --git a/internal/encurl/encurl_test.go b/internal/encurl/encurl_test.go index 48a1dd8..338f678 100644 --- a/internal/encurl/encurl_test.go +++ b/internal/encurl/encurl_test.go @@ -6,7 +6,7 @@ import ( "time" "sneak.berlin/go/pixa/internal/encurl" - "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/imagerequest" ) // Shared test fixture strings. @@ -24,19 +24,22 @@ func TestGenerator_GenerateAndParse(t *testing.T) { t.Fatalf("NewGenerator() error = %v", err) } - payload := &encurl.Payload{ - SourceHost: testSourceHost, - SourcePath: testSourcePath, - SourceQuery: testSourceQuery, - Width: 800, - Height: 600, - Format: imgcache.FormatWebP, - Quality: 90, - FitMode: imgcache.FitContain, - ExpiresAt: time.Now().Add(time.Hour).Unix(), + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testSourceHost, + Path: testSourcePath, + Query: testSourceQuery, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: imagerequest.FormatWebP, + Quality: 90, + FitMode: imagerequest.FitContain, + }, + Expires: time.Now().Add(time.Hour), } - token, err := gen.Generate(payload) + token, err := gen.Generate(req) if err != nil { t.Fatalf("Generate() error = %v", err) } @@ -51,40 +54,45 @@ func TestGenerator_GenerateAndParse(t *testing.T) { } // Verify all fields - if parsed.SourceHost != payload.SourceHost { - t.Errorf("SourceHost = %q, want %q", parsed.SourceHost, payload.SourceHost) + if parsed.Source.Host != req.Source.Host { + t.Errorf("SourceHost = %q, want %q", parsed.Source.Host, req.Source.Host) } - if parsed.SourcePath != payload.SourcePath { - t.Errorf("SourcePath = %q, want %q", parsed.SourcePath, payload.SourcePath) + if parsed.Source.Path != req.Source.Path { + t.Errorf("SourcePath = %q, want %q", parsed.Source.Path, req.Source.Path) } - if parsed.SourceQuery != payload.SourceQuery { - t.Errorf("SourceQuery = %q, want %q", parsed.SourceQuery, payload.SourceQuery) + if parsed.Source.Query != req.Source.Query { + t.Errorf("SourceQuery = %q, want %q", parsed.Source.Query, req.Source.Query) } - if parsed.Width != payload.Width { - t.Errorf("Width = %d, want %d", parsed.Width, payload.Width) + if parsed.Transform.Size.Width != req.Transform.Size.Width { + t.Errorf("Width = %d, want %d", + parsed.Transform.Size.Width, req.Transform.Size.Width) } - if parsed.Height != payload.Height { - t.Errorf("Height = %d, want %d", parsed.Height, payload.Height) + if parsed.Transform.Size.Height != req.Transform.Size.Height { + t.Errorf("Height = %d, want %d", + parsed.Transform.Size.Height, req.Transform.Size.Height) } - if parsed.Format != payload.Format { - t.Errorf("Format = %q, want %q", parsed.Format, payload.Format) + if parsed.Transform.Format != req.Transform.Format { + t.Errorf("Format = %q, want %q", + parsed.Transform.Format, req.Transform.Format) } - if parsed.Quality != payload.Quality { - t.Errorf("Quality = %d, want %d", parsed.Quality, payload.Quality) + if parsed.Transform.Quality != req.Transform.Quality { + t.Errorf("Quality = %d, want %d", + parsed.Transform.Quality, req.Transform.Quality) } - if parsed.FitMode != payload.FitMode { - t.Errorf("FitMode = %q, want %q", parsed.FitMode, payload.FitMode) + if parsed.Transform.FitMode != req.Transform.FitMode { + t.Errorf("FitMode = %q, want %q", + parsed.Transform.FitMode, req.Transform.FitMode) } - if parsed.ExpiresAt != payload.ExpiresAt { - t.Errorf("ExpiresAt = %d, want %d", parsed.ExpiresAt, payload.ExpiresAt) + if parsed.Expires.Unix() != req.Expires.Unix() { + t.Errorf("ExpiresAt = %d, want %d", parsed.Expires.Unix(), req.Expires.Unix()) } } @@ -93,13 +101,15 @@ func TestGenerator_Parse_Expired(t *testing.T) { gen, _ := encurl.NewGenerator("test-signing-key-12345") - payload := &encurl.Payload{ - SourceHost: testSourceHost, - SourcePath: testSourcePath, - ExpiresAt: time.Now().Add(-time.Hour).Unix(), // Already expired + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testSourceHost, + Path: testSourcePath, + }, + Expires: time.Now().Add(-time.Hour), // Already expired } - token, err := gen.Generate(payload) + token, err := gen.Generate(req) if err != nil { t.Fatalf("Generate() error = %v", err) } @@ -130,13 +140,15 @@ func TestGenerator_Parse_TamperedToken(t *testing.T) { gen, _ := encurl.NewGenerator("test-signing-key-12345") - payload := &encurl.Payload{ - SourceHost: testSourceHost, - SourcePath: testSourcePath, - ExpiresAt: time.Now().Add(time.Hour).Unix(), + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testSourceHost, + Path: testSourcePath, + }, + Expires: time.Now().Add(time.Hour), } - token, _ := gen.Generate(payload) + token, _ := gen.Generate(req) // Tamper with the token tampered := []byte(token) @@ -156,13 +168,15 @@ func TestGenerator_Parse_WrongKey(t *testing.T) { gen1, _ := encurl.NewGenerator("signing-key-1") gen2, _ := encurl.NewGenerator("signing-key-2") - payload := &encurl.Payload{ - SourceHost: testSourceHost, - SourcePath: testSourcePath, - ExpiresAt: time.Now().Add(time.Hour).Unix(), + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testSourceHost, + Path: testSourcePath, + }, + Expires: time.Now().Add(time.Hour), } - token, _ := gen1.Generate(payload) + token, _ := gen1.Generate(req) _, err := gen2.Parse(token) if err == nil { @@ -170,140 +184,41 @@ func TestGenerator_Parse_WrongKey(t *testing.T) { } } -func TestPayload_ToImageRequest(t *testing.T) { +func TestGenerator_Parse_Defaults(t *testing.T) { t.Parallel() - payload := &encurl.Payload{ - SourceHost: testSourceHost, - SourcePath: testSourcePath, - SourceQuery: testSourceQuery, - Width: 800, - Height: 600, - Format: imgcache.FormatWebP, - Quality: 90, - FitMode: imgcache.FitContain, - ExpiresAt: time.Now().Add(time.Hour).Unix(), + gen, _ := encurl.NewGenerator("test-signing-key-12345") + + // Request with only required fields - should get defaults + token, err := gen.Generate(&imagerequest.Request{ + Source: imagerequest.Source{ + Host: testSourceHost, + Path: testSourcePath, + }, + Expires: time.Now().Add(time.Hour), + }) + if err != nil { + t.Fatalf("Generate() error = %v", err) } - req := payload.ToImageRequest() - - if req.SourceHost != payload.SourceHost { - t.Errorf("SourceHost = %q, want %q", req.SourceHost, payload.SourceHost) + req, err := gen.Parse(token) + if err != nil { + t.Fatalf("Parse() error = %v", err) } - if req.SourcePath != payload.SourcePath { - t.Errorf("SourcePath = %q, want %q", req.SourcePath, payload.SourcePath) + if req.Transform.Format != encurl.DefaultFormat { + t.Errorf("Format = %q, want default %q", + req.Transform.Format, encurl.DefaultFormat) } - if req.SourceQuery != payload.SourceQuery { - t.Errorf("SourceQuery = %q, want %q", req.SourceQuery, payload.SourceQuery) + if req.Transform.Quality != encurl.DefaultQuality { + t.Errorf("Quality = %d, want default %d", + req.Transform.Quality, encurl.DefaultQuality) } - if req.Size.Width != payload.Width { - t.Errorf("Width = %d, want %d", req.Size.Width, payload.Width) - } - - if req.Size.Height != payload.Height { - t.Errorf("Height = %d, want %d", req.Size.Height, payload.Height) - } - - if req.Format != payload.Format { - t.Errorf("Format = %q, want %q", req.Format, payload.Format) - } - - if req.Quality != payload.Quality { - t.Errorf("Quality = %d, want %d", req.Quality, payload.Quality) - } - - if req.FitMode != payload.FitMode { - t.Errorf("FitMode = %q, want %q", req.FitMode, payload.FitMode) - } -} - -func TestPayload_ToImageRequest_Defaults(t *testing.T) { - t.Parallel() - - // Payload with only required fields - should get defaults - payload := &encurl.Payload{ - SourceHost: testSourceHost, - SourcePath: testSourcePath, - ExpiresAt: time.Now().Add(time.Hour).Unix(), - } - - req := payload.ToImageRequest() - - if req.Format != encurl.DefaultFormat { - t.Errorf("Format = %q, want default %q", req.Format, encurl.DefaultFormat) - } - - if req.Quality != encurl.DefaultQuality { - t.Errorf("Quality = %d, want default %d", req.Quality, encurl.DefaultQuality) - } - - if req.FitMode != encurl.DefaultFitMode { - t.Errorf("FitMode = %q, want default %q", req.FitMode, encurl.DefaultFitMode) - } -} - -func TestFromImageRequest(t *testing.T) { - t.Parallel() - - req := &imgcache.ImageRequest{ - SourceHost: testSourceHost, - SourcePath: testSourcePath, - SourceQuery: testSourceQuery, - Size: imgcache.Size{Width: 800, Height: 600}, - Format: imgcache.FormatWebP, - Quality: 90, - FitMode: imgcache.FitContain, - } - - expiresAt := time.Now().Add(time.Hour) - payload := encurl.FromImageRequest(req, expiresAt) - - if payload.SourceHost != req.SourceHost { - t.Errorf("SourceHost = %q, want %q", payload.SourceHost, req.SourceHost) - } - - if payload.SourcePath != req.SourcePath { - t.Errorf("SourcePath = %q, want %q", payload.SourcePath, req.SourcePath) - } - - if payload.Width != req.Size.Width { - t.Errorf("Width = %d, want %d", payload.Width, req.Size.Width) - } - - if payload.ExpiresAt != expiresAt.Unix() { - t.Errorf("ExpiresAt = %d, want %d", payload.ExpiresAt, expiresAt.Unix()) - } -} - -func TestFromImageRequest_OmitsDefaults(t *testing.T) { - t.Parallel() - - // Request with default values - payload should omit them for - // smaller encoding - req := &imgcache.ImageRequest{ - SourceHost: testSourceHost, - SourcePath: testSourcePath, - Format: encurl.DefaultFormat, - Quality: encurl.DefaultQuality, - FitMode: encurl.DefaultFitMode, - } - - payload := encurl.FromImageRequest(req, time.Now().Add(time.Hour)) - - // These should be zero/empty because they match defaults - if payload.Format != "" { - t.Errorf("Format should be empty for default, got %q", payload.Format) - } - - if payload.Quality != 0 { - t.Errorf("Quality should be 0 for default, got %d", payload.Quality) - } - - if payload.FitMode != "" { - t.Errorf("FitMode should be empty for default, got %q", payload.FitMode) + if req.Transform.FitMode != encurl.DefaultFitMode { + t.Errorf("FitMode = %q, want default %q", + req.Transform.FitMode, encurl.DefaultFitMode) } } @@ -312,13 +227,15 @@ func TestGenerator_TokenIsURLSafe(t *testing.T) { gen, _ := encurl.NewGenerator("test-signing-key-12345") - payload := &encurl.Payload{ - SourceHost: testSourceHost, - SourcePath: testSourcePath, - ExpiresAt: time.Now().Add(time.Hour).Unix(), + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testSourceHost, + Path: testSourcePath, + }, + Expires: time.Now().Add(time.Hour), } - token, _ := gen.Generate(payload) + token, _ := gen.Generate(req) // Check that token only contains URL-safe characters for _, c := range token { diff --git a/internal/handlers/auth.go b/internal/handlers/auth.go index 27105f2..a5a6702 100644 --- a/internal/handlers/auth.go +++ b/internal/handlers/auth.go @@ -16,7 +16,7 @@ import ( "sneak.berlin/go/pixa/internal/clientip" "sneak.berlin/go/pixa/internal/encurl" - "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/templates" ) @@ -145,7 +145,7 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc { return } - payload, expiresAt, ttl, err := buildGeneratePayload(parsed, r.Form) + req, expiresAt, ttl, err := buildGenerateRequest(parsed, r.Form) if err != nil { s.renderGeneratorWithForm(w, r, err.Error(), r.Form, http.StatusBadRequest) @@ -154,9 +154,8 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc { } // The image URL must fetch the source URL as it was written - sourceRequest := payload.ToImageRequest() - sourceRequest.AllowHTTP = s.config.AllowHTTP - fetchURL := sourceRequest.SourceURL() + req.AllowHTTP = s.config.AllowHTTP + fetchURL := req.SourceURL() if !fetchedAsWritten(parsed, fetchURL) { s.renderGeneratorWithForm(w, r, "Source URL cannot be fetched as "+ @@ -167,7 +166,7 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc { } // Generate encrypted token - token, err := s.encGen.Generate(payload) + token, err := s.encGen.Generate(req) if err != nil { s.log.Error("failed to generate encrypted URL", "error", err) s.renderGeneratorWithForm(w, r, "Failed to generate URL", r.Form, @@ -240,14 +239,15 @@ func fetchedAsWritten(source *url.URL, fetchURL string) bool { fetched.RawQuery == source.RawQuery } -// buildGeneratePayload parses the numeric form fields and assembles the -// encrypted URL payload. ttl=0 means never expires (ExpiresAt stays 0). A -// non-numeric or out-of-range width, height, quality or ttl, or an -// unrecognized fit mode, is a client error naming the offending field. The -// format field is passed through unchecked. -func buildGeneratePayload( +// buildGenerateRequest parses the numeric form fields and assembles the +// image request the encrypted URL's token is made from. ttl=0 means never +// expires (Expires stays the zero time). A non-numeric or out-of-range +// width, height, quality or ttl, or an unrecognized fit mode, is a client +// error naming the offending field. The format field is passed through +// unchecked. +func buildGenerateRequest( parsed *url.URL, form url.Values, -) (*encurl.Payload, time.Time, int, error) { +) (*imagerequest.Request, time.Time, int, error) { width, err := parseFormDimension(form, "width") if err != nil { return nil, time.Time{}, 0, err @@ -269,42 +269,40 @@ func buildGeneratePayload( return nil, time.Time{}, 0, err } - fitMode := imgcache.FitMode(form.Get("fit")) + fitMode := imagerequest.Fit(form.Get("fit")) - err = imgcache.ValidateFitMode(fitMode) + err = imagerequest.ValidateFitMode(fitMode) if err != nil { return nil, time.Time{}, 0, - fmt.Errorf("%w: %s", imgcache.ErrInvalidFitMode, form.Get("fit")) + fmt.Errorf("%w: %s", imagerequest.ErrInvalidFitMode, form.Get("fit")) } - var ( - expiresAt time.Time - expiresAtUnix int64 - ) - + var expiresAt time.Time if ttl > 0 { expiresAt = time.Now().Add(time.Duration(ttl) * time.Second) - expiresAtUnix = expiresAt.Unix() } - payload := &encurl.Payload{ - SourceHost: parsed.Host, - SourcePath: parsed.Path, - SourceQuery: parsed.RawQuery, - Width: width, - Height: height, - Format: imgcache.ImageFormat(form.Get("format")), - Quality: quality, - FitMode: fitMode, - ExpiresAt: expiresAtUnix, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: parsed.Host, + Path: parsed.Path, + Query: parsed.RawQuery, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: width, Height: height}, + Format: imagerequest.Format(form.Get("format")), + Quality: quality, + FitMode: fitMode, + }, + Expires: expiresAt, } - return payload, expiresAt, ttl, nil + return req, expiresAt, ttl, nil } // parseFormDimension reads an optional width or height form field. An empty // value means "original size" (0). A non-numeric value, or one -// imgcache.ValidateDimension rejects, is an error naming the field. +// imagerequest.ValidateDimension rejects, is an error naming the field. func parseFormDimension(form url.Values, field string) (int, error) { raw := form.Get(field) if raw == "" { @@ -316,7 +314,7 @@ func parseFormDimension(form url.Values, field string) (int, error) { return 0, fmt.Errorf("%w %s: not a number", errInvalidFormField, field) } - err = imgcache.ValidateDimension(field, value) + err = imagerequest.ValidateDimension(field, value) if err != nil { return 0, err } diff --git a/internal/handlers/auth_generate_internal_test.go b/internal/handlers/auth_generate_internal_test.go index bc26990..1da7c1b 100644 --- a/internal/handlers/auth_generate_internal_test.go +++ b/internal/handlers/auth_generate_internal_test.go @@ -11,7 +11,7 @@ import ( "time" "sneak.berlin/go/pixa/internal/encurl" - "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/imagerequest" ) // Generator form field names, and a value that is not a number. @@ -125,10 +125,10 @@ func TestGeneratePostRejectsBadField(t *testing.T) { } } -// TestBuildGeneratePayloadDefaultAndLimits verifies that an empty quality +// TestBuildGenerateRequestDefaultAndLimits verifies that an empty quality // takes the default, and that the largest accepted width, height, quality and // ttl are accepted with an expiry still in the future. -func TestBuildGeneratePayloadDefaultAndLimits(t *testing.T) { +func TestBuildGenerateRequestDefaultAndLimits(t *testing.T) { t.Parallel() parsed, err := url.Parse(testSourceURL) @@ -136,19 +136,19 @@ func TestBuildGeneratePayloadDefaultAndLimits(t *testing.T) { t.Fatalf("url.Parse() error = %v", err) } - payload, _, _, err := buildGeneratePayload(parsed, url.Values{}) + req, _, _, err := buildGenerateRequest(parsed, url.Values{}) if err != nil { t.Fatalf("empty form: error = %v", err) } - if payload.Quality != encurl.DefaultQuality { + if req.Transform.Quality != encurl.DefaultQuality { t.Errorf("empty quality gave %d, want %d", - payload.Quality, encurl.DefaultQuality) + req.Transform.Quality, encurl.DefaultQuality) } - _, expiresAt, _, err := buildGeneratePayload(parsed, url.Values{ - widthField: {strconv.Itoa(imgcache.MaxDimension)}, - heightField: {strconv.Itoa(imgcache.MaxDimension)}, + _, expiresAt, _, err := buildGenerateRequest(parsed, url.Values{ + widthField: {strconv.Itoa(imagerequest.MaxDimension)}, + heightField: {strconv.Itoa(imagerequest.MaxDimension)}, qualityField: {strconv.Itoa(maxQuality)}, ttlField: {strconv.Itoa(maxTTL)}, }) diff --git a/internal/handlers/auth_session_internal_test.go b/internal/handlers/auth_session_internal_test.go index 250d627..d99ca86 100644 --- a/internal/handlers/auth_session_internal_test.go +++ b/internal/handlers/auth_session_internal_test.go @@ -10,7 +10,7 @@ import ( "testing" "time" - "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/session" ) @@ -314,7 +314,7 @@ func TestGeneratePost_URLServesImage(t *testing.T) { sourceURLField: {"https://" + signedHost + photoPath}, widthField: {"50"}, heightField: {"50"}, - formatField: {string(imgcache.FormatJPEG)}, + formatField: {string(imagerequest.FormatJPEG)}, }) if rec.Code != http.StatusOK { @@ -347,7 +347,7 @@ func TestGeneratePost_URLWithTTLExpires(t *testing.T) { sourceURLField: {"https://" + signedHost + photoPath}, widthField: {"50"}, heightField: {"50"}, - formatField: {string(imgcache.FormatJPEG)}, + formatField: {string(imagerequest.FormatJPEG)}, ttlField: {"1"}, }) diff --git a/internal/handlers/avif_too_large_internal_test.go b/internal/handlers/avif_too_large_internal_test.go index c634a03..1efad83 100644 --- a/internal/handlers/avif_too_large_internal_test.go +++ b/internal/handlers/avif_too_large_internal_test.go @@ -9,7 +9,7 @@ import ( "testing" "sneak.berlin/go/pixa/internal/imageprocessor" - "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/imagerequest" ) // TestAVIFOutputTooLargeAnswers400 checks that respondImageError and @@ -26,7 +26,7 @@ func TestAVIFOutputTooLargeAnswers400(t *testing.T) { wantBody := fmt.Sprintf("above %d pixels", imageprocessor.MaxAVIFOutputPixels) plain := httptest.NewRecorder() - h.respondImageError(plain, &imgcache.ImageRequest{}, err) + h.respondImageError(plain, &imagerequest.Request{}, err) encrypted := httptest.NewRecorder() h.handleImageError(encrypted, err) diff --git a/internal/handlers/first_bytes_internal_test.go b/internal/handlers/first_bytes_internal_test.go index fb3ba30..63e7aad 100644 --- a/internal/handlers/first_bytes_internal_test.go +++ b/internal/handlers/first_bytes_internal_test.go @@ -14,6 +14,7 @@ import ( "github.com/go-chi/chi/v5" "sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/httpfetcher" + "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imgcache" ) @@ -75,12 +76,15 @@ func mismatchedSourcePlainURL(*testing.T, *Handlers) string { func mismatchedSourceEncURL(t *testing.T, h *Handlers) string { t.Helper() - token, err := h.encGen.Generate(&encurl.Payload{ - SourceHost: allowlistedHost, - SourcePath: mismatchedSourcePath, - Width: 50, - Height: 50, - Format: imgcache.FormatJPEG, + token, err := h.encGen.Generate(&imagerequest.Request{ + Source: imagerequest.Source{ + Host: allowlistedHost, + Path: mismatchedSourcePath, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + }, }) if err != nil { t.Fatalf("Generate() error = %v", err) diff --git a/internal/handlers/format_auto.go b/internal/handlers/format_auto.go index bca3bde..0dda8f8 100644 --- a/internal/handlers/format_auto.go +++ b/internal/handlers/format_auto.go @@ -8,7 +8,7 @@ import ( "strconv" "strings" - "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/imagerequest" ) // Errors for an Accept header that an auto URL cannot be served for. @@ -24,9 +24,9 @@ var ( // header that is not valid and 406 for one that allows none of the formats, // and reports whether req can be served. Any other format is left as it is. func (s *Handlers) chooseAutoFormat( - w http.ResponseWriter, r *http.Request, req *imgcache.ImageRequest, + w http.ResponseWriter, r *http.Request, req *imagerequest.Request, ) bool { - if req.Format != imgcache.FormatAuto { + if req.Transform.Format != imagerequest.FormatAuto { return true } @@ -45,7 +45,7 @@ func (s *Handlers) chooseAutoFormat( return false } - req.Format = format + req.Transform.Format = format return true } @@ -56,26 +56,26 @@ func (s *Handlers) chooseAutoFormat( // specific entry of image/jpeg, image/* and */* allows it, or when it names // nothing. A q of 0 refuses a format. JPEG XL, AVIF and WebP must be named, as // clients that cannot show them send image/* and */* too. -func formatForAccept(accept string) (imgcache.ImageFormat, error) { +func formatForAccept(accept string) (imagerequest.Format, error) { qualities, err := parseAccept(accept) if err != nil { return "", err } if len(qualities) == 0 { - return imgcache.FormatJPEG, nil + return imagerequest.FormatJPEG, nil } if qualities["image/jxl"] > 0 { - return imgcache.FormatJXL, nil + return imagerequest.FormatJXL, nil } if qualities["image/avif"] > 0 { - return imgcache.FormatAVIF, nil + return imagerequest.FormatAVIF, nil } if qualities["image/webp"] > 0 { - return imgcache.FormatWebP, nil + return imagerequest.FormatWebP, nil } // For JPEG, the most specific entry the header has decides @@ -89,7 +89,7 @@ func formatForAccept(accept string) (imgcache.ImageFormat, error) { } if quality > 0 { - return imgcache.FormatJPEG, nil + return imagerequest.FormatJPEG, nil } return "", errNotAcceptable diff --git a/internal/handlers/format_auto_internal_test.go b/internal/handlers/format_auto_internal_test.go index 9b28d13..d899b36 100644 --- a/internal/handlers/format_auto_internal_test.go +++ b/internal/handlers/format_auto_internal_test.go @@ -12,8 +12,7 @@ import ( "github.com/go-chi/chi/v5" - "sneak.berlin/go/pixa/internal/encurl" - "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/imagerequest" ) // The content types the tests below expect. @@ -33,24 +32,24 @@ func TestFormatForAccept(t *testing.T) { tests := []struct { name string accept string - want imgcache.ImageFormat + want imagerequest.Format wantErr error }{ {"AVIF-capable browser", "image/avif,image/webp,image/apng,image/svg+xml,image/*,*/*;q=0.8", - imgcache.FormatAVIF, nil}, + imagerequest.FormatAVIF, nil}, {"WebP-capable browser", "image/webp,image/png,image/svg+xml,image/*;q=0.8,*/*;q=0.5", - imgcache.FormatWebP, nil}, - {"WebP only", webpType, imgcache.FormatWebP, nil}, - {"neither", "image/png,image/*;q=0.8,*/*;q=0.5", imgcache.FormatJPEG, nil}, - {"wildcard only", "*/*", imgcache.FormatJPEG, nil}, - {"image wildcard only", "image/*", imgcache.FormatJPEG, nil}, - {"absent", "", imgcache.FormatJPEG, nil}, - {"q=0 on AVIF", "image/avif;q=0,image/webp,*/*", imgcache.FormatWebP, nil}, + imagerequest.FormatWebP, nil}, + {"WebP only", webpType, imagerequest.FormatWebP, nil}, + {"neither", "image/png,image/*;q=0.8,*/*;q=0.5", imagerequest.FormatJPEG, nil}, + {"wildcard only", "*/*", imagerequest.FormatJPEG, nil}, + {"image wildcard only", "image/*", imagerequest.FormatJPEG, nil}, + {"absent", "", imagerequest.FormatJPEG, nil}, + {"q=0 on AVIF", "image/avif;q=0,image/webp,*/*", imagerequest.FormatWebP, nil}, {"AVIF named twice, once with q=0", "image/avif,image/avif;q=0.0,*/*", - imgcache.FormatJPEG, nil}, - {"upper case and spaces", " Image/AVIF ; Q=0.5 ", imgcache.FormatAVIF, nil}, + imagerequest.FormatJPEG, nil}, + {"upper case and spaces", " Image/AVIF ; Q=0.5 ", imagerequest.FormatAVIF, nil}, {"q=0 on JPEG", "image/jpeg;q=0,image/*", "", errNotAcceptable}, {"q=0 on everything", "*/*;q=0", "", errNotAcceptable}, {"PNG only", "image/png", "", errNotAcceptable}, @@ -80,23 +79,30 @@ func TestFormatForAccept(t *testing.T) { func autoPhotoURLs(t *testing.T, h *Handlers) (string, string) { t.Helper() - signedURL, err := h.imgSvc.GenerateSignedURL("", &imgcache.ImageRequest{ - SourceHost: signedHost, - SourcePath: photoPath, - Size: imgcache.Size{Width: 50, Height: 50}, - Format: imgcache.FormatAuto, + signedURL, err := h.imgSvc.GenerateSignedURL("", &imagerequest.Request{ + Source: imagerequest.Source{ + Host: signedHost, + Path: photoPath, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatAuto, + }, }, time.Minute) if err != nil { t.Fatalf("GenerateSignedURL() error = %v", err) } - token, err := h.encGen.Generate(&encurl.Payload{ - SourceHost: signedHost, - SourcePath: photoPath, - Width: 50, - Height: 50, - Format: imgcache.FormatAuto, - ExpiresAt: time.Now().Add(time.Minute).Unix(), + token, err := h.encGen.Generate(&imagerequest.Request{ + Source: imagerequest.Source{ + Host: signedHost, + Path: photoPath, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatAuto, + }, + Expires: time.Now().Add(time.Minute), }) if err != nil { t.Fatalf("Generate() error = %v", err) @@ -184,11 +190,15 @@ func TestFormatAuto_SignatureCoversAuto(t *testing.T) { h, srv := newSignedHostServer(t, slog.New(slog.DiscardHandler)) - signedForAVIF, err := h.imgSvc.GenerateSignedURL("", &imgcache.ImageRequest{ - SourceHost: signedHost, - SourcePath: photoPath, - Size: imgcache.Size{Width: 50, Height: 50}, - Format: imgcache.FormatAVIF, + signedForAVIF, err := h.imgSvc.GenerateSignedURL("", &imagerequest.Request{ + Source: imagerequest.Source{ + Host: signedHost, + Path: photoPath, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatAVIF, + }, }, time.Minute) if err != nil { t.Fatalf("GenerateSignedURL() error = %v", err) diff --git a/internal/handlers/image.go b/internal/handlers/image.go index f1a6ff0..b11b0f5 100644 --- a/internal/handlers/image.go +++ b/internal/handlers/image.go @@ -13,6 +13,7 @@ import ( "sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/httpfetcher" "sneak.berlin/go/pixa/internal/imageprocessor" + "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imgcache" "sneak.berlin/go/pixa/internal/magic" ) @@ -35,8 +36,8 @@ func (s *Handlers) HandleImage() http.HandlerFunc { err := s.imgSvc.ValidateRequest(req) if err != nil { s.log.Warn("signature validation failed", - "host", req.SourceHost, - "path", req.SourcePath, + "host", req.Source.Host, + "path", req.Source.Path, "error", err, ) s.respondError(w, "unauthorized", http.StatusUnauthorized) @@ -50,7 +51,7 @@ func (s *Handlers) HandleImage() http.HandlerFunc { } // Get cache key for logging - cacheKey := imgcache.CacheKey(req) + cacheKey := imagerequest.CacheKey(req) // Get the image (from cache or fetch/process) startTime := time.Now() @@ -81,17 +82,17 @@ func (s *Handlers) HandleRobotsTxt() http.HandlerFunc { } // parseImageRequest parses the image URL's path and query parameters into -// an ImageRequest. On invalid input it writes an error response and +// an image request. On invalid input it writes an error response and // returns false. func (s *Handlers) parseImageRequest( w http.ResponseWriter, r *http.Request, -) (*imgcache.ImageRequest, bool) { +) (*imagerequest.Request, bool) { // The path is parsed decoded once, as r.URL.Path holds it, and not from // chi's wildcard: chi routes on the path as the client wrote it whenever // that is not Go's own encoding of it, so its wildcard would still hold // the %3F before a source query when, say, a "/" in the query is written // as %2F - parsed, err := imgcache.ParseImageURL(r.URL.Path) + req, err := imagerequest.ParseImageURL(r.URL.Path) if err != nil { s.log.Warn("failed to parse image URL", "path", r.URL.Path, @@ -102,9 +103,6 @@ func (s *Handlers) parseImageRequest( return nil, false } - // Convert to ImageRequest - req := parsed.ToImageRequest() - // Parse signature params from query string. r.URL.Query() would silently // drop a pair it cannot decode, such as q=80%, so that q would be served // at 85; a query string that cannot be decoded is refused instead. A @@ -148,7 +146,7 @@ func (s *Handlers) parseImageRequest( return nil, false } - req.Quality, err = parseFormInt(query, "q", + req.Transform.Quality, err = parseFormInt(query, "q", encurl.DefaultQuality, minQuality, maxQuality) if err != nil { s.respondError(w, fmt.Sprintf("%v, got %q", err, qStr), @@ -160,23 +158,23 @@ func (s *Handlers) parseImageRequest( // Only a fit missing from the URL is cover. A fit in the URL that is not a // fit mode is refused by the fit-mode check below; that check would take an // empty fit as missing, so an empty one is refused here. - req.FitMode = imgcache.FitMode(query.Get("fit")) - if query.Has("fit") && req.FitMode == "" { + req.Transform.FitMode = imagerequest.Fit(query.Get("fit")) + if query.Has("fit") && req.Transform.FitMode == "" { s.respondError(w, `invalid fit: not a fit mode, got ""`, http.StatusBadRequest) return nil, false } // Default fit mode if not set - if req.FitMode == "" { - req.FitMode = imgcache.FitCover + if req.Transform.FitMode == "" { + req.Transform.FitMode = imagerequest.FitCover } // Enforce dimension and fit-mode bounds, shared with the encrypted-URL // route. Dimensions are already bounded by the path parser above; this // also rejects an unrecognized fit mode with 400 instead of letting it // reach the processor as a 500. - err = imgcache.ValidateImageRequest(req) + err = imagerequest.ValidateImageRequest(req) if err != nil { s.respondError(w, "invalid image request: "+err.Error(), http.StatusBadRequest) @@ -219,7 +217,7 @@ func (s *Handlers) HandleTooManyRequests() http.HandlerFunc { // respondImageError maps image retrieval errors to HTTP responses. func (s *Handlers) respondImageError( - w http.ResponseWriter, req *imgcache.ImageRequest, err error, + w http.ResponseWriter, req *imagerequest.Request, err error, ) { // An upstream body whose first bytes are not those of the format its // Content-Type names is the upstream's error, not pixa's: the image @@ -234,8 +232,8 @@ func (s *Handlers) respondImageError( } s.log.Error("failed to get image", - "host", req.SourceHost, - "path", req.SourcePath, + "host", req.Source.Host, + "path", req.Source.Path, "error", err, ) @@ -346,8 +344,8 @@ func notModified(w http.ResponseWriter, r *http.Request, etag string) bool { // handling conditional and HEAD requests. func (s *Handlers) writeImageResponse( w http.ResponseWriter, r *http.Request, - req *imgcache.ImageRequest, resp *imgcache.ImageResponse, - cacheKey imgcache.VariantKey, startTime time.Time, + req *imagerequest.Request, resp *imgcache.ImageResponse, + cacheKey imagerequest.VariantKey, startTime time.Time, ) { // Set response headers w.Header().Set("Content-Type", resp.ContentType) @@ -388,7 +386,7 @@ func (s *Handlers) writeImageResponse( "cache_key", cacheKey, "cache_status", resp.CacheStatus, "duration_ms", duration.Milliseconds(), - "format", req.Format, + "format", req.Transform.Format, "served_bytes", servedBytes, "fetched_bytes", resp.FetchedBytes, ) diff --git a/internal/handlers/image_cache_control_internal_test.go b/internal/handlers/image_cache_control_internal_test.go index f4e65a5..e94c30f 100644 --- a/internal/handlers/image_cache_control_internal_test.go +++ b/internal/handlers/image_cache_control_internal_test.go @@ -14,6 +14,7 @@ import ( "github.com/go-chi/chi/v5" "sneak.berlin/go/pixa/internal/encurl" + "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imgcache" ) @@ -107,11 +108,15 @@ func TestHandleImage_SignedURL_MaxAgeEndsAtExp(t *testing.T) { h, srv := newSignedHostServer(t, slog.New(slog.DiscardHandler)) - signedURL, err := h.imgSvc.GenerateSignedURL("", &imgcache.ImageRequest{ - SourceHost: signedHost, - SourcePath: photoPath, - Size: imgcache.Size{Width: 50, Height: 50}, - Format: imgcache.FormatJPEG, + signedURL, err := h.imgSvc.GenerateSignedURL("", &imagerequest.Request{ + Source: imagerequest.Source{ + Host: signedHost, + Path: photoPath, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + }, }, time.Minute) if err != nil { t.Fatalf("GenerateSignedURL() error = %v", err) @@ -168,13 +173,13 @@ func TestHandleImageEnc_MaxAge(t *testing.T) { tests := []struct { name string - expiresAt int64 + expires time.Time wantAtLeast int wantAtMost int }{ - {"60 second TTL", time.Now().Add(time.Minute).Unix(), 50, 60}, - {"two-year TTL", time.Now().Add(2 * 365 * 24 * time.Hour).Unix(), 31536000, 31536000}, - {"no TTL", 0, 31536000, 31536000}, + {"60 second TTL", time.Now().Add(time.Minute), 50, 60}, + {"two-year TTL", time.Now().Add(2 * 365 * 24 * time.Hour), 31536000, 31536000}, + {"no TTL", time.Time{}, 31536000, 31536000}, } for _, tt := range tests { @@ -183,13 +188,16 @@ func TestHandleImageEnc_MaxAge(t *testing.T) { h, srv := newSignedHostServer(t, slog.New(slog.DiscardHandler)) - token, err := h.encGen.Generate(&encurl.Payload{ - SourceHost: signedHost, - SourcePath: photoPath, - Width: 50, - Height: 50, - Format: imgcache.FormatJPEG, - ExpiresAt: tt.expiresAt, + token, err := h.encGen.Generate(&imagerequest.Request{ + Source: imagerequest.Source{ + Host: signedHost, + Path: photoPath, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + }, + Expires: tt.expires, }) if err != nil { t.Fatalf("Generate() error = %v", err) diff --git a/internal/handlers/image_errors_internal_test.go b/internal/handlers/image_errors_internal_test.go index 25fe061..72acfd8 100644 --- a/internal/handlers/image_errors_internal_test.go +++ b/internal/handlers/image_errors_internal_test.go @@ -14,6 +14,7 @@ import ( "github.com/go-chi/chi/v5" "sneak.berlin/go/pixa/internal/httpfetcher" + "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imgcache" "sneak.berlin/go/pixa/internal/signature" ) @@ -84,15 +85,18 @@ func photoURLWithSig(host, sig string, expires time.Time) string { // photoSignature returns the signature of photoURL(host) at the default // quality and fit, made with key and expiring at expires. func photoSignature(key, host string, expires time.Time) string { - return signature.New(key).Sign(&signature.Request{ - SourceHost: host, - SourcePath: photoPath, - Width: 50, - Height: 50, - Format: string(imgcache.FormatJPEG), - Quality: 85, - FitMode: string(imgcache.FitCover), - Expires: expires, + return signature.New(key).Sign(&imagerequest.Request{ + Source: imagerequest.Source{ + Host: host, + Path: photoPath, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, + Expires: expires, }) } @@ -353,15 +357,18 @@ func TestHandleImage_HashInSourcePathOrQuery_Returns400(t *testing.T) { photos := newPhotoFetcher(t, allowlistedHost, signedHost) exp := time.Now().Add(time.Hour) - sig := signature.New(testSigningKey).Sign(&signature.Request{ - SourceHost: signedHost, - SourcePath: photoPath + "#x", - Width: 50, - Height: 50, - Format: string(imgcache.FormatJPEG), - Quality: 85, - FitMode: string(imgcache.FitCover), - Expires: exp, + sig := signature.New(testSigningKey).Sign(&imagerequest.Request{ + Source: imagerequest.Source{ + Host: signedHost, + Path: photoPath + "#x", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, + Expires: exp, }) tests := []struct { diff --git a/internal/handlers/image_signature_internal_test.go b/internal/handlers/image_signature_internal_test.go index babdb1e..2849565 100644 --- a/internal/handlers/image_signature_internal_test.go +++ b/internal/handlers/image_signature_internal_test.go @@ -9,7 +9,7 @@ import ( "time" "github.com/go-chi/chi/v5" - "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/signature" ) @@ -44,15 +44,18 @@ func TestHandleImage_SignatureCoversQualityAndFit(t *testing.T) { expires := time.Now().Add(time.Hour) signer := signature.New("test-signing-key-must-be-32-chars") - sig := signer.Sign(&signature.Request{ - SourceHost: signedHost, - SourcePath: "/images/photo.jpg", - Width: 50, - Height: 50, - Format: string(imgcache.FormatJPEG), - Quality: 85, - FitMode: string(imgcache.FitCover), - Expires: expires, + sig := signer.Sign(&imagerequest.Request{ + Source: imagerequest.Source{ + Host: signedHost, + Path: "/images/photo.jpg", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, + Expires: expires, }) signedURL := fmt.Sprintf("/v1/image/%s/images/photo.jpg/50x50.jpeg?sig=%s&exp=%d", signedHost, sig, expires.Unix()) @@ -88,9 +91,9 @@ func TestHandleImage_GeneratedSignedURLVerifies(t *testing.T) { tests := []struct { name string quality int - fitMode imgcache.FitMode + fitMode imagerequest.Fit }{ - {"quality 40 and fit contain", 40, imgcache.FitContain}, + {"quality 40 and fit contain", 40, imagerequest.FitContain}, {"quality and fit unset", 0, ""}, } @@ -100,13 +103,17 @@ func TestHandleImage_GeneratedSignedURLVerifies(t *testing.T) { fix := setupTestHandler(t) - signedURL, err := fix.service.GenerateSignedURL("", &imgcache.ImageRequest{ - SourceHost: signedHost, - SourcePath: "/images/photo.jpg", - Size: imgcache.Size{Width: 50, Height: 50}, - Format: imgcache.FormatJPEG, - Quality: tt.quality, - FitMode: tt.fitMode, + signedURL, err := fix.service.GenerateSignedURL("", &imagerequest.Request{ + Source: imagerequest.Source{ + Host: signedHost, + Path: "/images/photo.jpg", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: tt.quality, + FitMode: tt.fitMode, + }, }, time.Hour) if err != nil { t.Fatalf("GenerateSignedURL() error = %v", err) @@ -196,12 +203,16 @@ func TestHandleImage_GeneratedSignedURLWithSourceQueryVerifies(t *testing.T) { fix := setupTestHandler(t) - signedURL, err := fix.service.GenerateSignedURL("", &imgcache.ImageRequest{ - SourceHost: signedHost, - SourcePath: photoPath, - SourceQuery: tt.query, - Size: imgcache.Size{Width: 50, Height: 50}, - Format: imgcache.FormatJPEG, + signedURL, err := fix.service.GenerateSignedURL("", &imagerequest.Request{ + Source: imagerequest.Source{ + Host: signedHost, + Path: photoPath, + Query: tt.query, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + }, }, time.Hour) if err != nil { t.Fatalf("GenerateSignedURL() error = %v", err) @@ -224,16 +235,19 @@ func TestHandleImage_SourceQueryEncodedAnyWay(t *testing.T) { expires := time.Now().Add(time.Hour) signer := signature.New("test-signing-key-must-be-32-chars") - sig := signer.Sign(&signature.Request{ - SourceHost: signedHost, - SourcePath: photoPath, - SourceQuery: "url=https://origin.example.com/my%20cat.jpg", - Width: 50, - Height: 50, - Format: string(imgcache.FormatJPEG), - Quality: 85, - FitMode: string(imgcache.FitCover), - Expires: expires, + sig := signer.Sign(&imagerequest.Request{ + Source: imagerequest.Source{ + Host: signedHost, + Path: photoPath, + Query: "url=https://origin.example.com/my%20cat.jpg", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, + Expires: expires, }) tests := []struct { @@ -288,16 +302,19 @@ func TestHandleImage_SourceURLHoldingPercent25Verifies(t *testing.T) { expires := time.Now().Add(time.Hour) signer := signature.New("test-signing-key-must-be-32-chars") - sig := signer.Sign(&signature.Request{ - SourceHost: signedHost, - SourcePath: tt.sourcePath, - SourceQuery: tt.sourceQuery, - Width: 50, - Height: 50, - Format: string(imgcache.FormatJPEG), - Quality: 85, - FitMode: string(imgcache.FitCover), - Expires: expires, + sig := signer.Sign(&imagerequest.Request{ + Source: imagerequest.Source{ + Host: signedHost, + Path: tt.sourcePath, + Query: tt.sourceQuery, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, + Expires: expires, }) target := fmt.Sprintf("/v1/image/%s%s/50x50.jpeg?sig=%s&exp=%d", @@ -314,12 +331,16 @@ func TestHandleImage_SourceURLHoldingPercent25Verifies(t *testing.T) { fix := setupTestHandler(t) - signedURL, err := fix.service.GenerateSignedURL("", &imgcache.ImageRequest{ - SourceHost: signedHost, - SourcePath: tt.sourcePath, - SourceQuery: tt.sourceQuery, - Size: imgcache.Size{Width: 50, Height: 50}, - Format: imgcache.FormatJPEG, + signedURL, err := fix.service.GenerateSignedURL("", &imagerequest.Request{ + Source: imagerequest.Source{ + Host: signedHost, + Path: tt.sourcePath, + Query: tt.sourceQuery, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + }, }, time.Hour) if err != nil { t.Fatalf("GenerateSignedURL() error = %v", err) diff --git a/internal/handlers/imageenc.go b/internal/handlers/imageenc.go index eefb74f..e5a94d4 100644 --- a/internal/handlers/imageenc.go +++ b/internal/handlers/imageenc.go @@ -14,6 +14,7 @@ import ( "sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/httpfetcher" "sneak.berlin/go/pixa/internal/imageprocessor" + "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imgcache" "sneak.berlin/go/pixa/internal/magic" ) @@ -37,10 +38,11 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc { // Log the request s.log.Debug("encrypted image request", - "host", req.SourceHost, - "path", req.SourcePath, - "dimensions", fmt.Sprintf("%dx%d", req.Size.Width, req.Size.Height), - "format", req.Format, + "host", req.Source.Host, + "path", req.Source.Path, + "dimensions", fmt.Sprintf("%dx%d", + req.Transform.Size.Width, req.Transform.Size.Height), + "format", req.Transform.Format, ) // Fetch and process the image (no signature validation @@ -88,10 +90,10 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc { duration := time.Since(start) s.log.Info("image served", "request_id", middleware.GetReqID(ctx), - "cache_key", imgcache.CacheKey(req), - "host", req.SourceHost, - "path", req.SourcePath, - "format", req.Format, + "cache_key", imagerequest.CacheKey(req), + "host", req.Source.Host, + "path", req.Source.Path, + "format", req.Transform.Format, "cache_status", resp.CacheStatus, "served_bytes", written, "duration_ms", duration.Milliseconds(), @@ -100,12 +102,12 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc { } // parseImageEncRequest decrypts the token of an encrypted image URL into an -// ImageRequest and checks it. On a token that is missing, does not decrypt, +// image request and checks it. On a token that is missing, does not decrypt, // has expired or asks for something not valid, it writes an error response // and returns false. func (s *Handlers) parseImageEncRequest( w http.ResponseWriter, r *http.Request, -) (*imgcache.ImageRequest, bool) { +) (*imagerequest.Request, bool) { // Extract token from URL token := chi.URLParam(r, "token") if token == "" { @@ -115,7 +117,7 @@ func (s *Handlers) parseImageEncRequest( } // Decrypt and validate the payload - payload, err := s.encGen.Parse(token) + req, err := s.encGen.Parse(token) if err != nil { if errors.Is(err, encurl.ErrExpired) { s.log.Debug("encrypted URL expired", "error", err) @@ -130,14 +132,11 @@ func (s *Handlers) parseImageEncRequest( return nil, false } - // Convert payload to ImageRequest - req := payload.ToImageRequest() - // Apply the same dimension and fit-mode bounds as the plain image // route: a sealed payload is trusted for its origin, not for staying // within limits, so an over-limit size or unknown fit mode is a 400 // here rather than an out-of-memory or a 500 from the processor. - err = imgcache.ValidateImageRequest(req) + err = imagerequest.ValidateImageRequest(req) if err != nil { s.log.Debug("encrypted URL failed validation", "error", err) s.respondError(w, "invalid encrypted URL: "+err.Error(), diff --git a/internal/handlers/imageenc_internal_test.go b/internal/handlers/imageenc_internal_test.go index d4f8c6b..ec034c5 100644 --- a/internal/handlers/imageenc_internal_test.go +++ b/internal/handlers/imageenc_internal_test.go @@ -10,7 +10,7 @@ import ( "github.com/go-chi/chi/v5" "sneak.berlin/go/pixa/internal/encurl" - "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/imagerequest" ) // newEncTestServer builds a router serving the encrypted-URL route with a @@ -54,11 +54,14 @@ func TestHandleImageEnc_OverLimitDimension_Returns400(t *testing.T) { encGen, srv := newEncTestServer(t) - token, err := encGen.Generate(&encurl.Payload{ - SourceHost: "cdn.example.com", - SourcePath: "/photo.jpg", - Width: 100000, - Height: 100000, + token, err := encGen.Generate(&imagerequest.Request{ + Source: imagerequest.Source{ + Host: "cdn.example.com", + Path: "/photo.jpg", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 100000, Height: 100000}, + }, }) if err != nil { t.Fatalf("Generate() error = %v", err) @@ -79,12 +82,15 @@ func TestHandleImageEnc_InvalidFitMode_Returns400(t *testing.T) { encGen, srv := newEncTestServer(t) - token, err := encGen.Generate(&encurl.Payload{ - SourceHost: "cdn.example.com", - SourcePath: "/photo.jpg", - Width: 800, - Height: 600, - FitMode: imgcache.FitMode("bogus"), + token, err := encGen.Generate(&imagerequest.Request{ + Source: imagerequest.Source{ + Host: "cdn.example.com", + Path: "/photo.jpg", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + FitMode: imagerequest.Fit("bogus"), + }, }) if err != nil { t.Fatalf("Generate() error = %v", err) diff --git a/internal/handlers/imageenc_token_internal_test.go b/internal/handlers/imageenc_token_internal_test.go index f2fcda2..f647a11 100644 --- a/internal/handlers/imageenc_token_internal_test.go +++ b/internal/handlers/imageenc_token_internal_test.go @@ -9,6 +9,7 @@ import ( "time" "sneak.berlin/go/pixa/internal/encurl" + "sneak.berlin/go/pixa/internal/imagerequest" ) // requireServedPhoto requires that rec answers 200 with the JPEG at photoPath @@ -59,11 +60,14 @@ func TestHandleImageEnc_RejectedToken(t *testing.T) { h, srv := newSignedHostServer(t, slog.New(slog.DiscardHandler)) - photo := encurl.Payload{ - SourceHost: signedHost, - SourcePath: photoPath, - Width: 50, - Height: 50, + photo := imagerequest.Request{ + Source: imagerequest.Source{ + Host: signedHost, + Path: photoPath, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + }, } valid, err := h.encGen.Generate(&photo) @@ -72,7 +76,7 @@ func TestHandleImageEnc_RejectedToken(t *testing.T) { } expiredPhoto := photo - expiredPhoto.ExpiresAt = time.Now().Add(-time.Minute).Unix() + expiredPhoto.Expires = time.Now().Add(-time.Minute) expired, err := h.encGen.Generate(&expiredPhoto) if err != nil { diff --git a/internal/handlers/jpegxl_default_internal_test.go b/internal/handlers/jpegxl_default_internal_test.go index 1d22bca..219c0f0 100644 --- a/internal/handlers/jpegxl_default_internal_test.go +++ b/internal/handlers/jpegxl_default_internal_test.go @@ -14,7 +14,7 @@ import ( "github.com/davidbyttow/govips/v2/vips" "sneak.berlin/go/pixa/internal/encurl" - "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/signature" ) @@ -58,15 +58,18 @@ func TestImageWithoutFormat_SignedAsJXL(t *testing.T) { route := newImageRoute(t, newPhotoFetcher(t, signedHost)) expires := time.Now().Add(time.Hour) - sig := signature.New(testSigningKey).Sign(&signature.Request{ - SourceHost: signedHost, - SourcePath: photoPath, - Width: 50, - Height: 50, - Format: string(imgcache.FormatJXL), - Quality: encurl.DefaultQuality, - FitMode: string(imgcache.FitCover), - Expires: expires, + sig := signature.New(testSigningKey).Sign(&imagerequest.Request{ + Source: imagerequest.Source{ + Host: signedHost, + Path: photoPath, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJXL, + Quality: encurl.DefaultQuality, + FitMode: imagerequest.FitCover, + }, + Expires: expires, }) query := fmt.Sprintf("?sig=%s&exp=%d", sig, expires.Unix()) @@ -83,11 +86,14 @@ func TestImageEncWithoutFormat_ServesJPEGXL(t *testing.T) { h, srv := newSignedHostServer(t, slog.New(slog.DiscardHandler)) - token, err := h.encGen.Generate(&encurl.Payload{ - SourceHost: signedHost, - SourcePath: photoPath, - Width: 50, - Height: 50, + token, err := h.encGen.Generate(&imagerequest.Request{ + Source: imagerequest.Source{ + Host: signedHost, + Path: photoPath, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + }, }) if err != nil { t.Fatalf("Generate() error = %v", err) diff --git a/internal/handlers/jpegxl_internal_test.go b/internal/handlers/jpegxl_internal_test.go index 68bbe14..c788eee 100644 --- a/internal/handlers/jpegxl_internal_test.go +++ b/internal/handlers/jpegxl_internal_test.go @@ -7,7 +7,7 @@ import ( "github.com/davidbyttow/govips/v2/vips" - "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/imagerequest" ) // jxlType is the content type of JPEG XL. @@ -22,14 +22,14 @@ func TestFormatForAccept_JPEGXL(t *testing.T) { tests := []struct { name string accept string - want imgcache.ImageFormat + want imagerequest.Format }{ {"JPEG XL-capable browser", - "image/jxl,image/avif,image/webp,image/*,*/*;q=0.8", imgcache.FormatJXL}, - {"JPEG XL only", jxlType, imgcache.FormatJXL}, + "image/jxl,image/avif,image/webp,image/*,*/*;q=0.8", imagerequest.FormatJXL}, + {"JPEG XL only", jxlType, imagerequest.FormatJXL}, {"JPEG XL with a lower q than AVIF", "image/avif,image/jxl;q=0.5", - imgcache.FormatJXL}, - {"q=0 on JPEG XL", "image/jxl;q=0,image/avif,*/*", imgcache.FormatAVIF}, + imagerequest.FormatJXL}, + {"q=0 on JPEG XL", "image/jxl;q=0,image/avif,*/*", imagerequest.FormatAVIF}, } for _, tt := range tests { diff --git a/internal/handlers/request_id_internal_test.go b/internal/handlers/request_id_internal_test.go index d26a2f6..a785923 100644 --- a/internal/handlers/request_id_internal_test.go +++ b/internal/handlers/request_id_internal_test.go @@ -13,8 +13,7 @@ import ( "github.com/go-chi/chi/v5/middleware" - "sneak.berlin/go/pixa/internal/encurl" - "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/imagerequest" ) // signedPhotoURL returns a signed /v1/image/ URL, valid for a minute, for the @@ -22,11 +21,15 @@ import ( func signedPhotoURL(t *testing.T, h *Handlers) string { t.Helper() - signedURL, err := h.imgSvc.GenerateSignedURL("", &imgcache.ImageRequest{ - SourceHost: signedHost, - SourcePath: photoPath, - Size: imgcache.Size{Width: 50, Height: 50}, - Format: imgcache.FormatJPEG, + signedURL, err := h.imgSvc.GenerateSignedURL("", &imagerequest.Request{ + Source: imagerequest.Source{ + Host: signedHost, + Path: photoPath, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + }, }, time.Minute) if err != nil { t.Fatalf("GenerateSignedURL() error = %v", err) @@ -40,12 +43,15 @@ func signedPhotoURL(t *testing.T, h *Handlers) string { func encPhotoURL(t *testing.T, h *Handlers) string { t.Helper() - token, err := h.encGen.Generate(&encurl.Payload{ - SourceHost: signedHost, - SourcePath: photoPath, - Width: 50, - Height: 50, - Format: imgcache.FormatJPEG, + token, err := h.encGen.Generate(&imagerequest.Request{ + Source: imagerequest.Source{ + Host: signedHost, + Path: photoPath, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + }, }) if err != nil { t.Fatalf("Generate() error = %v", err) diff --git a/internal/handlers/server_busy_internal_test.go b/internal/handlers/server_busy_internal_test.go index 58bae9b..a7cf030 100644 --- a/internal/handlers/server_busy_internal_test.go +++ b/internal/handlers/server_busy_internal_test.go @@ -11,6 +11,7 @@ import ( "sneak.berlin/go/pixa/internal/httpfetcher" "sneak.berlin/go/pixa/internal/imageprocessor" + "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imgcache" ) @@ -21,7 +22,9 @@ func TestServerBusyAnswers503(t *testing.T) { t.Parallel() h := &Handlers{log: slog.New(slog.DiscardHandler)} - req := &imgcache.ImageRequest{SourceHost: "img.example.com", SourcePath: "/a.jpg"} + req := &imagerequest.Request{ + Source: imagerequest.Source{Host: "img.example.com", Path: "/a.jpg"}, + } for _, err := range []error{ fmt.Errorf("upstream fetch failed: %w", httpfetcher.ErrTooManyConnections), @@ -64,7 +67,7 @@ func TestProcessingTimedOutAnswers503(t *testing.T) { imgcache.ErrProcessingTimedOut, context.DeadlineExceeded) plain := httptest.NewRecorder() - h.respondImageError(plain, &imgcache.ImageRequest{}, err) + h.respondImageError(plain, &imagerequest.Request{}, err) encrypted := httptest.NewRecorder() h.handleImageError(encrypted, err) diff --git a/internal/handlers/unsupported_source_internal_test.go b/internal/handlers/unsupported_source_internal_test.go index a10e37e..c319700 100644 --- a/internal/handlers/unsupported_source_internal_test.go +++ b/internal/handlers/unsupported_source_internal_test.go @@ -10,7 +10,7 @@ import ( "sneak.berlin/go/pixa/internal/httpfetcher" "sneak.berlin/go/pixa/internal/imageprocessor" - "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/imagerequest" ) // TestUnsupportedSourceFormatAnswers415 checks that both image routes answer @@ -21,7 +21,9 @@ func TestUnsupportedSourceFormatAnswers415(t *testing.T) { t.Parallel() h := &Handlers{log: slog.New(slog.DiscardHandler)} - req := &imgcache.ImageRequest{SourceHost: "img.example.com", SourcePath: "/a.svg"} + req := &imagerequest.Request{ + Source: imagerequest.Source{Host: "img.example.com", Path: "/a.svg"}, + } for _, err := range []error{ fmt.Errorf("upstream fetch failed: %w: image/svg+xml", diff --git a/internal/imageprocessor/avif_too_large_internal_test.go b/internal/imageprocessor/avif_too_large_internal_test.go index 2770dd4..1c1a576 100644 --- a/internal/imageprocessor/avif_too_large_internal_test.go +++ b/internal/imageprocessor/avif_too_large_internal_test.go @@ -6,6 +6,8 @@ import ( "errors" "os" "testing" + + "sneak.berlin/go/pixa/internal/imagerequest" ) // TestImageProcessor_RefusesAVIFAboveMaxPixels verifies that an AVIF output @@ -37,22 +39,30 @@ func TestImageProcessor_RefusesAVIFAboveMaxPixels(t *testing.T) { tests := []struct { name string input []byte - format Format + format imagerequest.Format width, height int - fit FitMode + fit imagerequest.Fit refused bool }{ - {"AVIF at the limit", cutOff, FormatAVIF, 4096, 4096, FitCover, false}, - {"AVIF above the limit", cutOff, FormatAVIF, 4096, 4097, FitCover, true}, - {"WebP above the AVIF limit", cutOff, FormatWebP, 4096, 4097, FitCover, false}, + {"AVIF at the limit", cutOff, imagerequest.FormatAVIF, + 4096, 4096, imagerequest.FitCover, false}, + {"AVIF above the limit", cutOff, imagerequest.FormatAVIF, + 4096, 4097, imagerequest.FitCover, true}, + {"WebP above the AVIF limit", cutOff, imagerequest.FormatWebP, + 4096, 4097, imagerequest.FitCover, false}, // FitInside does not enlarge, so the output stays 64x32 - {"AVIF inside a larger size", cutOff, FormatAVIF, 8192, 8192, FitInside, false}, + {"AVIF inside a larger size", cutOff, imagerequest.FormatAVIF, + 8192, 8192, imagerequest.FitInside, false}, // FitOutside scales 64x32 by 128 to cover 8192x1: 8192x4096 - {"AVIF outside a smaller size", cutOff, FormatAVIF, 8192, 1, FitOutside, true}, + {"AVIF outside a smaller size", cutOff, imagerequest.FormatAVIF, + 8192, 1, imagerequest.FitOutside, true}, // With no size, the output keeps the source's 4097x4096 - {"AVIF of the source's size", largeAVIF, FormatAVIF, 0, 0, FitCover, true}, - {"orig of an AVIF source", largeAVIF, FormatOriginal, 0, 0, FitCover, true}, - {"JPEG of the source's size", largeAVIF, FormatJPEG, 0, 0, FitCover, false}, + {"AVIF of the source's size", largeAVIF, imagerequest.FormatAVIF, + 0, 0, imagerequest.FitCover, true}, + {"orig of an AVIF source", largeAVIF, imagerequest.FormatOriginal, + 0, 0, imagerequest.FitCover, true}, + {"JPEG of the source's size", largeAVIF, imagerequest.FormatJPEG, + 0, 0, imagerequest.FitCover, false}, } for _, tt := range tests { @@ -60,8 +70,8 @@ func TestImageProcessor_RefusesAVIFAboveMaxPixels(t *testing.T) { t.Parallel() _, err := New(Params{}).Process(t.Context(), bytes.NewReader(tt.input), - &Request{ - Size: Size{Width: tt.width, Height: tt.height}, + &imagerequest.Transform{ + Size: imagerequest.Size{Width: tt.width, Height: tt.height}, Format: tt.format, FitMode: tt.fit, }) diff --git a/internal/imageprocessor/computed_side_internal_test.go b/internal/imageprocessor/computed_side_internal_test.go index 231087d..fbd49e6 100644 --- a/internal/imageprocessor/computed_side_internal_test.go +++ b/internal/imageprocessor/computed_side_internal_test.go @@ -4,6 +4,8 @@ import ( "bytes" "errors" "testing" + + "sneak.berlin/go/pixa/internal/imagerequest" ) // TestImageProcessor_RefusesComputedSideAboveMaximum verifies that a size @@ -17,13 +19,17 @@ func TestImageProcessor_RefusesComputedSideAboveMaximum(t *testing.T) { tests := []struct { name string inputWidth, inputHeight int - size Size + size imagerequest.Size wantErr error }{ - {"width at the maximum", 4096, 1, Size{Height: 2}, nil}, - {"width above the maximum", 4096, 1, Size{Height: 3}, ErrOutputTooLarge}, - {"height at the maximum", 1, 4096, Size{Width: 2}, nil}, - {"height above the maximum", 1, 4096, Size{Width: 3}, ErrOutputTooLarge}, + {"width at the maximum", 4096, 1, + imagerequest.Size{Height: 2}, nil}, + {"width above the maximum", 4096, 1, + imagerequest.Size{Height: 3}, ErrOutputTooLarge}, + {"height at the maximum", 1, 4096, + imagerequest.Size{Width: 2}, nil}, + {"height above the maximum", 1, 4096, + imagerequest.Size{Width: 3}, ErrOutputTooLarge}, } for _, tt := range tests { @@ -34,7 +40,11 @@ func TestImageProcessor_RefusesComputedSideAboveMaximum(t *testing.T) { result, err := New(Params{}).Process(t.Context(), bytes.NewReader(input), - &Request{Size: tt.size, Format: FormatJPEG, FitMode: FitCover}) + &imagerequest.Transform{ + Size: tt.size, + Format: imagerequest.FormatJPEG, + FitMode: imagerequest.FitCover, + }) if result != nil { t.Logf("Process() made %dx%d", result.Width, result.Height) @@ -64,18 +74,21 @@ func TestImageProcessor_ComputedSideIsAtLeastOne(t *testing.T) { tests := []struct { name string inputWidth, inputHeight int - size Size + size imagerequest.Size wantWidth, wantHeight int }{ - {"8192x1 at 100x0", 8192, 1, Size{Width: 100}, 100, 1}, - {"8192x1 at 1300x0", 8192, 1, Size{Width: 1300}, 1300, 1}, - {"8192x2 at 100x0", 8192, 2, Size{Width: 100}, 100, 1}, - {"1x8192 at 0x100", 1, 8192, Size{Height: 100}, 1, 100}, - {"1x8192 at 0x1300", 1, 8192, Size{Height: 1300}, 1, 1300}, - {"2x8192 at 0x100", 2, 8192, Size{Height: 100}, 1, 100}, + {"8192x1 at 100x0", 8192, 1, imagerequest.Size{Width: 100}, 100, 1}, + {"8192x1 at 1300x0", 8192, 1, imagerequest.Size{Width: 1300}, 1300, 1}, + {"8192x2 at 100x0", 8192, 2, imagerequest.Size{Width: 100}, 100, 1}, + {"1x8192 at 0x100", 1, 8192, imagerequest.Size{Height: 100}, 1, 100}, + {"1x8192 at 0x1300", 1, 8192, imagerequest.Size{Height: 1300}, 1, 1300}, + {"2x8192 at 0x100", 2, 8192, imagerequest.Size{Height: 100}, 1, 100}, } - fits := []FitMode{FitCover, FitContain, FitFill, FitInside, FitOutside} + fits := []imagerequest.Fit{ + imagerequest.FitCover, imagerequest.FitContain, imagerequest.FitFill, + imagerequest.FitInside, imagerequest.FitOutside, + } for _, tt := range tests { for _, fit := range fits { @@ -93,7 +106,11 @@ func TestImageProcessor_ComputedSideIsAtLeastOne(t *testing.T) { result, err := New(Params{}).Process(t.Context(), bytes.NewReader(input), - &Request{Size: tt.size, Format: FormatJPEG, FitMode: fit}) + &imagerequest.Transform{ + Size: tt.size, + Format: imagerequest.FormatJPEG, + FitMode: fit, + }) if err != nil { t.Fatalf("Process() error = %v", err) } @@ -103,14 +120,14 @@ func TestImageProcessor_ComputedSideIsAtLeastOne(t *testing.T) { t.Logf("Process() made %dx%d", result.Width, result.Height) ok := result.Width == tt.wantWidth && result.Height == tt.wantHeight - if fit == FitOutside { + if fit == imagerequest.FitOutside { ok = result.Width >= tt.wantWidth && result.Height >= tt.wantHeight } if !ok { t.Errorf("Process() made %dx%d, want %dx%d (at least, for %s)", result.Width, result.Height, tt.wantWidth, tt.wantHeight, - FitOutside) + imagerequest.FitOutside) } }) } diff --git a/internal/imageprocessor/deadline_internal_test.go b/internal/imageprocessor/deadline_internal_test.go index 1bde965..fb9b2ea 100644 --- a/internal/imageprocessor/deadline_internal_test.go +++ b/internal/imageprocessor/deadline_internal_test.go @@ -86,7 +86,7 @@ func checkProcessStopsAtDeadline( proc := New(Params{MaxConcurrentProcessing: 1}) - result, err := proc.Process(ctx, input, smallJPEGRequest()) + result, err := proc.Process(ctx, input, smallJPEGTransform()) if result != nil { t.Logf("Process() made a %dx%d image after its deadline passed", result.Width, result.Height) diff --git a/internal/imageprocessor/empty_format_internal_test.go b/internal/imageprocessor/empty_format_internal_test.go index 4ddda3f..4feda08 100644 --- a/internal/imageprocessor/empty_format_internal_test.go +++ b/internal/imageprocessor/empty_format_internal_test.go @@ -4,6 +4,8 @@ import ( "bytes" "errors" "testing" + + "sneak.berlin/go/pixa/internal/imagerequest" ) // TestImageProcessor_EmptyFormatRefused verifies that a request with no format @@ -13,7 +15,8 @@ func TestImageProcessor_EmptyFormatRefused(t *testing.T) { t.Parallel() _, err := New(Params{}).Process( - t.Context(), bytes.NewReader(createTestJPEG(t, 20, 20)), &Request{}, + t.Context(), bytes.NewReader(createTestJPEG(t, 20, 20)), + &imagerequest.Transform{}, ) if !errors.Is(err, ErrUnsupportedOutputFormat) { t.Errorf("Process() error = %v, want %v", err, ErrUnsupportedOutputFormat) diff --git a/internal/imageprocessor/encoder_settings_internal_test.go b/internal/imageprocessor/encoder_settings_internal_test.go index af1b31d..8cf3388 100644 --- a/internal/imageprocessor/encoder_settings_internal_test.go +++ b/internal/imageprocessor/encoder_settings_internal_test.go @@ -8,14 +8,17 @@ import ( "testing" "github.com/davidbyttow/govips/v2/vips" + "sneak.berlin/go/pixa/internal/imagerequest" ) // processedSize runs input through Process and returns the output's size in // bytes. -func processedSize(t *testing.T, input []byte, req *Request) int64 { +func processedSize( + t *testing.T, input []byte, transform *imagerequest.Transform, +) int64 { t.Helper() - result, err := New(Params{}).Process(t.Context(), bytes.NewReader(input), req) + result, err := New(Params{}).Process(t.Context(), bytes.NewReader(input), transform) if err != nil { t.Fatalf("Process() error = %v", err) } @@ -68,7 +71,8 @@ func TestImageProcessor_PNGIsCompressed(t *testing.T) { } } - size := processedSize(t, encodePNG(t, flat), &Request{Format: FormatPNG}) + size := processedSize(t, encodePNG(t, flat), + &imagerequest.Transform{Format: imagerequest.FormatPNG}) const rawBytes = width * height * 3 if size > rawBytes/10 { @@ -85,7 +89,8 @@ func TestImageProcessor_WebPAtEffort2(t *testing.T) { input := createTestJPEG(t, 200, 150) - size := processedSize(t, input, &Request{Format: FormatWebP, Quality: 85}) + size := processedSize(t, input, + &imagerequest.Transform{Format: imagerequest.FormatWebP, Quality: 85}) want, _, err := decode(t, input).ExportWebp(&vips.WebpExportParams{ StripMetadata: true, @@ -126,7 +131,8 @@ func TestImageProcessor_AVIFAtEffort1(t *testing.T) { input := encodePNG(t, source) - size := processedSize(t, input, &Request{Format: FormatAVIF, Quality: 85}) + size := processedSize(t, input, + &imagerequest.Transform{Format: imagerequest.FormatAVIF, Quality: 85}) want, _, err := decode(t, input).ExportAvif(&vips.AvifExportParams{ StripMetadata: true, @@ -152,7 +158,8 @@ func TestImageProcessor_JPEGXLAtEffort4(t *testing.T) { input := createTestJPEG(t, 200, 150) - size := processedSize(t, input, &Request{Format: FormatJXL, Quality: 85}) + size := processedSize(t, input, + &imagerequest.Transform{Format: imagerequest.FormatJXL, Quality: 85}) params := vips.NewJxlExportParams() params.Quality = 85 diff --git a/internal/imageprocessor/fit_cover_internal_test.go b/internal/imageprocessor/fit_cover_internal_test.go index 0b01211..030074e 100644 --- a/internal/imageprocessor/fit_cover_internal_test.go +++ b/internal/imageprocessor/fit_cover_internal_test.go @@ -7,6 +7,8 @@ import ( "image/png" "math" "testing" + + "sneak.berlin/go/pixa/internal/imagerequest" ) // TestImageProcessor_FitCoverServesThinSource verifies that FitCover serves @@ -20,12 +22,12 @@ func TestImageProcessor_FitCoverServesThinSource(t *testing.T) { tests := []struct { name string inputWidth, inputHeight int - size Size + size imagerequest.Size }{ - {"8192x1 at 8192x8192", 8192, 1, Size{Width: 8192, Height: 8192}}, - {"8192x1 at 1x8192", 8192, 1, Size{Width: 1, Height: 8192}}, - {"1x8192 at 8192x8192", 1, 8192, Size{Width: 8192, Height: 8192}}, - {"1x8192 at 8192x1", 1, 8192, Size{Width: 8192, Height: 1}}, + {"8192x1 at 8192x8192", 8192, 1, imagerequest.Size{Width: 8192, Height: 8192}}, + {"8192x1 at 1x8192", 8192, 1, imagerequest.Size{Width: 1, Height: 8192}}, + {"1x8192 at 8192x8192", 1, 8192, imagerequest.Size{Width: 8192, Height: 8192}}, + {"1x8192 at 8192x1", 1, 8192, imagerequest.Size{Width: 8192, Height: 1}}, } for _, tt := range tests { @@ -36,7 +38,11 @@ func TestImageProcessor_FitCoverServesThinSource(t *testing.T) { result, err := New(Params{}).Process(t.Context(), bytes.NewReader(input), - &Request{Size: tt.size, Format: FormatJPEG, FitMode: FitCover}) + &imagerequest.Transform{ + Size: tt.size, + Format: imagerequest.FormatJPEG, + FitMode: imagerequest.FitCover, + }) if err != nil { t.Fatalf("Process() error = %v", err) } @@ -102,10 +108,10 @@ func TestImageProcessor_FitCoverCropsTheCentre(t *testing.T) { result, err := New(Params{}).Process(t.Context(), bytes.NewReader(input), - &Request{ - Size: Size{Width: 50, Height: 50}, - Format: FormatPNG, - FitMode: FitCover, + &imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatPNG, + FitMode: imagerequest.FitCover, }) if err != nil { t.Fatalf("Process() error = %v", err) @@ -208,14 +214,14 @@ func TestImageProcessor_FitCoverKeepsProportions(t *testing.T) { tests := []struct { name string side, squareSide int - size Size + size imagerequest.Size }{ - {"16x16 at 100x60", 16, 8, Size{Width: 100, Height: 60}}, - {"16x16 at 60x100", 16, 8, Size{Width: 60, Height: 100}}, - {"10x10 at 100x35", 10, 2, Size{Width: 100, Height: 35}}, - {"10x10 at 35x100", 10, 2, Size{Width: 35, Height: 100}}, - {"64x64 at 300x100", 64, 16, Size{Width: 300, Height: 100}}, - {"64x64 at 100x300", 64, 16, Size{Width: 100, Height: 300}}, + {"16x16 at 100x60", 16, 8, imagerequest.Size{Width: 100, Height: 60}}, + {"16x16 at 60x100", 16, 8, imagerequest.Size{Width: 60, Height: 100}}, + {"10x10 at 100x35", 10, 2, imagerequest.Size{Width: 100, Height: 35}}, + {"10x10 at 35x100", 10, 2, imagerequest.Size{Width: 35, Height: 100}}, + {"64x64 at 300x100", 64, 16, imagerequest.Size{Width: 300, Height: 100}}, + {"64x64 at 100x300", 64, 16, imagerequest.Size{Width: 100, Height: 300}}, } for _, tt := range tests { @@ -226,7 +232,11 @@ func TestImageProcessor_FitCoverKeepsProportions(t *testing.T) { result, err := New(Params{}).Process(t.Context(), bytes.NewReader(input), - &Request{Size: tt.size, Format: FormatPNG, FitMode: FitCover}) + &imagerequest.Transform{ + Size: tt.size, + Format: imagerequest.FormatPNG, + FitMode: imagerequest.FitCover, + }) if err != nil { t.Fatalf("Process() error = %v", err) } diff --git a/internal/imageprocessor/fit_outside_internal_test.go b/internal/imageprocessor/fit_outside_internal_test.go index 62d9674..3d331a5 100644 --- a/internal/imageprocessor/fit_outside_internal_test.go +++ b/internal/imageprocessor/fit_outside_internal_test.go @@ -4,6 +4,8 @@ import ( "bytes" "errors" "testing" + + "sneak.berlin/go/pixa/internal/imagerequest" ) // TestImageProcessor_RefusesFitOutsideAboveMaximum verifies that FitOutside, @@ -17,15 +19,20 @@ func TestImageProcessor_RefusesFitOutsideAboveMaximum(t *testing.T) { tests := []struct { name string inputWidth, inputHeight int - size Size + size imagerequest.Size // wantWidth and wantHeight are the output's, 0 for a refusal wantWidth, wantHeight int }{ - {"width above the maximum", 3000, 600, Size{Width: 1, Height: 8192}, 0, 0}, - {"height above the maximum", 600, 3000, Size{Width: 8192, Height: 1}, 0, 0}, - {"width at the maximum", 1024, 256, Size{Width: 1, Height: 2048}, 8192, 2048}, - {"height at the maximum", 256, 1024, Size{Width: 2048, Height: 1}, 2048, 8192}, - {"within the maximum", 3000, 600, Size{Width: 1, Height: 1200}, 6000, 1200}, + {"width above the maximum", 3000, 600, + imagerequest.Size{Width: 1, Height: 8192}, 0, 0}, + {"height above the maximum", 600, 3000, + imagerequest.Size{Width: 8192, Height: 1}, 0, 0}, + {"width at the maximum", 1024, 256, + imagerequest.Size{Width: 1, Height: 2048}, 8192, 2048}, + {"height at the maximum", 256, 1024, + imagerequest.Size{Width: 2048, Height: 1}, 2048, 8192}, + {"within the maximum", 3000, 600, + imagerequest.Size{Width: 1, Height: 1200}, 6000, 1200}, } for _, tt := range tests { @@ -36,7 +43,11 @@ func TestImageProcessor_RefusesFitOutsideAboveMaximum(t *testing.T) { result, err := New(Params{}).Process(t.Context(), bytes.NewReader(input), - &Request{Size: tt.size, Format: FormatJPEG, FitMode: FitOutside}) + &imagerequest.Transform{ + Size: tt.size, + Format: imagerequest.FormatJPEG, + FitMode: imagerequest.FitOutside, + }) if result != nil { t.Logf("Process() made %dx%d", result.Width, result.Height) } @@ -71,11 +82,11 @@ func TestImageProcessor_FitOutsideSideIsAtLeastOne(t *testing.T) { tests := []struct { name string inputWidth, inputHeight int - size Size + size imagerequest.Size wantWidth, wantHeight int }{ - {"49x8192 at 0x100", 49, 8192, Size{Height: 100}, 1, 167}, - {"49x49 at 1x1", 49, 49, Size{Width: 1, Height: 1}, 1, 1}, + {"49x8192 at 0x100", 49, 8192, imagerequest.Size{Height: 100}, 1, 167}, + {"49x49 at 1x1", 49, 49, imagerequest.Size{Width: 1, Height: 1}, 1, 1}, } for _, tt := range tests { @@ -86,7 +97,11 @@ func TestImageProcessor_FitOutsideSideIsAtLeastOne(t *testing.T) { result, err := New(Params{}).Process(t.Context(), bytes.NewReader(input), - &Request{Size: tt.size, Format: FormatJPEG, FitMode: FitOutside}) + &imagerequest.Transform{ + Size: tt.size, + Format: imagerequest.FormatJPEG, + FitMode: imagerequest.FitOutside, + }) if err != nil { t.Fatalf("Process() error = %v", err) } diff --git a/internal/imageprocessor/imageprocessor.go b/internal/imageprocessor/imageprocessor.go index c50452b..c344eea 100644 --- a/internal/imageprocessor/imageprocessor.go +++ b/internal/imageprocessor/imageprocessor.go @@ -12,6 +12,7 @@ import ( "github.com/davidbyttow/govips/v2/vips" "sneak.berlin/go/pixa/internal/config" + "sneak.berlin/go/pixa/internal/imagerequest" ) // vipsOnce ensures vips is initialized exactly once. @@ -54,49 +55,6 @@ func CheckJPEGXLSupport() error { return nil } -// Format represents supported output image formats. -type Format string - -// Supported image output formats. -const ( - FormatOriginal Format = "orig" - FormatJPEG Format = "jpeg" - FormatPNG Format = "png" - FormatWebP Format = "webp" - FormatAVIF Format = "avif" - FormatJXL Format = "jxl" - FormatGIF Format = "gif" -) - -// FitMode represents how to fit an image into requested dimensions. -type FitMode string - -// Supported image fit modes. -const ( - FitCover FitMode = "cover" - FitContain FitMode = "contain" - FitFill FitMode = "fill" - FitInside FitMode = "inside" - FitOutside FitMode = "outside" -) - -// ErrInvalidFitMode is returned when an invalid fit mode is provided. -var ErrInvalidFitMode = errors.New("invalid fit mode") - -// Size represents requested image dimensions. -type Size struct { - Width int - Height int -} - -// Request holds the parameters for image processing. -type Request struct { - Size Size - Format Format - Quality int - FitMode FitMode -} - // Result contains the output of image processing. type Result struct { // Content is the processed image data. @@ -133,8 +91,8 @@ var ErrInputTooLarge = errors.New("input image dimensions exceed maximum") var ErrInputDataTooLarge = errors.New("input data exceeds maximum allowed size") // MaxOutputDimension is the maximum width or height of an output image, and -// so of a requested size (imgcache.MaxDimension). -const MaxOutputDimension = 8192 +// so of a requested size (imagerequest.MaxDimension). +const MaxOutputDimension = imagerequest.MaxDimension // ErrOutputTooLarge is returned, before resizing, when the output would have a // side above MaxOutputDimension: the side of a size with one side 0, computed @@ -217,7 +175,7 @@ func New(params Params) *ImageProcessor { } } -// Process transforms an image according to the request. When +// Process transforms an image according to transform. When // MaxConcurrentProcessing images are already being processed, it waits up // to ProcessingWaitTimeout for one to finish, then fails with // ErrTooManyImages. When ctx has ended once Process has a slot, or before it @@ -228,7 +186,7 @@ func New(params Params) *ImageProcessor { func (p *ImageProcessor) Process( ctx context.Context, input io.Reader, - req *Request, + transform *imagerequest.Transform, ) (*Result, error) { release, err := p.acquireSlot(ctx) if err != nil { @@ -277,35 +235,36 @@ func (p *ImageProcessor) Process( } // Determine target dimensions; too large ones are refused before resizing - targetWidth, targetHeight, err := targetDimensions(req.Size, origWidth, origHeight) + targetWidth, targetHeight, err := targetDimensions(transform.Size, + origWidth, origHeight) if err != nil { return nil, err } // orig is the source's own format; encode refuses an empty format - outputFormat := req.Format - if outputFormat == FormatOriginal { + outputFormat := transform.Format + if outputFormat == imagerequest.FormatOriginal { outputFormat = sourceFormat } // Determine output dimensions; too large ones are refused before // resizing, which with FitCover already decodes the input outputWidth, outputHeight, err := outputDimensions(origWidth, origHeight, - targetWidth, targetHeight, req.FitMode, outputFormat) + targetWidth, targetHeight, transform.FitMode, outputFormat) if err != nil { return nil, err } // Resize if needed if targetWidth != origWidth || targetHeight != origHeight { - err := p.resize(img, outputWidth, outputHeight, req.FitMode) + err := p.resize(img, outputWidth, outputHeight, transform.FitMode) if err != nil { return nil, fmt.Errorf("failed to resize: %w", err) } } // Encode to target format - output, err := p.encode(ctx, img, outputFormat, req.Quality) + output, err := p.encode(ctx, img, outputFormat, transform.Quality) if err != nil { return nil, fmt.Errorf("failed to encode: %w", err) } @@ -328,7 +287,9 @@ func (p *ImageProcessor) Process( // rounded down but kept at 1 or more, as for a thin image it would round // down to 0, a side of no pixels. It can come out above MaxOutputDimension, // which is ErrOutputTooLarge. -func targetDimensions(size Size, origWidth, origHeight int) (int, int, error) { +func targetDimensions( + size imagerequest.Size, origWidth, origHeight int, +) (int, int, error) { width, height := size.Width, size.Height switch { @@ -374,33 +335,33 @@ func (p *ImageProcessor) SupportedInputFormats() []string { } // SupportedOutputFormats returns formats this processor can write. -func (p *ImageProcessor) SupportedOutputFormats() []Format { - return []Format{ - FormatJPEG, - FormatPNG, - FormatGIF, - FormatWebP, - FormatAVIF, - FormatJXL, +func (p *ImageProcessor) SupportedOutputFormats() []imagerequest.Format { + return []imagerequest.Format{ + imagerequest.FormatJPEG, + imagerequest.FormatPNG, + imagerequest.FormatGIF, + imagerequest.FormatWebP, + imagerequest.FormatAVIF, + imagerequest.FormatJXL, } } // FormatToMIME converts a Format to its MIME type string. -func FormatToMIME(format Format) string { +func FormatToMIME(format imagerequest.Format) string { switch format { - case FormatJPEG: + case imagerequest.FormatJPEG: return mimeJPEG - case FormatPNG: + case imagerequest.FormatPNG: return mimePNG - case FormatWebP: + case imagerequest.FormatWebP: return mimeWebP - case FormatGIF: + case imagerequest.FormatGIF: return mimeGIF - case FormatAVIF: + case imagerequest.FormatAVIF: return mimeAVIF - case FormatJXL: + case imagerequest.FormatJXL: return mimeJXL - case FormatOriginal: + case imagerequest.FormatOriginal, imagerequest.FormatAuto: return "application/octet-stream" default: return "application/octet-stream" @@ -487,9 +448,9 @@ func (p *ImageProcessor) detectFormat(imageType vips.ImageType) string { case vips.ImageTypeWEBP: return "webp" case vips.ImageTypeAVIF, vips.ImageTypeHEIF: - return string(FormatAVIF) + return string(imagerequest.FormatAVIF) case vips.ImageTypeJXL: - return string(FormatJXL) + return string(imagerequest.FormatJXL) case vips.ImageTypeUnknown, vips.ImageTypeMagick, vips.ImageTypePDF, vips.ImageTypeSVG, vips.ImageTypeTIFF, vips.ImageTypeBMP, vips.ImageTypeJP2K: @@ -506,17 +467,18 @@ func (p *ImageProcessor) detectFormat(imageType vips.ImageType) string { // outputDimensions returns ErrOutputTooLarge. An AVIF output of more than // MaxAVIFOutputPixels is ErrAVIFOutputTooLarge. func outputDimensions( - imgWidth, imgHeight, width, height int, fit FitMode, format Format, + imgWidth, imgHeight, width, height int, + fit imagerequest.Fit, format imagerequest.Format, ) (int, int, error) { scaleW := float64(width) / float64(imgWidth) scaleH := float64(height) / float64(imgHeight) outputWidth, outputHeight := width, height switch fit { - case FitCover, FitFill, "": + case imagerequest.FitCover, imagerequest.FitFill, "": // Cover crops, and fill stretches, to exactly the target dimensions - case FitContain: + case imagerequest.FitContain: // Fit within the target dimensions, maintaining aspect ratio. Scaled // by the smaller scale, the short side of a thin image can round // down to 0, which libvips refuses as a width, so each side is at @@ -525,13 +487,13 @@ func outputDimensions( outputWidth = max(1, int(float64(imgWidth)*scale)) outputHeight = max(1, int(float64(imgHeight)*scale)) - case FitInside: + case imagerequest.FitInside: // Same as contain, but only shrink scale := min(scaleW, scaleH, 1) outputWidth = max(1, int(float64(imgWidth)*scale)) outputHeight = max(1, int(float64(imgHeight)*scale)) - case FitOutside: + case imagerequest.FitOutside: // The smallest dimension fits, the other may exceed the target. A // side scaled to 1 can come out just under 1 in floating point, as // 49 times 1/49 does, and round down to 0, which libvips refuses as @@ -541,7 +503,7 @@ func outputDimensions( outputHeight = max(1, int(float64(imgHeight)*scale)) default: - return 0, 0, fmt.Errorf("%w: %s", ErrInvalidFitMode, fit) + return 0, 0, fmt.Errorf("%w: %s", imagerequest.ErrInvalidFitMode, fit) } // The side FitOutside scales past the target is held to the maximum too @@ -550,7 +512,8 @@ func outputDimensions( outputWidth, outputHeight, MaxOutputDimension) } - if format == FormatAVIF && outputWidth*outputHeight > MaxAVIFOutputPixels { + if format == imagerequest.FormatAVIF && + outputWidth*outputHeight > MaxAVIFOutputPixels { return 0, 0, fmt.Errorf("%w: %dx%d is above %d pixels", ErrAVIFOutputTooLarge, outputWidth, outputHeight, MaxAVIFOutputPixels) } @@ -561,18 +524,18 @@ func outputDimensions( // resize resizes the image to width x height, the dimensions // outputDimensions gives for the fit mode. func (p *ImageProcessor) resize( - img *vips.ImageRef, width, height int, fit FitMode, + img *vips.ImageRef, width, height int, fit imagerequest.Fit, ) error { switch fit { - case FitCover, "": + case imagerequest.FitCover, "": // Crop the centre and resize to fill exact dimensions (default) return cropCentreAndResize(img, width, height) - case FitFill: + case imagerequest.FitFill: // Resize to exact dimensions (may distort) return img.ThumbnailWithSize(width, height, vips.InterestingNone, vips.SizeForce) - case FitInside: + case imagerequest.FitInside: // outputDimensions keeps the dimensions of an image that already fits if img.Width() == width && img.Height() == height { return nil @@ -580,11 +543,11 @@ func (p *ImageProcessor) resize( return img.Thumbnail(width, height, vips.InterestingNone) - case FitContain, FitOutside: + case imagerequest.FitContain, imagerequest.FitOutside: return img.Thumbnail(width, height, vips.InterestingNone) default: - return fmt.Errorf("%w: %s", ErrInvalidFitMode, fit) + return fmt.Errorf("%w: %s", imagerequest.ErrInvalidFitMode, fit) } } @@ -626,7 +589,8 @@ const defaultQuality = 85 // to stop libvips once an encode has begun, so when ctx has ended encode // returns ctx's error without beginning one. func (p *ImageProcessor) encode( - ctx context.Context, img *vips.ImageRef, format Format, quality int, + ctx context.Context, img *vips.ImageRef, format imagerequest.Format, + quality int, ) ([]byte, error) { err := ctx.Err() if err != nil { @@ -649,25 +613,25 @@ func (p *ImageProcessor) encode( } switch format { - case FormatJPEG: + case imagerequest.FormatJPEG: return exportJPEG(img, quality) - case FormatPNG: + case imagerequest.FormatPNG: return exportPNG(img) - case FormatGIF: + case imagerequest.FormatGIF: return exportGIF(img) - case FormatWebP: + case imagerequest.FormatWebP: return exportWebP(img, quality) - case FormatAVIF: + case imagerequest.FormatAVIF: return exportAVIF(img, quality) - case FormatJXL: + case imagerequest.FormatJXL: return exportJXL(img, quality) - case FormatOriginal: + case imagerequest.FormatOriginal, imagerequest.FormatAuto: return nil, fmt.Errorf("%w: %s", ErrUnsupportedOutputFormat, format) default: @@ -849,20 +813,22 @@ func jxlDistance(quality int) float64 { // formatFromString converts a format string from detectFormat to Format. A // format detectFormat could not name is ErrUnsupportedInputFormat. -func (p *ImageProcessor) formatFromString(format string) (Format, error) { +func (p *ImageProcessor) formatFromString( + format string, +) (imagerequest.Format, error) { switch format { case "jpeg": - return FormatJPEG, nil + return imagerequest.FormatJPEG, nil case "png": - return FormatPNG, nil + return imagerequest.FormatPNG, nil case "gif": - return FormatGIF, nil + return imagerequest.FormatGIF, nil case "webp": - return FormatWebP, nil - case string(FormatAVIF): - return FormatAVIF, nil - case string(FormatJXL): - return FormatJXL, nil + return imagerequest.FormatWebP, nil + case string(imagerequest.FormatAVIF): + return imagerequest.FormatAVIF, nil + case string(imagerequest.FormatJXL): + return imagerequest.FormatJXL, nil default: return "", fmt.Errorf("%w: %s", ErrUnsupportedInputFormat, format) } diff --git a/internal/imageprocessor/imageprocessor_internal_test.go b/internal/imageprocessor/imageprocessor_internal_test.go index 73decd0..f5a865e 100644 --- a/internal/imageprocessor/imageprocessor_internal_test.go +++ b/internal/imageprocessor/imageprocessor_internal_test.go @@ -15,6 +15,7 @@ import ( "testing" "github.com/davidbyttow/govips/v2/vips" + "sneak.berlin/go/pixa/internal/imagerequest" ) func TestMain(m *testing.M) { @@ -87,7 +88,7 @@ func isAVIF(data []byte) bool { brand := string(data[8:12]) - return brand == string(FormatAVIF) || brand == "avis" + return brand == string(imagerequest.FormatAVIF) || brand == "avis" } // detectMIME is a minimal magic-byte detector for test assertions. @@ -123,14 +124,14 @@ func TestImageProcessor_ResizeJPEG(t *testing.T) { input := createTestJPEG(t, 800, 600) - req := &Request{ - Size: Size{Width: 400, Height: 300}, - Format: FormatJPEG, + transform := &imagerequest.Transform{ + Size: imagerequest.Size{Width: 400, Height: 300}, + Format: imagerequest.FormatJPEG, Quality: 85, - FitMode: FitCover, + FitMode: imagerequest.FitCover, } - result, err := proc.Process(ctx, bytes.NewReader(input), req) + result, err := proc.Process(ctx, bytes.NewReader(input), transform) if err != nil { t.Fatalf("Process() error = %v", err) } @@ -169,13 +170,13 @@ func TestImageProcessor_ConvertToPNG(t *testing.T) { input := createTestJPEG(t, 200, 150) - req := &Request{ - Size: Size{Width: 200, Height: 150}, - Format: FormatPNG, - FitMode: FitCover, + transform := &imagerequest.Transform{ + Size: imagerequest.Size{Width: 200, Height: 150}, + Format: imagerequest.FormatPNG, + FitMode: imagerequest.FitCover, } - result, err := proc.Process(ctx, bytes.NewReader(input), req) + result, err := proc.Process(ctx, bytes.NewReader(input), transform) if err != nil { t.Fatalf("Process() error = %v", err) } @@ -196,7 +197,7 @@ func TestImageProcessor_ConvertToPNG(t *testing.T) { // processAndCheckSize processes a test JPEG of the given input dimensions // with the requested size and asserts the resulting dimensions. func processAndCheckSize( - t *testing.T, inputW, inputH int, size Size, wantW, wantH int, + t *testing.T, inputW, inputH int, size imagerequest.Size, wantW, wantH int, ) { t.Helper() @@ -205,14 +206,14 @@ func processAndCheckSize( input := createTestJPEG(t, inputW, inputH) - req := &Request{ + transform := &imagerequest.Transform{ Size: size, - Format: FormatJPEG, + Format: imagerequest.FormatJPEG, Quality: 85, - FitMode: FitCover, + FitMode: imagerequest.FitCover, } - result, err := proc.Process(ctx, bytes.NewReader(input), req) + result, err := proc.Process(ctx, bytes.NewReader(input), transform) if err != nil { t.Fatalf("Process() error = %v", err) } @@ -232,7 +233,7 @@ func TestImageProcessor_OriginalSize(t *testing.T) { t.Parallel() // Width and height 0: keep original size - processAndCheckSize(t, 640, 480, Size{Width: 0, Height: 0}, 640, 480) + processAndCheckSize(t, 640, 480, imagerequest.Size{Width: 0, Height: 0}, 640, 480) } func TestImageProcessor_FitContain(t *testing.T) { @@ -245,14 +246,14 @@ func TestImageProcessor_FitContain(t *testing.T) { // Should result in 400x200 (maintaining aspect ratio) input := createTestJPEG(t, 800, 400) - req := &Request{ - Size: Size{Width: 400, Height: 400}, - Format: FormatJPEG, + transform := &imagerequest.Transform{ + Size: imagerequest.Size{Width: 400, Height: 400}, + Format: imagerequest.FormatJPEG, Quality: 85, - FitMode: FitContain, + FitMode: imagerequest.FitContain, } - result, err := proc.Process(ctx, bytes.NewReader(input), req) + result, err := proc.Process(ctx, bytes.NewReader(input), transform) if err != nil { t.Fatalf("Process() error = %v", err) } @@ -270,7 +271,7 @@ func TestImageProcessor_ProportionalScale_WidthOnly(t *testing.T) { // 800x600 image, request width=400 height=0 // Should scale proportionally to 400x300 - processAndCheckSize(t, 800, 600, Size{Width: 400, Height: 0}, 400, 300) + processAndCheckSize(t, 800, 600, imagerequest.Size{Width: 400, Height: 0}, 400, 300) } func TestImageProcessor_ProportionalScale_HeightOnly(t *testing.T) { @@ -278,7 +279,7 @@ func TestImageProcessor_ProportionalScale_HeightOnly(t *testing.T) { // 800x600 image, request width=0 height=300 // Should scale proportionally to 400x300 - processAndCheckSize(t, 800, 600, Size{Width: 0, Height: 300}, 400, 300) + processAndCheckSize(t, 800, 600, imagerequest.Size{Width: 0, Height: 300}, 400, 300) } func TestImageProcessor_ProcessPNG(t *testing.T) { @@ -289,13 +290,13 @@ func TestImageProcessor_ProcessPNG(t *testing.T) { input := createTestPNG(t, 400, 300) - req := &Request{ - Size: Size{Width: 200, Height: 150}, - Format: FormatPNG, - FitMode: FitCover, + transform := &imagerequest.Transform{ + Size: imagerequest.Size{Width: 200, Height: 150}, + Format: imagerequest.FormatPNG, + FitMode: imagerequest.FitCover, } - result, err := proc.Process(ctx, bytes.NewReader(input), req) + result, err := proc.Process(ctx, bytes.NewReader(input), transform) if err != nil { t.Fatalf("Process() error = %v", err) } @@ -349,14 +350,14 @@ func TestImageProcessor_RejectsOversizedInput(t *testing.T) { ctx := context.Background() input := createTestJPEG(t, tt.width, tt.height) - req := &Request{ - Size: Size{Width: 100, Height: 100}, - Format: FormatJPEG, + transform := &imagerequest.Transform{ + Size: imagerequest.Size{Width: 100, Height: 100}, + Format: imagerequest.FormatJPEG, Quality: 85, - FitMode: FitCover, + FitMode: imagerequest.FitCover, } - _, err := proc.Process(ctx, bytes.NewReader(input), req) + _, err := proc.Process(ctx, bytes.NewReader(input), transform) if err == nil { t.Error("Process() should reject oversized input images") } @@ -377,14 +378,14 @@ func TestImageProcessor_AcceptsMaxDimensionInput(t *testing.T) { // Create an image at exactly MaxInputDimension - should be accepted input := createTestJPEG(t, MaxInputDimension, 100) - req := &Request{ - Size: Size{Width: 100, Height: 100}, - Format: FormatJPEG, + transform := &imagerequest.Transform{ + Size: imagerequest.Size{Width: 100, Height: 100}, + Format: imagerequest.FormatJPEG, Quality: 85, - FitMode: FitCover, + FitMode: imagerequest.FitCover, } - result, err := proc.Process(ctx, bytes.NewReader(input), req) + result, err := proc.Process(ctx, bytes.NewReader(input), transform) if err != nil { t.Fatalf( "Process() should accept images at MaxInputDimension, got error: %v", @@ -397,7 +398,9 @@ func TestImageProcessor_AcceptsMaxDimensionInput(t *testing.T) { // encodeAndCheck processes a 200x150 test JPEG into a 100x75 output of the // given format and asserts the output MIME type and dimensions. -func encodeAndCheck(t *testing.T, format Format, quality int, wantMIME string) { +func encodeAndCheck( + t *testing.T, format imagerequest.Format, quality int, wantMIME string, +) { t.Helper() proc := New(Params{}) @@ -405,14 +408,14 @@ func encodeAndCheck(t *testing.T, format Format, quality int, wantMIME string) { input := createTestJPEG(t, 200, 150) - req := &Request{ - Size: Size{Width: 100, Height: 75}, + transform := &imagerequest.Transform{ + Size: imagerequest.Size{Width: 100, Height: 75}, Format: format, Quality: quality, - FitMode: FitCover, + FitMode: imagerequest.FitCover, } - result, err := proc.Process(ctx, bytes.NewReader(input), req) + result, err := proc.Process(ctx, bytes.NewReader(input), transform) if err != nil { t.Fatalf("Process() error = %v, want nil", err) } @@ -443,7 +446,7 @@ func encodeAndCheck(t *testing.T, format Format, quality int, wantMIME string) { func TestImageProcessor_EncodeWebP(t *testing.T) { t.Parallel() - encodeAndCheck(t, FormatWebP, 80, mimeWebP) + encodeAndCheck(t, imagerequest.FormatWebP, 80, mimeWebP) } func TestImageProcessor_DecodeAVIF(t *testing.T) { @@ -459,14 +462,14 @@ func TestImageProcessor_DecodeAVIF(t *testing.T) { } // Request resize and convert to JPEG - req := &Request{ - Size: Size{Width: 2, Height: 2}, - Format: FormatJPEG, + transform := &imagerequest.Transform{ + Size: imagerequest.Size{Width: 2, Height: 2}, + Format: imagerequest.FormatJPEG, Quality: 85, - FitMode: FitCover, + FitMode: imagerequest.FitCover, } - result, err := proc.Process(ctx, bytes.NewReader(input), req) + result, err := proc.Process(ctx, bytes.NewReader(input), transform) if err != nil { t.Fatalf("Process() error = %v, want nil (AVIF decoding should work)", err) } @@ -500,14 +503,14 @@ func TestImageProcessor_RejectsOversizedInputData(t *testing.T) { t.Fatalf("test JPEG must exceed %d bytes, got %d", limit, len(input)) } - req := &Request{ - Size: Size{Width: 100, Height: 75}, - Format: FormatJPEG, + transform := &imagerequest.Transform{ + Size: imagerequest.Size{Width: 100, Height: 75}, + Format: imagerequest.FormatJPEG, Quality: 85, - FitMode: FitCover, + FitMode: imagerequest.FitCover, } - _, err := proc.Process(ctx, bytes.NewReader(input), req) + _, err := proc.Process(ctx, bytes.NewReader(input), transform) if err == nil { t.Fatal("Process() should reject input exceeding maxInputBytes") } @@ -527,14 +530,14 @@ func TestImageProcessor_AcceptsInputWithinLimit(t *testing.T) { proc := New(Params{MaxInputBytes: limit}) ctx := context.Background() - req := &Request{ - Size: Size{Width: 10, Height: 10}, - Format: FormatJPEG, + transform := &imagerequest.Transform{ + Size: imagerequest.Size{Width: 10, Height: 10}, + Format: imagerequest.FormatJPEG, Quality: 85, - FitMode: FitCover, + FitMode: imagerequest.FitCover, } - result, err := proc.Process(ctx, bytes.NewReader(input), req) + result, err := proc.Process(ctx, bytes.NewReader(input), transform) if err != nil { t.Fatalf("Process() error = %v, want nil", err) } @@ -561,16 +564,18 @@ func TestImageProcessor_DefaultMaxInputBytes(t *testing.T) { func TestImageProcessor_EncodeAVIF(t *testing.T) { t.Parallel() - encodeAndCheck(t, FormatAVIF, 85, mimeAVIF) + encodeAndCheck(t, imagerequest.FormatAVIF, 85, mimeAVIF) } // processAndDecode runs input through Process and decodes the output with // vips, so a test can inspect the image a client would receive. -func processAndDecode(t *testing.T, input []byte, req *Request) *vips.ImageRef { +func processAndDecode( + t *testing.T, input []byte, transform *imagerequest.Transform, +) *vips.ImageRef { t.Helper() result, err := New(Params{}).Process( - context.Background(), bytes.NewReader(input), req, + context.Background(), bytes.NewReader(input), transform, ) if err != nil { t.Fatalf("Process() error = %v", err) @@ -614,15 +619,17 @@ func TestImageProcessor_StripsEXIF(t *testing.T) { t.Fatal("testdata/gps-exif.jpg has no GPS latitude") } - formats := []Format{ - FormatJPEG, FormatPNG, FormatWebP, FormatAVIF, FormatGIF, FormatOriginal, + formats := []imagerequest.Format{ + imagerequest.FormatJPEG, imagerequest.FormatPNG, imagerequest.FormatWebP, + imagerequest.FormatAVIF, imagerequest.FormatGIF, imagerequest.FormatOriginal, } for _, format := range formats { t.Run(string(format), func(t *testing.T) { t.Parallel() - output := processAndDecode(t, input, &Request{Format: format}) + output := processAndDecode(t, input, + &imagerequest.Transform{Format: format}) if output.HasExif() { t.Errorf("output has EXIF: %v", output.GetExif()) @@ -644,21 +651,21 @@ func TestImageProcessor_AppliesEXIFOrientation(t *testing.T) { tests := []struct { name string - size Size + size imagerequest.Size wantW int wantH int }{ - {name: "original size", size: Size{}, wantW: 8, wantH: 16}, - {name: "width only", size: Size{Width: 4}, wantW: 4, wantH: 8}, + {name: "original size", size: imagerequest.Size{}, wantW: 8, wantH: 16}, + {name: "width only", size: imagerequest.Size{Width: 4}, wantW: 4, wantH: 8}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - output := processAndDecode(t, input, &Request{ + output := processAndDecode(t, input, &imagerequest.Transform{ Size: tt.size, - Format: FormatPNG, + Format: imagerequest.FormatPNG, }) if output.Width() != tt.wantW || output.Height() != tt.wantH { @@ -694,7 +701,8 @@ func TestImageProcessor_ConvertsWideGamutToSRGB(t *testing.T) { t.Fatalf("failed to read test JPEG: %v", err) } - output := processAndDecode(t, input, &Request{Format: FormatPNG}) + output := processAndDecode(t, input, + &imagerequest.Transform{Format: imagerequest.FormatPNG}) if output.HasICCProfile() { t.Error("output has an ICC profile") diff --git a/internal/imageprocessor/jpegxl_output_internal_test.go b/internal/imageprocessor/jpegxl_output_internal_test.go index 71f8f14..44b3f6a 100644 --- a/internal/imageprocessor/jpegxl_output_internal_test.go +++ b/internal/imageprocessor/jpegxl_output_internal_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/davidbyttow/govips/v2/vips" + "sneak.berlin/go/pixa/internal/imagerequest" ) // TestImageProcessor_EncodeJPEGXL converts a JPEG to a smaller JPEG XL and @@ -15,10 +16,13 @@ import ( func TestImageProcessor_EncodeJPEGXL(t *testing.T) { t.Parallel() - req := &Request{Size: Size{Width: 100, Height: 75}, Format: FormatJXL} + transform := &imagerequest.Transform{ + Size: imagerequest.Size{Width: 100, Height: 75}, + Format: imagerequest.FormatJXL, + } result, err := New(Params{}).Process( - t.Context(), bytes.NewReader(createTestJPEG(t, 200, 150)), req, + t.Context(), bytes.NewReader(createTestJPEG(t, 200, 150)), transform, ) if err != nil { t.Fatalf("Process() error = %v", err) @@ -61,7 +65,8 @@ func TestImageProcessor_JPEGXLQuality(t *testing.T) { for _, quality := range []int{30, 90} { result, err := New(Params{}).Process(t.Context(), - bytes.NewReader(input), &Request{Format: FormatJXL, Quality: quality}) + bytes.NewReader(input), + &imagerequest.Transform{Format: imagerequest.FormatJXL, Quality: quality}) if err != nil { t.Fatalf("Process() at quality %d error = %v", quality, err) } @@ -89,7 +94,8 @@ func TestImageProcessor_JPEGXLDropsSourceEXIF(t *testing.T) { t.Fatalf("failed to read test JPEG: %v", err) } - exif := processAndDecode(t, input, &Request{Format: FormatJXL}).GetExif() + exif := processAndDecode(t, input, + &imagerequest.Transform{Format: imagerequest.FormatJXL}).GetExif() for _, field := range []string{ "exif-ifd0-Make", "exif-ifd0-Model", "exif-ifd2-BodySerialNumber", @@ -115,7 +121,8 @@ func TestImageProcessor_JPEGXLDropsSourceResolution(t *testing.T) { t.Fatalf("failed to read test JPEG: %v", err) } - output := processAndDecode(t, input, &Request{Format: FormatJXL}) + output := processAndDecode(t, input, + &imagerequest.Transform{Format: imagerequest.FormatJXL}) // libvips gives the resolution in pixels per millimetre. xDPI := math.Round(output.ResX() * 25.4) @@ -140,7 +147,8 @@ func TestImageProcessor_JPEGXLAppliesEXIFOrientation(t *testing.T) { t.Fatalf("failed to read test JPEG: %v", err) } - output := processAndDecode(t, input, &Request{Format: FormatJXL}) + output := processAndDecode(t, input, + &imagerequest.Transform{Format: imagerequest.FormatJXL}) if output.Width() != 8 || output.Height() != 16 { t.Fatalf("output is %dx%d, want 8x16", output.Width(), output.Height()) @@ -176,7 +184,8 @@ func TestImageProcessor_JPEGXLConvertsWideGamutToSRGB(t *testing.T) { t.Fatalf("failed to read test JPEG: %v", err) } - output := processAndDecode(t, input, &Request{Format: FormatJXL}) + output := processAndDecode(t, input, + &imagerequest.Transform{Format: imagerequest.FormatJXL}) pixel, err := output.GetPoint(4, 4) if err != nil { @@ -203,7 +212,8 @@ func TestImageProcessor_JPEGXLFromCMYK(t *testing.T) { t.Fatalf("failed to read test JPEG: %v", err) } - output := processAndDecode(t, input, &Request{Format: FormatJXL}) + output := processAndDecode(t, input, + &imagerequest.Transform{Format: imagerequest.FormatJXL}) if output.Bands() != 3 { t.Fatalf("output has %d bands, want 3", output.Bands()) diff --git a/internal/imageprocessor/max_concurrent_processing_internal_test.go b/internal/imageprocessor/max_concurrent_processing_internal_test.go index 2dbbbbf..c3d428e 100644 --- a/internal/imageprocessor/max_concurrent_processing_internal_test.go +++ b/internal/imageprocessor/max_concurrent_processing_internal_test.go @@ -11,6 +11,8 @@ import ( "testing" "testing/iotest" "time" + + "sneak.berlin/go/pixa/internal/imagerequest" ) // errTestReadFailed is the error the unreadable test input returns. @@ -76,13 +78,13 @@ func (r *gatedReader) Read(p []byte) (int, error) { return n, err } -// smallJPEGRequest asks for a 5x5 JPEG. -func smallJPEGRequest() *Request { - return &Request{ - Size: Size{Width: 5, Height: 5}, - Format: FormatJPEG, +// smallJPEGTransform asks for a 5x5 JPEG. +func smallJPEGTransform() *imagerequest.Transform { + return &imagerequest.Transform{ + Size: imagerequest.Size{Width: 5, Height: 5}, + Format: imagerequest.FormatJPEG, Quality: 85, - FitMode: FitCover, + FitMode: imagerequest.FitCover, } } @@ -92,7 +94,7 @@ func processInBackground( proc *ImageProcessor, reader *gatedReader, results chan<- error, ) { go func() { - result, err := proc.Process(context.Background(), reader, smallJPEGRequest()) + result, err := proc.Process(context.Background(), reader, smallJPEGTransform()) if err == nil { _ = result.Content.Close() } @@ -237,7 +239,7 @@ func TestProcessWaitsThenFailsWhenNoSlotFrees(t *testing.T) { start := time.Now() _, err := proc.Process(context.Background(), bytes.NewReader(input), - smallJPEGRequest()) + smallJPEGTransform()) if !errors.Is(err, ErrTooManyImages) { t.Fatalf("Process() error = %v, want ErrTooManyImages", err) } @@ -263,38 +265,38 @@ func TestProcessReleasesSlotOnError(t *testing.T) { valid := createTestJPEG(t, 10, 10) - unsupported := smallJPEGRequest() + unsupported := smallJPEGTransform() unsupported.Format = "bmp" cases := []struct { - name string - input io.Reader - req *Request + name string + input io.Reader + transform *imagerequest.Transform // want is the error Process must return; nil means any error. want error }{ { - name: "input cannot be read", - input: iotest.ErrReader(errTestReadFailed), - req: smallJPEGRequest(), - want: errTestReadFailed, + name: "input cannot be read", + input: iotest.ErrReader(errTestReadFailed), + transform: smallJPEGTransform(), + want: errTestReadFailed, }, { - name: "input over the byte limit", - input: bytes.NewReader(createTestJPEG(t, 800, 600)), - req: smallJPEGRequest(), - want: ErrInputDataTooLarge, + name: "input over the byte limit", + input: bytes.NewReader(createTestJPEG(t, 800, 600)), + transform: smallJPEGTransform(), + want: ErrInputDataTooLarge, }, { - name: "input not an image", - input: strings.NewReader("not an image"), - req: smallJPEGRequest(), + name: "input not an image", + input: strings.NewReader("not an image"), + transform: smallJPEGTransform(), }, { - name: "output format not supported", - input: bytes.NewReader(valid), - req: unsupported, - want: ErrUnsupportedOutputFormat, + name: "output format not supported", + input: bytes.NewReader(valid), + transform: unsupported, + want: ErrUnsupportedOutputFormat, }, } @@ -305,7 +307,7 @@ func TestProcessReleasesSlotOnError(t *testing.T) { proc := New(Params{MaxInputBytes: 4096, MaxConcurrentProcessing: 1}) proc.processingWaitTimeout = 100 * time.Millisecond - _, err := proc.Process(context.Background(), tc.input, tc.req) + _, err := proc.Process(context.Background(), tc.input, tc.transform) if err == nil || (tc.want != nil && !errors.Is(err, tc.want)) { t.Fatalf("Process() error = %v, want %v", err, tc.want) } @@ -315,7 +317,7 @@ func TestProcessReleasesSlotOnError(t *testing.T) { } result, err := proc.Process(context.Background(), bytes.NewReader(valid), - smallJPEGRequest()) + smallJPEGTransform()) if err != nil { t.Fatalf("Process() after the error = %v, want nil", err) } diff --git a/internal/imageprocessor/orig_format_internal_test.go b/internal/imageprocessor/orig_format_internal_test.go index 453d176..a0ad8dc 100644 --- a/internal/imageprocessor/orig_format_internal_test.go +++ b/internal/imageprocessor/orig_format_internal_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/davidbyttow/govips/v2/vips" + "sneak.berlin/go/pixa/internal/imagerequest" ) // TestImageProcessor_OrigOfUnnamedFormatRefused verifies that orig of a TIFF @@ -27,7 +28,8 @@ func TestImageProcessor_OrigOfUnnamedFormatRefused(t *testing.T) { } result, err := New(Params{}).Process( - t.Context(), bytes.NewReader(tiff), &Request{Format: FormatOriginal}, + t.Context(), bytes.NewReader(tiff), + &imagerequest.Transform{Format: imagerequest.FormatOriginal}, ) if result != nil { t.Logf("Process() returned %s", result.ContentType) diff --git a/internal/imageprocessor/source_formats_internal_test.go b/internal/imageprocessor/source_formats_internal_test.go index 5f1dbbd..d724976 100644 --- a/internal/imageprocessor/source_formats_internal_test.go +++ b/internal/imageprocessor/source_formats_internal_test.go @@ -4,6 +4,8 @@ import ( "bytes" "io" "testing" + + "sneak.berlin/go/pixa/internal/imagerequest" ) // TestImageProcessor_LoadsEachSourceFormat verifies that an image in each of @@ -14,11 +16,13 @@ func TestImageProcessor_LoadsEachSourceFormat(t *testing.T) { proc := New(Params{}) - for _, format := range []Format{ - FormatJPEG, FormatPNG, FormatGIF, FormatWebP, FormatAVIF, FormatJXL, + for _, format := range []imagerequest.Format{ + imagerequest.FormatJPEG, imagerequest.FormatPNG, imagerequest.FormatGIF, + imagerequest.FormatWebP, imagerequest.FormatAVIF, imagerequest.FormatJXL, } { saved, err := proc.Process(t.Context(), - bytes.NewReader(createTestPNG(t, 20, 20)), &Request{Format: format}) + bytes.NewReader(createTestPNG(t, 20, 20)), + &imagerequest.Transform{Format: format}) if err != nil { t.Fatalf("saving the test image as %s: %v", format, err) } @@ -29,7 +33,7 @@ func TestImageProcessor_LoadsEachSourceFormat(t *testing.T) { } result, err := proc.Process(t.Context(), bytes.NewReader(source), - &Request{Format: FormatOriginal}) + &imagerequest.Transform{Format: imagerequest.FormatOriginal}) if err != nil { t.Errorf("orig of the %s image: %v", format, err) diff --git a/internal/imagerequest/imagerequest.go b/internal/imagerequest/imagerequest.go new file mode 100644 index 0000000..7b30a61 --- /dev/null +++ b/internal/imagerequest/imagerequest.go @@ -0,0 +1,147 @@ +// Package imagerequest holds the image request type, its parts, its checks, +// the parsing of the /v1/image/ path and the variant key. It imports no other +// package of this repo, so every package that handles an image request can +// import it. +package imagerequest + +import ( + "errors" + "net/url" + "time" +) + +// ErrInvalidFitMode is returned when an invalid fit mode is provided. +var ErrInvalidFitMode = errors.New("invalid fit mode") + +// Format represents supported output image formats. +type Format string + +// Supported image output formats. +const ( + FormatOriginal Format = "orig" + FormatJPEG Format = "jpeg" + FormatPNG Format = "png" + FormatWebP Format = "webp" + FormatAVIF Format = "avif" + FormatJXL Format = "jxl" + FormatGIF Format = "gif" + + // FormatAuto stands for JPEG XL, AVIF, WebP or JPEG, chosen for each + // request from its Accept header once the URL's signature or token has + // been checked; it is never processed or cached as itself. + FormatAuto Format = "auto" +) + +// Size represents requested image dimensions +type Size struct { + Width int + Height int +} + +// OriginalSize returns true if this represents "keep original size" +func (s Size) OriginalSize() bool { + return s.Width == 0 && s.Height == 0 +} + +// Fit represents how to fit image into requested dimensions. +type Fit string + +// Supported image fit modes. +const ( + FitCover Fit = "cover" + FitContain Fit = "contain" + FitFill Fit = "fill" + FitInside Fit = "inside" + FitOutside Fit = "outside" +) + +// ValidateFitMode checks if the given fit mode is valid. +// Returns ErrInvalidFitMode for unrecognized fit modes. +func ValidateFitMode(fit Fit) error { + switch fit { + case FitCover, FitContain, FitFill, FitInside, FitOutside, "": + return nil + default: + return ErrInvalidFitMode + } +} + +// ValidateImageRequest checks a request's width and height with +// ValidateDimension and its fit mode with ValidateFitMode. Both the plain +// /v1/image/ route and the encrypted /v1/e/ route validate through this +// function so a request from either source enforces identical bounds. +func ValidateImageRequest(req *Request) error { + err := ValidateDimension("width", req.Transform.Size.Width) + if err != nil { + return err + } + + err = ValidateDimension("height", req.Transform.Size.Height) + if err != nil { + return err + } + + return ValidateFitMode(req.Transform.FitMode) +} + +// Request represents a request for a processed image: the source to fetch +// and the transform to apply to it. +type Request struct { + // Source is what to fetch + Source Source + // Transform is what to do to it + Transform Transform + // Signature is the HMAC signature for non-allowlisted hosts + Signature string + // Expires is when the URL expires: the exp of a signed URL, or the expiry + // of an encrypted URL; the zero time if it has none + Expires time.Time + // AllowHTTP indicates whether HTTP (non-TLS) is allowed for this request + AllowHTTP bool +} + +// Source is the image a request fetches from the origin. +type Source struct { + // Host is the origin host (e.g., "cdn.example.com") + Host string + // Path is the path on the origin with its percent-escapes decoded, as + // url.URL's Path holds it (e.g., "/photos/my cat.jpg") + Path string + // Query is the optional query string for the origin URL as the URL + // writes it, percent-escapes included, as url.URL's RawQuery holds it + Query string +} + +// Transform is what a request does to its source; the image processor +// takes it alone. +type Transform struct { + // Size is the requested output dimensions + Size Size + // Format is the requested output format + Format Format + // Quality is the output quality (1-100) for lossy formats + Quality int + // FitMode is how to fit the image into requested dimensions + FitMode Fit +} + +// SourceURL returns the full upstream URL to fetch: the source path +// percent-encoded again, as net/url writes a path, so each source path is one +// upstream path, and the source query as it is, with the characters +// escapeQuery encodes percent-encoded. Uses http:// scheme when AllowHTTP is +// true, otherwise https://. +func (r *Request) SourceURL() string { + scheme := "https" + if r.AllowHTTP { + scheme = "http" + } + + sourceURL := url.URL{ + Scheme: scheme, + Host: r.Source.Host, + Path: r.Source.Path, + RawQuery: escapeQuery(r.Source.Query), + } + + return sourceURL.String() +} diff --git a/internal/imgcache/sourceurl_internal_test.go b/internal/imagerequest/sourceurl_internal_test.go similarity index 50% rename from internal/imgcache/sourceurl_internal_test.go rename to internal/imagerequest/sourceurl_internal_test.go index 36bd3fe..6e6b671 100644 --- a/internal/imgcache/sourceurl_internal_test.go +++ b/internal/imagerequest/sourceurl_internal_test.go @@ -1,14 +1,16 @@ -package imgcache +package imagerequest import "testing" -func TestImageRequest_SourceURL_DefaultHTTPS(t *testing.T) { +func TestRequest_SourceURL_DefaultHTTPS(t *testing.T) { t.Parallel() - req := &ImageRequest{ - SourceHost: testHostCDN, - SourcePath: testPathCat, - SourceQuery: "v=2", + req := &Request{ + Source: Source{ + Host: testHostCDN, + Path: testPathCat, + Query: "v=2", + }, } got := req.SourceURL() @@ -19,13 +21,15 @@ func TestImageRequest_SourceURL_DefaultHTTPS(t *testing.T) { } } -func TestImageRequest_SourceURL_AllowHTTP(t *testing.T) { +func TestRequest_SourceURL_AllowHTTP(t *testing.T) { t.Parallel() - req := &ImageRequest{ - SourceHost: "localhost:8080", - SourcePath: testPathCat, - AllowHTTP: true, + req := &Request{ + Source: Source{ + Host: "localhost:8080", + Path: testPathCat, + }, + AllowHTTP: true, } got := req.SourceURL() @@ -36,13 +40,15 @@ func TestImageRequest_SourceURL_AllowHTTP(t *testing.T) { } } -func TestImageRequest_SourceURL_AllowHTTPFalse(t *testing.T) { +func TestRequest_SourceURL_AllowHTTPFalse(t *testing.T) { t.Parallel() - req := &ImageRequest{ - SourceHost: testHostCDN, - SourcePath: "/img.jpg", - AllowHTTP: false, + req := &Request{ + Source: Source{ + Host: testHostCDN, + Path: "/img.jpg", + }, + AllowHTTP: false, } got := req.SourceURL() diff --git a/internal/imgcache/urlparser.go b/internal/imagerequest/urlparser.go similarity index 88% rename from internal/imgcache/urlparser.go rename to internal/imagerequest/urlparser.go index 46bba16..ce96231 100644 --- a/internal/imgcache/urlparser.go +++ b/internal/imagerequest/urlparser.go @@ -1,4 +1,4 @@ -package imgcache +package imagerequest import ( "errors" @@ -8,8 +8,6 @@ import ( "regexp" "strconv" "strings" - - "sneak.berlin/go/pixa/internal/imageprocessor" ) // URL parsing errors. @@ -25,8 +23,8 @@ var ( ) // MaxDimension is the maximum allowed width or height, the same as the image -// processor's maximum for an output image. -const MaxDimension = imageprocessor.MaxOutputDimension +// processor's maximum for an output image (imageprocessor.MaxOutputDimension). +const MaxDimension = 8192 // ValidateDimension checks one requested width or height; name ("width" or // "height") appears in the error. 0 is valid: that side is then worked out from @@ -68,20 +66,6 @@ var queryEscapeRegex = regexp.MustCompile(`%[0-9A-Fa-f]{2}|[^!-~]|["#'<>%]`) var hostRegex = regexp.MustCompile( `^(?:[A-Za-z0-9_.-]+|\[([0-9A-Fa-f:.]+)\])(?::[0-9]*)?$`) -// ParsedURL contains the parsed components of an image proxy URL. -type ParsedURL struct { - // Host is the source origin host (e.g., "cdn.example.com") - Host string - // Path is the path on the origin (e.g., "/photos/cat.jpg") - Path string - // Query is the optional query string for the origin - Query string - // Size is the requested output dimensions - Size Size - // Format is the requested output format - Format ImageFormat -} - // ParseImagePath parses an image URL's path after /v1/image/, decoded once: // //., or // for JPEG XL. // Examples: @@ -89,7 +73,7 @@ type ParsedURL struct { // - cdn.example.com/photos/cat.jpg/0x0.jpeg // - cdn.example.com/photos/cat.jpg/orig.png // - cdn.example.com/photos/cat.jpg/800x600 -func ParseImagePath(path string) (*ParsedURL, error) { +func ParseImagePath(path string) (*Request, error) { // Strip leading slash if present (chi may include it) path = strings.TrimPrefix(path, "/") if path == "" { @@ -103,7 +87,7 @@ func ParseImagePath(path string) (*ParsedURL, error) { // it: /v1/image///., or /v1/image/// // for JPEG XL. A source query follows the source path after a "?", which the // URL writes as %3F. -func ParseImageURL(urlPath string) (*ParsedURL, error) { +func ParseImageURL(urlPath string) (*Request, error) { // Remove the /v1/image/ prefix const prefix = "/v1/image/" if !strings.HasPrefix(urlPath, prefix) { @@ -120,7 +104,7 @@ func ParseImageURL(urlPath string) (*ParsedURL, error) { // parseImageComponents parses //., or // // for JPEG XL. -func parseImageComponents(remainder string) (*ParsedURL, error) { +func parseImageComponents(remainder string) (*Request, error) { // Find the last path segment, which holds "size" or "size.format" lastSlash := strings.LastIndex(remainder, "/") if lastSlash == -1 { @@ -209,12 +193,9 @@ func parseImageComponents(remainder string) (*ParsedURL, error) { path = "/" + path } - return &ParsedURL{ - Host: host, - Path: path, - Query: query, - Size: size, - Format: format, + return &Request{ + Source: Source{Host: host, Path: path, Query: query}, + Transform: Transform{Size: size, Format: format}, }, nil } @@ -297,7 +278,7 @@ func escapeQuery(query string) string { } // parseSizeFormat parses strings like "800x600.webp", "orig.png" or "800x600" -func parseSizeFormat(s string) (Size, ImageFormat, error) { +func parseSizeFormat(s string) (Size, Format, error) { matches := sizeFormatRegex.FindStringSubmatch(s) if matches == nil { return Size{}, "", ErrInvalidSize @@ -351,8 +332,8 @@ func parseSizeFormat(s string) (Size, ImageFormat, error) { return size, format, nil } -// parseFormat converts a format string to ImageFormat. -func parseFormat(s string) (ImageFormat, error) { +// parseFormat converts a format string to Format. +func parseFormat(s string) (Format, error) { switch strings.ToLower(s) { case "orig", "original": return FormatOriginal, nil @@ -374,14 +355,3 @@ func parseFormat(s string) (ImageFormat, error) { return "", fmt.Errorf("%w: %s", ErrInvalidFormat, s) } } - -// ToImageRequest converts a ParsedURL to an ImageRequest. -func (p *ParsedURL) ToImageRequest() *ImageRequest { - return &ImageRequest{ - SourceHost: p.Host, - SourcePath: p.Path, - SourceQuery: p.Query, - Size: p.Size, - Format: p.Format, - } -} diff --git a/internal/imgcache/urlparser_internal_test.go b/internal/imagerequest/urlparser_internal_test.go similarity index 72% rename from internal/imgcache/urlparser_internal_test.go rename to internal/imagerequest/urlparser_internal_test.go index 603d516..83742ca 100644 --- a/internal/imgcache/urlparser_internal_test.go +++ b/internal/imagerequest/urlparser_internal_test.go @@ -1,33 +1,41 @@ -package imgcache +package imagerequest import ( "errors" "testing" ) -// assertParsedURL compares all fields of a parsed URL against the -// expected value. -func assertParsedURL(t *testing.T, got, want *ParsedURL) { +// Shared test fixture strings. +const ( + testHostCDN = "cdn.example.com" + testHostExample = "example.com" + testPathCat = "/photos/cat.jpg" +) + +// assertRequest compares the source host, path and query and the size and +// format of got against want. +func assertRequest(t *testing.T, got, want *Request) { t.Helper() - if got.Host != want.Host { - t.Errorf("Host = %q, want %q", got.Host, want.Host) + if got.Source.Host != want.Source.Host { + t.Errorf("Host = %q, want %q", got.Source.Host, want.Source.Host) } - if got.Path != want.Path { - t.Errorf("Path = %q, want %q", got.Path, want.Path) + if got.Source.Path != want.Source.Path { + t.Errorf("Path = %q, want %q", got.Source.Path, want.Source.Path) } - if got.Query != want.Query { - t.Errorf("Query = %q, want %q", got.Query, want.Query) + if got.Source.Query != want.Source.Query { + t.Errorf("Query = %q, want %q", got.Source.Query, want.Source.Query) } - if got.Size != want.Size { - t.Errorf("Size = %v, want %v", got.Size, want.Size) + if got.Transform.Size != want.Transform.Size { + t.Errorf("Size = %v, want %v", got.Transform.Size, want.Transform.Size) } - if got.Format != want.Format { - t.Errorf("Format = %q, want %q", got.Format, want.Format) + if got.Transform.Format != want.Transform.Format { + t.Errorf("Format = %q, want %q", + got.Transform.Format, want.Transform.Format) } } @@ -37,62 +45,64 @@ func TestParseImageURL(t *testing.T) { tests := []struct { name string input string - want *ParsedURL + want *Request }{ { name: "basic path with size", input: "/v1/image/cdn.example.com/photos/cat.jpg/800x600.webp", - want: &ParsedURL{ - Host: testHostCDN, Path: testPathCat, - Size: Size{Width: 800, Height: 600}, Format: FormatWebP, + want: &Request{ + Source: Source{Host: testHostCDN, Path: testPathCat}, + Transform: Transform{Size: Size{Width: 800, Height: 600}, Format: FormatWebP}, }, }, { name: "original size with 0x0", input: "/v1/image/cdn.example.com/photos/cat.jpg/0x0.jpeg", - want: &ParsedURL{ - Host: testHostCDN, Path: testPathCat, - Size: Size{Width: 0, Height: 0}, Format: FormatJPEG, + want: &Request{ + Source: Source{Host: testHostCDN, Path: testPathCat}, + Transform: Transform{Size: Size{Width: 0, Height: 0}, Format: FormatJPEG}, }, }, { name: "original size with orig keyword", input: "/v1/image/cdn.example.com/photos/cat.jpg/orig.png", - want: &ParsedURL{ - Host: testHostCDN, Path: testPathCat, - Size: Size{Width: 0, Height: 0}, Format: FormatPNG, + want: &Request{ + Source: Source{Host: testHostCDN, Path: testPathCat}, + Transform: Transform{Size: Size{Width: 0, Height: 0}, Format: FormatPNG}, }, }, { name: "path with query string", input: "/v1/image/cdn.example.com/photos/cat.jpg?arg1=val1&arg2=val2/800x600.webp", - want: &ParsedURL{ - Host: testHostCDN, Path: testPathCat, Query: "arg1=val1&arg2=val2", - Size: Size{Width: 800, Height: 600}, Format: FormatWebP, + want: &Request{ + Source: Source{ + Host: testHostCDN, Path: testPathCat, Query: "arg1=val1&arg2=val2", + }, + Transform: Transform{Size: Size{Width: 800, Height: 600}, Format: FormatWebP}, }, }, { name: "deep nested path", input: "/v1/image/cdn.example.com/a/b/c/d/image.jpg/1920x1080.avif", - want: &ParsedURL{ - Host: testHostCDN, Path: "/a/b/c/d/image.jpg", - Size: Size{Width: 1920, Height: 1080}, Format: FormatAVIF, + want: &Request{ + Source: Source{Host: testHostCDN, Path: "/a/b/c/d/image.jpg"}, + Transform: Transform{Size: Size{Width: 1920, Height: 1080}, Format: FormatAVIF}, }, }, { name: "jpg alias for jpeg", input: "/v1/image/example.com/img.png/100x100.jpg", - want: &ParsedURL{ - Host: testHostExample, Path: "/img.png", - Size: Size{Width: 100, Height: 100}, Format: FormatJPEG, + want: &Request{ + Source: Source{Host: testHostExample, Path: "/img.png"}, + Transform: Transform{Size: Size{Width: 100, Height: 100}, Format: FormatJPEG}, }, }, { name: "gif format", input: "/v1/image/example.com/animated.gif/200x200.gif", - want: &ParsedURL{ - Host: testHostExample, Path: "/animated.gif", - Size: Size{Width: 200, Height: 200}, Format: FormatGIF, + want: &Request{ + Source: Source{Host: testHostExample, Path: "/animated.gif"}, + Transform: Transform{Size: Size{Width: 200, Height: 200}, Format: FormatGIF}, }, }, } @@ -106,7 +116,7 @@ func TestParseImageURL(t *testing.T) { t.Fatalf("ParseImageURL() unexpected error = %v", err) } - assertParsedURL(t, got, tt.want) + assertRequest(t, got, tt.want) }) } } @@ -121,8 +131,8 @@ func TestParseImageURL_AutoFormat(t *testing.T) { t.Fatalf("ParseImageURL() error = %v", err) } - if got.Format != FormatAuto { - t.Errorf("Format = %q, want %q", got.Format, FormatAuto) + if got.Transform.Format != FormatAuto { + t.Errorf("Format = %q, want %q", got.Transform.Format, FormatAuto) } } @@ -189,27 +199,35 @@ func TestParseImagePath(t *testing.T) { tests := []struct { name string input string - want *ParsedURL + want *Request wantErr bool }{ { name: "chi wildcard capture", input: "cdn.example.com/photos/cat.jpg/800x600.webp", - want: &ParsedURL{ - Host: testHostCDN, - Path: testPathCat, - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, + want: &Request{ + Source: Source{ + Host: testHostCDN, + Path: testPathCat, + }, + Transform: Transform{ + Size: Size{Width: 800, Height: 600}, + Format: FormatWebP, + }, }, }, { name: "with leading slash from chi", input: "/cdn.example.com/photos/cat.jpg/800x600.webp", - want: &ParsedURL{ - Host: testHostCDN, - Path: testPathCat, - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, + want: &Request{ + Source: Source{ + Host: testHostCDN, + Path: testPathCat, + }, + Transform: Transform{ + Size: Size{Width: 800, Height: 600}, + Format: FormatWebP, + }, }, }, } @@ -229,7 +247,7 @@ func TestParseImagePath(t *testing.T) { return } - assertParsedURL(t, got, tt.want) + assertRequest(t, got, tt.want) }) } } @@ -246,14 +264,14 @@ func TestParseImagePath_LowerCasesHost(t *testing.T) { t.Fatalf("ParseImagePath() error = %v", err) } - t.Logf("parsed host %q, path %q", got.Host, got.Path) + t.Logf("parsed host %q, path %q", got.Source.Host, got.Source.Path) - if got.Host != testHostCDN { - t.Errorf("Host = %q, want %q", got.Host, testHostCDN) + if got.Source.Host != testHostCDN { + t.Errorf("Host = %q, want %q", got.Source.Host, testHostCDN) } - if got.Path != "/Photos/Cat.jpg" { - t.Errorf("Path = %q, want %q", got.Path, "/Photos/Cat.jpg") + if got.Source.Path != "/Photos/Cat.jpg" { + t.Errorf("Path = %q, want %q", got.Source.Path, "/Photos/Cat.jpg") } } @@ -277,40 +295,6 @@ func TestParseImagePath_RefusesUserinfo(t *testing.T) { } } -func TestParsedURL_ToImageRequest(t *testing.T) { - t.Parallel() - - parsed := &ParsedURL{ - Host: testHostCDN, - Path: testPathCat, - Query: "version=2", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, - } - - req := parsed.ToImageRequest() - - if req.SourceHost != parsed.Host { - t.Errorf("SourceHost = %q, want %q", req.SourceHost, parsed.Host) - } - - if req.SourcePath != parsed.Path { - t.Errorf("SourcePath = %q, want %q", req.SourcePath, parsed.Path) - } - - if req.SourceQuery != parsed.Query { - t.Errorf("SourceQuery = %q, want %q", req.SourceQuery, parsed.Query) - } - - if req.Size != parsed.Size { - t.Errorf("Size = %v, want %v", req.Size, parsed.Size) - } - - if req.Format != parsed.Format { - t.Errorf("Format = %q, want %q", req.Format, parsed.Format) - } -} - func TestParseImageURL_PathTraversal(t *testing.T) { t.Parallel() @@ -417,22 +401,24 @@ func TestParseImageURL_SourceURLHoldingPercent25(t *testing.T) { tests := []struct { name string input string - want *ParsedURL + want *Request }{ { name: "path", input: "/v1/image/cdn.example.com/sale/50%25off.jpg/800x600.webp", - want: &ParsedURL{ - Host: testHostCDN, Path: "/sale/50%25off.jpg", - Size: Size{Width: 800, Height: 600}, Format: FormatWebP, + want: &Request{ + Source: Source{Host: testHostCDN, Path: "/sale/50%25off.jpg"}, + Transform: Transform{Size: Size{Width: 800, Height: 600}, Format: FormatWebP}, }, }, { name: "query", input: "/v1/image/cdn.example.com/photos/cat.jpg?discount=50%25/800x600.webp", - want: &ParsedURL{ - Host: testHostCDN, Path: testPathCat, Query: "discount=50%25", - Size: Size{Width: 800, Height: 600}, Format: FormatWebP, + want: &Request{ + Source: Source{ + Host: testHostCDN, Path: testPathCat, Query: "discount=50%25", + }, + Transform: Transform{Size: Size{Width: 800, Height: 600}, Format: FormatWebP}, }, }, } @@ -446,7 +432,7 @@ func TestParseImageURL_SourceURLHoldingPercent25(t *testing.T) { t.Fatalf("ParseImageURL() unexpected error = %v", err) } - assertParsedURL(t, got, tt.want) + assertRequest(t, got, tt.want) }) } } diff --git a/internal/imgcache/validate_internal_test.go b/internal/imagerequest/validate_internal_test.go similarity index 54% rename from internal/imgcache/validate_internal_test.go rename to internal/imagerequest/validate_internal_test.go index 99ecf36..8cb3d23 100644 --- a/internal/imgcache/validate_internal_test.go +++ b/internal/imagerequest/validate_internal_test.go @@ -1,4 +1,4 @@ -package imgcache +package imagerequest import ( "errors" @@ -10,35 +10,43 @@ func TestValidateImageRequest(t *testing.T) { tests := []struct { name string - req ImageRequest + req Request wantErr error }{ { name: "within bounds", - req: ImageRequest{Size: Size{Width: 800, Height: 600}, FitMode: FitCover}, + req: Request{Transform: Transform{ + Size: Size{Width: 800, Height: 600}, FitMode: FitCover, + }}, }, { name: "original size and empty fit", - req: ImageRequest{Size: Size{Width: 0, Height: 0}}, + req: Request{Transform: Transform{Size: Size{Width: 0, Height: 0}}}, }, { - name: "width over limit", - req: ImageRequest{Size: Size{Width: MaxDimension + 1, Height: 600}}, + name: "width over limit", + req: Request{Transform: Transform{ + Size: Size{Width: MaxDimension + 1, Height: 600}, + }}, wantErr: ErrDimensionTooLarge, }, { - name: "height over limit", - req: ImageRequest{Size: Size{Width: 800, Height: MaxDimension + 1}}, + name: "height over limit", + req: Request{Transform: Transform{ + Size: Size{Width: 800, Height: MaxDimension + 1}, + }}, wantErr: ErrDimensionTooLarge, }, { name: "negative width", - req: ImageRequest{Size: Size{Width: -1, Height: 600}}, + req: Request{Transform: Transform{Size: Size{Width: -1, Height: 600}}}, wantErr: ErrInvalidSize, }, { - name: "invalid fit mode", - req: ImageRequest{Size: Size{Width: 800, Height: 600}, FitMode: "bogus"}, + name: "invalid fit mode", + req: Request{Transform: Transform{ + Size: Size{Width: 800, Height: 600}, FitMode: "bogus", + }}, wantErr: ErrInvalidFitMode, }, } diff --git a/internal/imagerequest/variantkey.go b/internal/imagerequest/variantkey.go new file mode 100644 index 0000000..bfa0f64 --- /dev/null +++ b/internal/imagerequest/variantkey.go @@ -0,0 +1,39 @@ +package imagerequest + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "strings" +) + +// VariantKey is a SHA256 hash identifying a specific image variant (hex-encoded). +type VariantKey string + +// CacheKey generates a unique key for a request variant. +// Format: sha256(host:path:query:width:height:format:quality:fit_mode) +// The path and query go through escapePercentAndColon, so that a ":" in +// either cannot be taken for the one between them. +func CacheKey(req *Request) VariantKey { + data := fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%s", + req.Source.Host, + escapePercentAndColon(req.Source.Path), + escapePercentAndColon(req.Source.Query), + req.Transform.Size.Width, + req.Transform.Size.Height, + req.Transform.Format, + req.Transform.Quality, + req.Transform.FitMode, + ) + h := sha256.Sum256([]byte(data)) + + return VariantKey(hex.EncodeToString(h[:])) +} + +// escapePercentAndColon returns s with each "%" written as "%25" and each +// ":" as "%3A", so that the result holds no ":". +func escapePercentAndColon(s string) string { + s = strings.ReplaceAll(s, "%", "%25") + + return strings.ReplaceAll(s, ":", "%3A") +} diff --git a/internal/imagerequest/variantkey_internal_test.go b/internal/imagerequest/variantkey_internal_test.go new file mode 100644 index 0000000..9d6c775 --- /dev/null +++ b/internal/imagerequest/variantkey_internal_test.go @@ -0,0 +1,142 @@ +package imagerequest + +import "testing" + +func TestCacheKey(t *testing.T) { + t.Parallel() + + req1 := &Request{ + Source: Source{Host: testHostCDN, Path: testPathCat, Query: ""}, + Transform: Transform{ + Size: Size{Width: 800, Height: 600}, Format: FormatWebP, + Quality: 85, FitMode: FitCover, + }, + } + + req2 := &Request{ + Source: Source{Host: testHostCDN, Path: testPathCat, Query: ""}, + Transform: Transform{ + Size: Size{Width: 800, Height: 600}, Format: FormatWebP, + Quality: 85, FitMode: FitCover, + }, + } + + // Same request should produce same key + key1 := CacheKey(req1) + key2 := CacheKey(req2) + + if key1 != key2 { + t.Errorf("CacheKey() not deterministic: %s vs %s", key1, key2) + } + + // Key should be 64 hex chars + if len(string(key1)) != 64 { + t.Errorf("CacheKey() length = %d, want 64", len(string(key1))) + } + + // Different size should produce different key + req3 := &Request{ + Source: Source{Host: testHostCDN, Path: testPathCat, Query: ""}, + Transform: Transform{ + Size: Size{Width: 400, Height: 300}, Format: FormatWebP, // Different size + Quality: 85, FitMode: FitCover, + }, + } + + key3 := CacheKey(req3) + if key1 == key3 { + t.Error("CacheKey() produced same key for different sizes") + } + + // Different format should produce different key + req4 := &Request{ + Source: Source{Host: testHostCDN, Path: testPathCat, Query: ""}, + Transform: Transform{ + Size: Size{Width: 800, Height: 600}, Format: FormatPNG, // Different format + Quality: 85, FitMode: FitCover, + }, + } + + key4 := CacheKey(req4) + if key1 == key4 { + t.Error("CacheKey() produced same key for different formats") + } + + // Different quality should produce different key + req5 := &Request{ + Source: Source{Host: testHostCDN, Path: testPathCat, Query: ""}, + Transform: Transform{ + Size: Size{Width: 800, Height: 600}, Format: FormatWebP, + Quality: 50, FitMode: FitCover, // Different quality + }, + } + + key5 := CacheKey(req5) + if key1 == key5 { + t.Error("CacheKey() produced same key for different quality") + } +} + +// TestCacheKey_ColonOrPercentInPathOrQuery checks that two different source +// URLs whose path or query holds ":" or "%" get different keys. +func TestCacheKey_ColonOrPercentInPathOrQuery(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + path1, query1 string + path2, query2 string + }{ + { + name: "colon moved from path to query", + path1: "/a:b", query1: "", + path2: "/a", query2: "b:", + }, + { + name: "colon in path and its percent-encoding", + path1: "/a:b", query1: "", + path2: "/a%3Ab", query2: "", + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + key1 := CacheKey(&Request{ + Source: Source{ + Host: testHostCDN, + Path: tt.path1, + Query: tt.query1, + }, + Transform: Transform{ + Size: Size{Width: 800, Height: 600}, + Format: FormatWebP, + Quality: 85, + FitMode: FitCover, + }, + }) + key2 := CacheKey(&Request{ + Source: Source{ + Host: testHostCDN, + Path: tt.path2, + Query: tt.query2, + }, + Transform: Transform{ + Size: Size{Width: 800, Height: 600}, + Format: FormatWebP, + Quality: 85, + FitMode: FitCover, + }, + }) + + t.Logf("CacheKey() of path %q query %q = %s", tt.path1, tt.query1, key1) + t.Logf("CacheKey() of path %q query %q = %s", tt.path2, tt.query2, key2) + + if key1 == key2 { + t.Errorf("CacheKey() gave path %q query %q and path %q query %q "+ + "the same key", tt.path1, tt.query1, tt.path2, tt.query2) + } + }) + } +} diff --git a/internal/imgcache/cache.go b/internal/imgcache/cache.go index 4a78953..35bac85 100644 --- a/internal/imgcache/cache.go +++ b/internal/imgcache/cache.go @@ -17,6 +17,7 @@ import ( lru "github.com/hashicorp/golang-lru/v2" "sneak.berlin/go/pixa/internal/httpfetcher" + "sneak.berlin/go/pixa/internal/imagerequest" ) // Cache errors. @@ -111,7 +112,7 @@ type Cache struct { // metaCache holds the content types of the variants most recently // stored or served, so a hit does not read the variant's .meta file. // It never stands in for the variant file, which is always opened. - metaCache *lru.Cache[VariantKey, string] + metaCache *lru.Cache[imagerequest.VariantKey, string] // contentLocks serializes StoreSource and evictSourceBlob per // content hash, closing the race window between an eviction's row @@ -153,7 +154,7 @@ func newCache( log = slog.Default() } - metaCache, err := lru.New[VariantKey, string](metaCacheSize) + metaCache, err := lru.New[imagerequest.VariantKey, string](metaCacheSize) if err != nil { return nil, fmt.Errorf("failed to create variant content type cache: %w", err) } @@ -223,7 +224,7 @@ func newCache( // LookupResult contains the result of a cache lookup. type LookupResult struct { Hit bool - CacheKey VariantKey + CacheKey imagerequest.VariantKey ContentType string SizeBytes int64 CacheStatus CacheStatus @@ -231,8 +232,10 @@ type LookupResult struct { // Lookup checks if a processed variant exists on disk. Hits touch the // variant's LRU timestamp; a disabled cache always misses. -func (c *Cache) Lookup(ctx context.Context, req *ImageRequest) (*LookupResult, error) { - cacheKey := CacheKey(req) +func (c *Cache) Lookup( + ctx context.Context, req *imagerequest.Request, +) (*LookupResult, error) { + cacheKey := imagerequest.CacheKey(req) // Check variant storage directly - no DB needed for cache hits if !c.disabled && c.variants.Exists(cacheKey) { @@ -257,7 +260,9 @@ func (c *Cache) Lookup(ctx context.Context, req *ImageRequest) (*LookupResult, e // variant's .meta file and is then kept in metaCache. A variant with // no .meta file is served as application/octet-stream, which is not // kept. -func (c *Cache) GetVariant(cacheKey VariantKey) (io.ReadCloser, int64, string, error) { +func (c *Cache) GetVariant( + cacheKey imagerequest.VariantKey, +) (io.ReadCloser, int64, string, error) { if c.disabled { return nil, 0, "", ErrNotFound } @@ -282,7 +287,7 @@ func (c *Cache) GetVariant(cacheKey VariantKey) (io.ReadCloser, int64, string, e // disabled cache it is a no-op returning an empty hash. func (c *Cache) StoreSource( ctx context.Context, - req *ImageRequest, + req *imagerequest.Request, content io.Reader, result *httpfetcher.FetchResult, ) (ContentHash, error) { @@ -322,7 +327,7 @@ func (c *Cache) StoreSource( } // Store in database - pathHash := HashPath(req.SourcePath + "?" + req.SourceQuery) + pathHash := HashPath(req.Source.Path + "?" + req.Source.Query) headersJSON, err := json.Marshal(result.Headers) if err != nil { @@ -349,7 +354,7 @@ func (c *Cache) StoreSource( content_type = excluded.content_type, response_headers = excluded.response_headers, fetched_at = CURRENT_TIMESTAMP - `, req.SourceHost, req.SourcePath, req.SourceQuery, pathHash, + `, req.Source.Host, req.Source.Path, req.Source.Query, pathHash, contentHash, httpStatusOK, result.ContentType, string(headersJSON)) if err != nil { return "", fmt.Errorf("failed to insert source metadata: %w", err) @@ -367,7 +372,8 @@ func (c *Cache) StoreSource( // accounting insert is best-effort (the startup reconciliation pass // adopts any variant file that misses its accounting row). func (c *Cache) StoreVariant( - ctx context.Context, cacheKey VariantKey, content io.Reader, contentType string, + ctx context.Context, cacheKey imagerequest.VariantKey, + content io.Reader, contentType string, ) error { if c.disabled { return nil @@ -403,7 +409,7 @@ func (c *Cache) StoreVariant( // if not. Hits touch the blob's LRU timestamp; a disabled cache always // reports no cached source. func (c *Cache) LookupSource( - ctx context.Context, req *ImageRequest, + ctx context.Context, req *imagerequest.Request, ) (ContentHash, string, error) { if c.disabled { return "", "", nil @@ -414,7 +420,7 @@ func (c *Cache) LookupSource( err := c.db.QueryRowContext(ctx, ` SELECT content_hash, content_type FROM source_metadata WHERE source_host = ? AND source_path = ? AND source_query = ? - `, req.SourceHost, req.SourcePath, req.SourceQuery).Scan(&hashStr, &contentType) + `, req.Source.Host, req.Source.Path, req.Source.Query).Scan(&hashStr, &contentType) if errors.Is(err, sql.ErrNoRows) { return "", "", nil @@ -438,7 +444,7 @@ func (c *Cache) LookupSource( // StoreNegative stores a negative cache entry for a failed fetch. func (c *Cache) StoreNegative( - ctx context.Context, req *ImageRequest, statusCode int, errMsg string, + ctx context.Context, req *imagerequest.Request, statusCode int, errMsg string, ) error { expiresAt := time.Now().UTC().Add(c.config.NegativeTTL) @@ -452,7 +458,7 @@ func (c *Cache) StoreNegative( error_message = excluded.error_message, fetched_at = CURRENT_TIMESTAMP, expires_at = excluded.expires_at - `, req.SourceHost, req.SourcePath, req.SourceQuery, statusCode, errMsg, expiresAt) + `, req.Source.Host, req.Source.Path, req.Source.Query, statusCode, errMsg, expiresAt) if err != nil { return fmt.Errorf("failed to insert negative cache: %w", err) } @@ -462,14 +468,14 @@ func (c *Cache) StoreNegative( // GetSourceMetadataID returns the source metadata ID for a request. func (c *Cache) GetSourceMetadataID( - ctx context.Context, req *ImageRequest, + ctx context.Context, req *imagerequest.Request, ) (int64, error) { var id int64 err := c.db.QueryRowContext(ctx, ` SELECT id FROM source_metadata WHERE source_host = ? AND source_path = ? AND source_query = ? - `, req.SourceHost, req.SourcePath, req.SourceQuery).Scan(&id) + `, req.Source.Host, req.Source.Path, req.Source.Query).Scan(&id) if err != nil { return 0, fmt.Errorf("failed to get source metadata ID: %w", err) } @@ -665,7 +671,7 @@ func (c *Cache) readHitAndMissCounts(ctx context.Context, stats *CacheStats) err // application/octet-stream and keeps nothing, so metaCache only ever // holds a type read from a .meta file or passed to StoreVariant. func (c *Cache) loadVariantWithMeta( - cacheKey VariantKey, + cacheKey imagerequest.VariantKey, ) (io.ReadCloser, int64, string, error) { reader, size, contentType, err := c.variants.LoadWithMeta(cacheKey) if err != nil { @@ -685,15 +691,15 @@ func (c *Cache) loadVariantWithMeta( // A failure is logged and is otherwise non-fatal; the metadata is in the // database. func (c *Cache) writeMetadataSidecar( - req *ImageRequest, + req *imagerequest.Request, pathHash PathHash, contentHash ContentHash, result *httpfetcher.FetchResult, ) { meta := &SourceMetadata{ - Host: req.SourceHost, - Path: req.SourcePath, - Query: req.SourceQuery, + Host: req.Source.Host, + Path: req.Source.Path, + Query: req.Source.Query, ContentHash: string(contentHash), StatusCode: result.StatusCode, ContentType: result.ContentType, @@ -704,16 +710,16 @@ func (c *Cache) writeMetadataSidecar( RemoteAddr: result.RemoteAddr, } - err := c.srcMetadata.Store(req.SourceHost, pathHash, meta) + err := c.srcMetadata.Store(req.Source.Host, pathHash, meta) if err != nil { c.log.Warn("failed to write metadata sidecar", - "host", req.SourceHost, "path_hash", pathHash, "error", err) + "host", req.Source.Host, "path_hash", pathHash, "error", err) } } // touchVariant updates the LRU timestamp of a variant, best-effort: // a failed touch only makes the entry look colder to eviction. -func (c *Cache) touchVariant(ctx context.Context, cacheKey VariantKey) { +func (c *Cache) touchVariant(ctx context.Context, cacheKey imagerequest.VariantKey) { _, err := c.db.ExecContext(ctx, ` UPDATE variant_content SET last_accessed_at = CURRENT_TIMESTAMP WHERE cache_key = ? @@ -739,14 +745,14 @@ func (c *Cache) touchSourceContent(ctx context.Context, contentHash ContentHash) // checkNegativeCache checks if a request is in the negative cache. func (c *Cache) checkNegativeCache( - ctx context.Context, req *ImageRequest, + ctx context.Context, req *imagerequest.Request, ) (bool, error) { var expiresAt time.Time err := c.db.QueryRowContext(ctx, ` SELECT expires_at FROM negative_cache WHERE source_host = ? AND source_path = ? AND source_query = ? - `, req.SourceHost, req.SourcePath, req.SourceQuery).Scan(&expiresAt) + `, req.Source.Host, req.Source.Path, req.Source.Query).Scan(&expiresAt) if errors.Is(err, sql.ErrNoRows) { return false, nil @@ -762,10 +768,10 @@ func (c *Cache) checkNegativeCache( _, err = c.db.ExecContext(ctx, ` DELETE FROM negative_cache WHERE source_host = ? AND source_path = ? AND source_query = ? - `, req.SourceHost, req.SourcePath, req.SourceQuery) + `, req.Source.Host, req.Source.Path, req.Source.Query) if err != nil { c.log.Warn("failed to delete expired negative cache entry", - "host", req.SourceHost, "path", req.SourcePath, "error", err) + "host", req.Source.Host, "path", req.Source.Path, "error", err) } return false, nil diff --git a/internal/imgcache/cache_internal_test.go b/internal/imgcache/cache_internal_test.go index 6659b93..95966e2 100644 --- a/internal/imgcache/cache_internal_test.go +++ b/internal/imgcache/cache_internal_test.go @@ -10,6 +10,7 @@ import ( _ "modernc.org/sqlite" "sneak.berlin/go/pixa/internal/httpfetcher" + "sneak.berlin/go/pixa/internal/imagerequest" ) func setupTestDB(t *testing.T) *sql.DB { @@ -117,13 +118,17 @@ func TestCache_LookupMiss(t *testing.T) { cache := setupTestCache(t) ctx := context.Background() - req := &ImageRequest{ - SourceHost: testHostCDN, - SourcePath: testPathCat, - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHostCDN, + Path: testPathCat, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: imagerequest.FormatWebP, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } result, err := cache.Lookup(ctx, req) @@ -146,13 +151,17 @@ func TestCache_StoreAndLookup(t *testing.T) { cache := setupTestCache(t) ctx := context.Background() - req := &ImageRequest{ - SourceHost: testHostCDN, - SourcePath: testPathCat, - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHostCDN, + Path: testPathCat, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: imagerequest.FormatWebP, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } // Store source content @@ -173,7 +182,7 @@ func TestCache_StoreAndLookup(t *testing.T) { } // Store variant - cacheKey := CacheKey(req) + cacheKey := imagerequest.CacheKey(req) outputContent := []byte("fake webp data") err = cache.StoreVariant( @@ -207,11 +216,15 @@ func TestCache_NegativeCache(t *testing.T) { cache := setupTestCache(t) ctx := context.Background() - req := &ImageRequest{ - SourceHost: testHostCDN, - SourcePath: "/photos/notfound.jpg", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHostCDN, + Path: "/photos/notfound.jpg", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: imagerequest.FormatWebP, + }, } // Store negative cache entry @@ -248,11 +261,15 @@ func TestCache_NegativeCacheExpiry(t *testing.T) { ctx := context.Background() - req := &ImageRequest{ - SourceHost: testHostCDN, - SourcePath: "/photos/expired.jpg", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHostCDN, + Path: "/photos/expired.jpg", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: imagerequest.FormatWebP, + }, } // Store negative cache entry @@ -281,17 +298,21 @@ func TestCache_VariantLookup(t *testing.T) { cache := setupTestCache(t) ctx := context.Background() - req := &ImageRequest{ - SourceHost: testHostCDN, - SourcePath: "/photos/variant.jpg", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHostCDN, + Path: "/photos/variant.jpg", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: imagerequest.FormatWebP, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } // Store variant - cacheKey := CacheKey(req) + cacheKey := imagerequest.CacheKey(req) outputContent := []byte("output data") err := cache.StoreVariant( @@ -331,17 +352,21 @@ func TestCache_GetVariant_ReturnsContentType(t *testing.T) { cache := setupTestCache(t) ctx := context.Background() - req := &ImageRequest{ - SourceHost: testHostCDN, - SourcePath: "/photos/variantct.jpg", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHostCDN, + Path: "/photos/variantct.jpg", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: imagerequest.FormatWebP, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } // Store variant - cacheKey := CacheKey(req) + cacheKey := imagerequest.CacheKey(req) outputContent := []byte("output webp data") err := cache.StoreVariant( @@ -383,17 +408,21 @@ func TestCache_GetVariant(t *testing.T) { cache := setupTestCache(t) ctx := context.Background() - req := &ImageRequest{ - SourceHost: testHostCDN, - SourcePath: "/photos/output.jpg", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHostCDN, + Path: "/photos/output.jpg", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: imagerequest.FormatWebP, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } // Store variant - cacheKey := CacheKey(req) + cacheKey := imagerequest.CacheKey(req) outputContent := []byte("the actual output content") err := cache.StoreVariant( diff --git a/internal/imgcache/cache_usage_internal_test.go b/internal/imgcache/cache_usage_internal_test.go index 64666aa..a4d9315 100644 --- a/internal/imgcache/cache_usage_internal_test.go +++ b/internal/imgcache/cache_usage_internal_test.go @@ -5,6 +5,8 @@ import ( "strconv" "strings" "testing" + + "sneak.berlin/go/pixa/internal/imagerequest" ) // TestEvictionReadsTheUsageTotal adds, stores again, resizes and evicts @@ -171,7 +173,8 @@ func TestReconciliationReadsAPageAtATime(t *testing.T) { storeEvictionTestSource(t, cache, "pages.example.com", "/"+strconv.Itoa(i)+".jpg", bytes.Repeat(content, 100)) - storeEvictionTestVariant(t, cache, VariantKey("aabbccdd000"+strconv.Itoa(i)), + storeEvictionTestVariant(t, cache, + imagerequest.VariantKey("aabbccdd000"+strconv.Itoa(i)), bytes.Repeat(content, 10)) } diff --git a/internal/imgcache/concurrent_misses_internal_test.go b/internal/imgcache/concurrent_misses_internal_test.go index d763b99..cb1570f 100644 --- a/internal/imgcache/concurrent_misses_internal_test.go +++ b/internal/imgcache/concurrent_misses_internal_test.go @@ -16,6 +16,7 @@ import ( "github.com/getsentry/sentry-go" "sneak.berlin/go/pixa/internal/httpfetcher" "sneak.berlin/go/pixa/internal/imageprocessor" + "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/magic" ) @@ -80,14 +81,20 @@ func setupHeldFetchService(t *testing.T) (*Service, *TestFixtures, *heldFetcher) // photoVariant asks for the test photo, 100x100, at 50x25 as a JPEG of the // given quality and fit mode. Each call returns a new request, as Get writes // to the request it is given. -func photoVariant(fixtures *TestFixtures, quality int, fit FitMode) *ImageRequest { - return &ImageRequest{ - SourceHost: fixtures.GoodHost, - SourcePath: testPathPhoto, - Size: Size{Width: 50, Height: 25}, - Format: FormatJPEG, - Quality: quality, - FitMode: fit, +func photoVariant( + fixtures *TestFixtures, quality int, fit imagerequest.Fit, +) *imagerequest.Request { + return &imagerequest.Request{ + Source: imagerequest.Source{ + Host: fixtures.GoodHost, + Path: testPathPhoto, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 25}, + Format: imagerequest.FormatJPEG, + Quality: quality, + FitMode: fit, + }, } } @@ -100,7 +107,7 @@ type getResult struct { // startGet calls Get in a goroutine of its own and delivers what it returned // on the channel. func startGet( - ctx context.Context, svc *Service, req *ImageRequest, + ctx context.Context, svc *Service, req *imagerequest.Request, ) <-chan getResult { results := make(chan getResult, 1) @@ -146,7 +153,7 @@ func TestService_Get_ConcurrentMissesShareOneFetch(t *testing.T) { pending := make([]<-chan getResult, 0, requests) for range requests { pending = append(pending, - startGet(t.Context(), svc, photoVariant(fixtures, 85, FitCover))) + startGet(t.Context(), svc, photoVariant(fixtures, 85, imagerequest.FitCover))) } <-fetcher.started @@ -199,9 +206,11 @@ func TestService_Get_ConcurrentVariantsStayApart(t *testing.T) { svc, fixtures, fetcher := setupHeldFetchService(t) - cover := startGet(t.Context(), svc, photoVariant(fixtures, 85, FitCover)) - lowQuality := startGet(t.Context(), svc, photoVariant(fixtures, 40, FitCover)) - contain := startGet(t.Context(), svc, photoVariant(fixtures, 85, FitContain)) + cover := startGet(t.Context(), svc, photoVariant(fixtures, 85, imagerequest.FitCover)) + lowQuality := startGet(t.Context(), svc, + photoVariant(fixtures, 40, imagerequest.FitCover)) + contain := startGet(t.Context(), svc, + photoVariant(fixtures, 85, imagerequest.FitContain)) for range 3 { select { @@ -253,12 +262,12 @@ func TestService_Get_WaiterStopsWhenItsContextEnds(t *testing.T) { svc, fixtures, fetcher := setupHeldFetchService(t) - first := startGet(t.Context(), svc, photoVariant(fixtures, 85, FitCover)) + first := startGet(t.Context(), svc, photoVariant(fixtures, 85, imagerequest.FitCover)) <-fetcher.started waiterCtx, cancelWaiter := context.WithCancel(t.Context()) - waiter := startGet(waiterCtx, svc, photoVariant(fixtures, 85, FitCover)) + waiter := startGet(waiterCtx, svc, photoVariant(fixtures, 85, imagerequest.FitCover)) time.Sleep(arrivalWait) cancelWaiter() @@ -300,11 +309,11 @@ func TestService_Get_FirstRequestLeavingKeepsTheWork(t *testing.T) { svc, fixtures, fetcher := setupHeldFetchService(t) firstCtx, cancelFirst := context.WithCancel(t.Context()) - first := startGet(firstCtx, svc, photoVariant(fixtures, 85, FitCover)) + first := startGet(firstCtx, svc, photoVariant(fixtures, 85, imagerequest.FitCover)) <-fetcher.started - second := startGet(t.Context(), svc, photoVariant(fixtures, 85, FitCover)) + second := startGet(t.Context(), svc, photoVariant(fixtures, 85, imagerequest.FitCover)) time.Sleep(arrivalWait) cancelFirst() @@ -355,9 +364,9 @@ func TestService_Get_ConcurrentMissesShareAFailure(t *testing.T) { svc, fixtures, fetcher := setupHeldFetchService(t) - request := func() *ImageRequest { - req := photoVariant(fixtures, 85, FitCover) - req.SourcePath = tc.path + request := func() *imagerequest.Request { + req := photoVariant(fixtures, 85, imagerequest.FitCover) + req.Source.Path = tc.path return req } @@ -399,7 +408,7 @@ func TestService_Get_EndedRequestFetchesNothing(t *testing.T) { ctx, cancel := context.WithCancel(t.Context()) cancel() - _, err := svc.Get(ctx, photoVariant(fixtures, 85, FitCover)) + _, err := svc.Get(ctx, photoVariant(fixtures, 85, imagerequest.FitCover)) if !errors.Is(err, context.Canceled) { t.Errorf("Get() error = %v, want %v", err, context.Canceled) } @@ -423,7 +432,7 @@ func TestService_Get_ReturnsByItsDeadline(t *testing.T) { ctx, cancel := context.WithTimeout(t.Context(), timeout) defer cancel() - results := startGet(ctx, svc, photoVariant(fixtures, 85, FitCover)) + results := startGet(ctx, svc, photoVariant(fixtures, 85, imagerequest.FitCover)) select { case got := <-results: @@ -456,11 +465,11 @@ func TestService_Get_WaiterGetsProcessingTimedOut(t *testing.T) { firstCtx, cancelFirst := context.WithTimeout(t.Context(), firstTimeout) defer cancelFirst() - first := startGet(firstCtx, svc, photoVariant(fixtures, 85, FitCover)) + first := startGet(firstCtx, svc, photoVariant(fixtures, 85, imagerequest.FitCover)) <-fetcher.started - waiter := startGet(t.Context(), svc, photoVariant(fixtures, 85, FitCover)) + waiter := startGet(t.Context(), svc, photoVariant(fixtures, 85, imagerequest.FitCover)) time.Sleep(arrivalWait) fetcher.releaseFetches() @@ -503,7 +512,7 @@ func TestService_Get_PanicBecomesAnError(t *testing.T) { } }() - _, err = svc.Get(t.Context(), photoVariant(fixtures, 85, FitCover)) + _, err = svc.Get(t.Context(), photoVariant(fixtures, 85, imagerequest.FitCover)) }() if err == nil || !strings.Contains(err.Error(), "upstream fetcher panicked") { @@ -540,7 +549,7 @@ func TestService_Get_PanicIsReportedToSentry(t *testing.T) { } }() - _, _ = svc.Get(ctx, photoVariant(fixtures, 85, FitCover)) + _, _ = svc.Get(ctx, photoVariant(fixtures, 85, imagerequest.FitCover)) }() events := transport.Events() diff --git a/internal/imgcache/eviction.go b/internal/imgcache/eviction.go index 83828c4..4c7a9f7 100644 --- a/internal/imgcache/eviction.go +++ b/internal/imgcache/eviction.go @@ -11,6 +11,8 @@ import ( "path/filepath" "strings" "time" + + "sneak.berlin/go/pixa/internal/imagerequest" ) // DefaultEvictionInterval is how often the background evictor checks @@ -85,7 +87,7 @@ func (c *Cache) UsageBytes(ctx context.Context) (int64, error) { // source content blob (identified by contentHash). type evictionCandidate struct { isVariant bool - cacheKey VariantKey + cacheKey imagerequest.VariantKey contentHash ContentHash sizeBytes int64 lastAccessedAt string @@ -240,7 +242,7 @@ func (c *Cache) variantCandidates(ctx context.Context) ([]evictionCandidate, err return nil, fmt.Errorf("failed to scan variant candidate: %w", err) } - candidate.cacheKey = VariantKey(key) + candidate.cacheKey = imagerequest.VariantKey(key) candidates = append(candidates, candidate) } @@ -300,7 +302,9 @@ func (c *Cache) sourceCandidates(ctx context.Context) ([]evictionCandidate, erro // file. The metaCache entry goes before the files; a GetVariant that // read them just before may put it back, and the next GetVariant then // fails to open the file and removes it again. -func (c *Cache) evictVariant(ctx context.Context, cacheKey VariantKey) error { +func (c *Cache) evictVariant( + ctx context.Context, cacheKey imagerequest.VariantKey, +) error { _, err := c.db.ExecContext(ctx, `DELETE FROM variant_content WHERE cache_key = ?`, string(cacheKey)) if err != nil { @@ -648,7 +652,7 @@ func (c *Cache) reconcileVariantFiles(ctx context.Context) error { return nil } - return c.reconcileVariantFile(ctx, path, entry, VariantKey(name)) + return c.reconcileVariantFile(ctx, path, entry, imagerequest.VariantKey(name)) }, ) } @@ -659,7 +663,7 @@ func (c *Cache) reconcileVariantFiles(ctx context.Context) error { // crash or a restored snapshot can leave it, is removed with its row and // a warning naming it, so the variant is converted again. func (c *Cache) reconcileVariantFile( - ctx context.Context, path string, entry fs.DirEntry, cacheKey VariantKey, + ctx context.Context, path string, entry fs.DirEntry, cacheKey imagerequest.VariantKey, ) error { info, err := entry.Info() if err != nil { @@ -691,7 +695,7 @@ func (c *Cache) reconcileVariantFile( // adoptVariantFile inserts an accounting row for a variant file that // has none, using the file's size and modification time. func (c *Cache) adoptVariantFile( - ctx context.Context, path string, info fs.FileInfo, cacheKey VariantKey, + ctx context.Context, path string, info fs.FileInfo, cacheKey imagerequest.VariantKey, ) error { modTime := info.ModTime().UTC().Format(sqliteTimestampLayout) contentType := c.variantContentTypeFromSidecar(path) @@ -731,7 +735,7 @@ func (c *Cache) variantContentTypeFromSidecar(variantPath string) string { // reconcileVariantRows drops accounting rows whose variant files are // missing, so the database never references deleted content. func (c *Cache) reconcileVariantRows(ctx context.Context) error { - var after VariantKey + var after imagerequest.VariantKey for { keys, err := c.variantKeysAfter(ctx, after) @@ -772,9 +776,9 @@ func (c *Cache) reconcileVariantRows(ctx context.Context) error { // variantKeysAfter returns, in order, up to c.reconciliationPageSize // tracked variant cache keys that sort after the given one. func (c *Cache) variantKeysAfter( - ctx context.Context, after VariantKey, -) ([]VariantKey, error) { - return queryStringColumn[VariantKey](ctx, c.db, ` + ctx context.Context, after imagerequest.VariantKey, +) ([]imagerequest.VariantKey, error) { + return queryStringColumn[imagerequest.VariantKey](ctx, c.db, ` SELECT cache_key FROM variant_content WHERE cache_key > ? ORDER BY cache_key LIMIT ? `, "variant keys", "variant key", string(after), c.reconciliationPageSize) diff --git a/internal/imgcache/eviction_internal_test.go b/internal/imgcache/eviction_internal_test.go index d1ddf31..ce5087d 100644 --- a/internal/imgcache/eviction_internal_test.go +++ b/internal/imgcache/eviction_internal_test.go @@ -16,6 +16,7 @@ import ( _ "modernc.org/sqlite" "sneak.berlin/go/pixa/internal/database" "sneak.berlin/go/pixa/internal/httpfetcher" + "sneak.berlin/go/pixa/internal/imagerequest" ) // sqliteTimestampFormat matches the format SQLite's CURRENT_TIMESTAMP @@ -27,10 +28,10 @@ const sqliteTimestampFormat = "2006-01-02 15:04:05" // keys reused across the eviction tests, in the order the tests store // them. const ( - testVariantKeyOne VariantKey = "aabbccdd0001" - testVariantKeyTwo VariantKey = "aabbccdd0002" - testVariantKeyThree VariantKey = "aabbccdd0003" - testVariantKeyFour VariantKey = "aabbccdd0004" + testVariantKeyOne imagerequest.VariantKey = "aabbccdd0001" + testVariantKeyTwo imagerequest.VariantKey = "aabbccdd0002" + testVariantKeyThree imagerequest.VariantKey = "aabbccdd0003" + testVariantKeyFour imagerequest.VariantKey = "aabbccdd0004" ) // evictionTestDB creates an in-memory SQLite database with the real @@ -88,12 +89,16 @@ func storeEvictionTestSource( ) ContentHash { t.Helper() - req := &ImageRequest{ - SourceHost: host, - SourcePath: path, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: host, + Path: path, + }, + Transform: imagerequest.Transform{ + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } result := &httpfetcher.FetchResult{ @@ -118,7 +123,7 @@ func storeEvictionTestSource( // storeEvictionTestVariant stores content as a processed variant under // the given cache key. func storeEvictionTestVariant( - t *testing.T, cache *Cache, key VariantKey, content []byte, + t *testing.T, cache *Cache, key imagerequest.VariantKey, content []byte, ) { t.Helper() @@ -131,7 +136,7 @@ func storeEvictionTestVariant( // setVariantLastAccessed backdates the last access time of a tracked // variant, to make LRU ordering deterministic in tests. func setVariantLastAccessed( - t *testing.T, cache *Cache, key VariantKey, when time.Time, + t *testing.T, cache *Cache, key imagerequest.VariantKey, when time.Time, ) { t.Helper() @@ -244,7 +249,7 @@ func assertNoDanglingReferences(t *testing.T, cache *Cache) { t.Fatalf("failed to scan cache_key: %v", err) } - if !cache.variants.Exists(VariantKey(key)) { + if !cache.variants.Exists(imagerequest.VariantKey(key)) { t.Errorf("variant_content references key %s but the file is missing", key) } } @@ -343,7 +348,7 @@ func TestEvictToLimitEvictsLeastRecentlyUsedFirst(t *testing.T) { cache, _ := newEvictionTestCache(t, limit) now := time.Now() - keys := []VariantKey{ + keys := []imagerequest.VariantKey{ testVariantKeyOne, testVariantKeyTwo, testVariantKeyThree, testVariantKeyFour, } @@ -516,12 +521,16 @@ func TestZeroMaxBytesDisablesDiskCache(t *testing.T) { cache, tmpDir := newEvictionTestCache(t, 0) - req := &ImageRequest{ - SourceHost: "src.example.com", - SourcePath: "/a.jpg", - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: "src.example.com", + Path: "/a.jpg", + }, + Transform: imagerequest.Transform{ + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } assertDisabledCacheWritesAreNoOps(t, cache, req) @@ -533,14 +542,14 @@ func TestZeroMaxBytesDisablesDiskCache(t *testing.T) { // assertDisabledCacheWritesAreNoOps verifies that stores against a // disabled cache report success without recording anything. func assertDisabledCacheWritesAreNoOps( - t *testing.T, cache *Cache, req *ImageRequest, + t *testing.T, cache *Cache, req *imagerequest.Request, ) { t.Helper() ctx := t.Context() err := cache.StoreVariant( - ctx, CacheKey(req), bytes.NewReader([]byte("data")), "image/webp", + ctx, imagerequest.CacheKey(req), bytes.NewReader([]byte("data")), "image/webp", ) if err != nil { t.Fatalf("StoreVariant on disabled cache must be a no-op, got error: %v", err) @@ -566,7 +575,7 @@ func assertDisabledCacheWritesAreNoOps( // assertDisabledCacheReadsAlwaysMiss verifies that lookups against a // disabled cache never report a hit. func assertDisabledCacheReadsAlwaysMiss( - t *testing.T, cache *Cache, req *ImageRequest, + t *testing.T, cache *Cache, req *imagerequest.Request, ) { t.Helper() @@ -661,7 +670,7 @@ func TestEvictionRunsUnderWritePressure(t *testing.T) { cache.StartEviction(time.Hour) defer func() { _ = cache.StopEviction(t.Context()) }() - keys := []VariantKey{ + keys := []imagerequest.VariantKey{ testVariantKeyOne, testVariantKeyTwo, testVariantKeyThree, } fills := []byte{0x11, 0x12, 0x13} @@ -715,7 +724,7 @@ func TestEvictionRunsOnPeriodicSchedule(t *testing.T) { time.Sleep(10 * time.Millisecond) } - keys := []VariantKey{ + keys := []imagerequest.VariantKey{ testVariantKeyOne, testVariantKeyTwo, testVariantKeyThree, } fills := []byte{0x21, 0x22, 0x23} @@ -1308,9 +1317,11 @@ func TestEvictSourceBlobExcludesConcurrentStoreOfIdenticalContent(t *testing.T) assertNoDanglingReferences(t, cache) - dupHash, _, err := cache.LookupSource(ctx, &ImageRequest{ - SourceHost: "race.example.com", - SourcePath: "/dup.jpg", + dupHash, _, err := cache.LookupSource(ctx, &imagerequest.Request{ + Source: imagerequest.Source{ + Host: "race.example.com", + Path: "/dup.jpg", + }, }) if err != nil { t.Fatalf("LookupSource failed: %v", err) @@ -1336,12 +1347,16 @@ func storeIdenticalContentConcurrently( storeDone := make(chan error, 1) go func() { - req := &ImageRequest{ - SourceHost: "race.example.com", - SourcePath: "/dup.jpg", - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: "race.example.com", + Path: "/dup.jpg", + }, + Transform: imagerequest.Transform{ + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } result := &httpfetcher.FetchResult{ StatusCode: 200, diff --git a/internal/imgcache/imgcache.go b/internal/imgcache/imgcache.go index 51b33b3..8354741 100644 --- a/internal/imgcache/imgcache.go +++ b/internal/imgcache/imgcache.go @@ -3,134 +3,12 @@ package imgcache import ( "context" - "errors" "io" - "net/url" "time" + + "sneak.berlin/go/pixa/internal/imagerequest" ) -// ErrInvalidFitMode is returned when an invalid fit mode is provided. -var ErrInvalidFitMode = errors.New("invalid fit mode") - -// ImageFormat represents supported output image formats. -type ImageFormat string - -// Supported image output formats. -const ( - FormatOriginal ImageFormat = "orig" - FormatJPEG ImageFormat = "jpeg" - FormatPNG ImageFormat = "png" - FormatWebP ImageFormat = "webp" - FormatAVIF ImageFormat = "avif" - FormatJXL ImageFormat = "jxl" - FormatGIF ImageFormat = "gif" - - // FormatAuto stands for JPEG XL, AVIF, WebP or JPEG, chosen for each - // request from its Accept header once the URL's signature or token has - // been checked; it is never processed or cached as itself. - FormatAuto ImageFormat = "auto" -) - -// Size represents requested image dimensions -type Size struct { - Width int - Height int -} - -// OriginalSize returns true if this represents "keep original size" -func (s Size) OriginalSize() bool { - return s.Width == 0 && s.Height == 0 -} - -// FitMode represents how to fit image into requested dimensions. -type FitMode string - -// Supported image fit modes. -const ( - FitCover FitMode = "cover" - FitContain FitMode = "contain" - FitFill FitMode = "fill" - FitInside FitMode = "inside" - FitOutside FitMode = "outside" -) - -// ValidateFitMode checks if the given fit mode is valid. -// Returns ErrInvalidFitMode for unrecognized fit modes. -func ValidateFitMode(fit FitMode) error { - switch fit { - case FitCover, FitContain, FitFill, FitInside, FitOutside, "": - return nil - default: - return ErrInvalidFitMode - } -} - -// ValidateImageRequest checks a request's width and height with -// ValidateDimension and its fit mode with ValidateFitMode. Both the plain -// /v1/image/ route and the encrypted /v1/e/ route validate through this -// function so a request from either source enforces identical bounds. -func ValidateImageRequest(req *ImageRequest) error { - err := ValidateDimension("width", req.Size.Width) - if err != nil { - return err - } - - err = ValidateDimension("height", req.Size.Height) - if err != nil { - return err - } - - return ValidateFitMode(req.FitMode) -} - -// ImageRequest represents a request for a processed image -type ImageRequest struct { - // SourceHost is the origin host (e.g., "cdn.example.com") - SourceHost string - // SourcePath is the path on the origin with its percent-escapes decoded, - // as url.URL's Path holds it (e.g., "/photos/my cat.jpg") - SourcePath string - // SourceQuery is the optional query string for the origin URL as the URL - // writes it, percent-escapes included, as url.URL's RawQuery holds it - SourceQuery string - // Size is the requested output dimensions - Size Size - // Format is the requested output format - Format ImageFormat - // Quality is the output quality (1-100) for lossy formats - Quality int - // FitMode is how to fit the image into requested dimensions - FitMode FitMode - // Signature is the HMAC signature for non-allowlisted hosts - Signature string - // Expires is when the URL expires: the exp of a signed URL, or the expiry - // of an encrypted URL; the zero time if it has none - Expires time.Time - // AllowHTTP indicates whether HTTP (non-TLS) is allowed for this request - AllowHTTP bool -} - -// SourceURL returns the full upstream URL to fetch: the source path -// percent-encoded again, as net/url writes a path, so each source path is one -// upstream path, and the source query as it is, with the characters -// escapeQuery encodes percent-encoded. Uses http:// scheme when AllowHTTP is -// true, otherwise https://. -func (r *ImageRequest) SourceURL() string { - scheme := "https" - if r.AllowHTTP { - scheme = "http" - } - - sourceURL := url.URL{ - Scheme: scheme, - Host: r.SourceHost, - Path: r.SourcePath, - RawQuery: escapeQuery(r.SourceQuery), - } - - return sourceURL.String() -} - // ImageResponse represents a processed image ready to serve type ImageResponse struct { // Content is the image data reader @@ -162,10 +40,10 @@ const ( // ImageCache is the main interface for the image caching proxy type ImageCache interface { // Get retrieves a processed image, fetching and processing if necessary - Get(ctx context.Context, req *ImageRequest) (*ImageResponse, error) + Get(ctx context.Context, req *imagerequest.Request) (*ImageResponse, error) // Warm pre-fetches and caches an image without returning it - Warm(ctx context.Context, req *ImageRequest) error + Warm(ctx context.Context, req *imagerequest.Request) error // Stats returns cache statistics Stats(ctx context.Context) (*CacheStats, error) diff --git a/internal/imgcache/max_concurrent_processing_internal_test.go b/internal/imgcache/max_concurrent_processing_internal_test.go index e4c310e..69aeb31 100644 --- a/internal/imgcache/max_concurrent_processing_internal_test.go +++ b/internal/imgcache/max_concurrent_processing_internal_test.go @@ -9,18 +9,23 @@ import ( "time" "sneak.berlin/go/pixa/internal/imageprocessor" + "sneak.berlin/go/pixa/internal/imagerequest" ) // widthOnlyRequest asks for the test photo at width, its height scaled to // keep the photo's aspect ratio. -func widthOnlyRequest(fixtures *TestFixtures, width int) *ImageRequest { - return &ImageRequest{ - SourceHost: fixtures.GoodHost, - SourcePath: testPathPhoto, - Size: Size{Width: width}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, +func widthOnlyRequest(fixtures *TestFixtures, width int) *imagerequest.Request { + return &imagerequest.Request{ + Source: imagerequest.Source{ + Host: fixtures.GoodHost, + Path: testPathPhoto, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: width}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } } @@ -36,7 +41,7 @@ func holdProcessingSlot( input, feed := io.Pipe() go func() { - _, _ = proc.Process(t.Context(), input, &imageprocessor.Request{}) + _, _ = proc.Process(t.Context(), input, &imagerequest.Transform{}) }() _, err := feed.Write([]byte{0}) diff --git a/internal/imgcache/metacache_internal_test.go b/internal/imgcache/metacache_internal_test.go index bade726..1eecbad 100644 --- a/internal/imgcache/metacache_internal_test.go +++ b/internal/imgcache/metacache_internal_test.go @@ -10,23 +10,31 @@ import ( "sync" "testing" "time" + + "sneak.berlin/go/pixa/internal/imagerequest" ) // webpRequest returns a request for a 100x100 WebP variant of path. -func webpRequest(path string) *ImageRequest { - return &ImageRequest{ - SourceHost: testHostCDN, - SourcePath: path, - Size: Size{Width: 100, Height: 100}, - Format: FormatWebP, - Quality: 85, - FitMode: FitCover, +func webpRequest(path string) *imagerequest.Request { + return &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHostCDN, + Path: path, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 100, Height: 100}, + Format: imagerequest.FormatWebP, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } } // assertVariantServed checks that GetVariant serves key with the given // content and the image/webp content type storeEvictionTestVariant stores. -func assertVariantServed(t *testing.T, cache *Cache, key VariantKey, content []byte) { +func assertVariantServed( + t *testing.T, cache *Cache, key imagerequest.VariantKey, content []byte, +) { t.Helper() reader, size, contentType, err := cache.GetVariant(key) @@ -57,7 +65,9 @@ func assertVariantServed(t *testing.T, cache *Cache, key VariantKey, content []b // assertVariantNotFound checks that GetVariant refuses key with // ErrNotFound. -func assertVariantNotFound(t *testing.T, cache *Cache, key VariantKey) { +func assertVariantNotFound( + t *testing.T, cache *Cache, key imagerequest.VariantKey, +) { t.Helper() reader, _, _, err := cache.GetVariant(key) @@ -71,16 +81,18 @@ func assertVariantNotFound(t *testing.T, cache *Cache, key VariantKey) { } // assertLookupMisses checks that Lookup reports request as a miss. -func assertLookupMisses(t *testing.T, cache *Cache, request *ImageRequest) { +func assertLookupMisses( + t *testing.T, cache *Cache, request *imagerequest.Request, +) { t.Helper() lookup, err := cache.Lookup(t.Context(), request) if err != nil { - t.Fatalf("Lookup(%s) error = %v", request.SourcePath, err) + t.Fatalf("Lookup(%s) error = %v", request.Source.Path, err) } if lookup.Hit { - t.Errorf("Lookup(%s) is a hit, want a miss", request.SourcePath) + t.Errorf("Lookup(%s) is a hit, want a miss", request.Source.Path) } } @@ -251,8 +263,8 @@ func TestEvictedVariantIsNotServed(t *testing.T) { oldRequest := webpRequest("/old.jpg") newRequest := webpRequest("/new.jpg") - oldKey := CacheKey(oldRequest) - newKey := CacheKey(newRequest) + oldKey := imagerequest.CacheKey(oldRequest) + newKey := imagerequest.CacheKey(newRequest) oldContent := bytes.Repeat([]byte{0x01}, 1000) newContent := bytes.Repeat([]byte{0x02}, 1000) @@ -280,7 +292,7 @@ func TestVariantDeletedFromDiskIsNotServed(t *testing.T) { cache, _ := newEvictionTestCache(t, 1<<20) request := webpRequest("/deleted.jpg") - key := CacheKey(request) + key := imagerequest.CacheKey(request) content := []byte("webp variant bytes") storeEvictionTestVariant(t, cache, key, content) @@ -307,7 +319,7 @@ func TestConcurrentVariantStoreReadAndEvict(t *testing.T) { for goroutine := range 8 { wg.Go(func() { - key := VariantKey(fmt.Sprintf("aabbccdd01%02d", goroutine)) + key := imagerequest.VariantKey(fmt.Sprintf("aabbccdd01%02d", goroutine)) content := []byte(key) for range 20 { diff --git a/internal/imgcache/negative_cache_internal_test.go b/internal/imgcache/negative_cache_internal_test.go index dae2433..1c3b2bf 100644 --- a/internal/imgcache/negative_cache_internal_test.go +++ b/internal/imgcache/negative_cache_internal_test.go @@ -6,6 +6,8 @@ import ( "strconv" "testing" "time" + + "sneak.berlin/go/pixa/internal/imagerequest" ) func TestNegativeCache_StoreAndCheck(t *testing.T) { @@ -23,9 +25,11 @@ func TestNegativeCache_StoreAndCheck(t *testing.T) { } ctx := context.Background() - req := &ImageRequest{ - SourceHost: testHostExample, - SourcePath: "/missing.jpg", + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHostExample, + Path: "/missing.jpg", + }, } // Initially should not be in negative cache @@ -70,9 +74,11 @@ func TestNegativeCache_Expired(t *testing.T) { } ctx := context.Background() - req := &ImageRequest{ - SourceHost: testHostExample, - SourcePath: "/expired.jpg", + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHostExample, + Path: "/expired.jpg", + }, } // Store a negative entry with very short TTL @@ -106,20 +112,21 @@ func TestService_Get_ServesCachedVariantOfNegativeCachedURL(t *testing.T) { svc, fixtures := SetupTestService(t) - resp, err := svc.Get(t.Context(), photoVariant(fixtures, 85, FitCover)) + resp, err := svc.Get(t.Context(), photoVariant(fixtures, 85, imagerequest.FitCover)) if err != nil { t.Fatalf("Get() first request error = %v", err) } _ = resp.Content.Close() - err = svc.cache.StoreNegative(t.Context(), photoVariant(fixtures, 85, FitCover), + err = svc.cache.StoreNegative(t.Context(), + photoVariant(fixtures, 85, imagerequest.FitCover), httpStatusBadGateway, "upstream answered 503") if err != nil { t.Fatalf("StoreNegative() error = %v", err) } - resp, err = svc.Get(t.Context(), photoVariant(fixtures, 85, FitCover)) + resp, err = svc.Get(t.Context(), photoVariant(fixtures, 85, imagerequest.FitCover)) if err != nil { t.Fatalf("Get() of the cached variant error = %v", err) } @@ -131,7 +138,7 @@ func TestService_Get_ServesCachedVariantOfNegativeCachedURL(t *testing.T) { resp.CacheStatus, CacheHit) } - resp, err = svc.Get(t.Context(), photoVariant(fixtures, 40, FitCover)) + resp, err = svc.Get(t.Context(), photoVariant(fixtures, 40, imagerequest.FitCover)) t.Logf("Get() of a variant made from the cached source: error = %v", err) if err != nil { @@ -141,7 +148,7 @@ func TestService_Get_ServesCachedVariantOfNegativeCachedURL(t *testing.T) { _ = resp.Content.Close() contentHash, _, err := svc.cache.LookupSource(t.Context(), - photoVariant(fixtures, 85, FitCover)) + photoVariant(fixtures, 85, imagerequest.FitCover)) if err != nil || contentHash == "" { t.Fatalf("LookupSource() = %q, %v, want the cached source", contentHash, err) } @@ -151,7 +158,7 @@ func TestService_Get_ServesCachedVariantOfNegativeCachedURL(t *testing.T) { t.Fatalf("evictSourceBlob() error = %v", err) } - _, err = svc.Get(t.Context(), photoVariant(fixtures, 60, FitCover)) + _, err = svc.Get(t.Context(), photoVariant(fixtures, 60, imagerequest.FitCover)) t.Logf("Get() of a variant whose source was evicted: error = %v", err) if !errors.Is(err, ErrNegativeCached) { @@ -190,8 +197,8 @@ func TestEvictionLoopRemovesExpiredNegativeCacheRows(t *testing.T) { t.Fatalf("failed to create cache: %v", err) } - expired := &ImageRequest{ - SourceHost: testHostExample, SourcePath: "/gone.jpg", + expired := &imagerequest.Request{ + Source: imagerequest.Source{Host: testHostExample, Path: "/gone.jpg"}, } err = cache.StoreNegative(t.Context(), expired, 404, "not found") @@ -215,7 +222,7 @@ func TestEvictionLoopRemovesExpiredNegativeCacheRows(t *testing.T) { cache.StartEviction(100 * time.Millisecond) defer func() { _ = cache.StopEviction(t.Context()) }() - for negativeCacheRowCount(t, cache, expired.SourcePath) > 0 { + for negativeCacheRowCount(t, cache, expired.Source.Path) > 0 { if time.Since(started) > 10*time.Second { t.Fatal("the expired row is still in negative_cache") } diff --git a/internal/imgcache/partial_cache_files_internal_test.go b/internal/imgcache/partial_cache_files_internal_test.go index 283ac13..364d5cc 100644 --- a/internal/imgcache/partial_cache_files_internal_test.go +++ b/internal/imgcache/partial_cache_files_internal_test.go @@ -10,23 +10,27 @@ import ( "path/filepath" "strings" "testing" + + "sneak.berlin/go/pixa/internal/imagerequest" ) // getImage calls Get for req and returns the image it answered with and // its cache status. -func getImage(t *testing.T, svc *Service, req *ImageRequest) ([]byte, CacheStatus) { +func getImage( + t *testing.T, svc *Service, req *imagerequest.Request, +) ([]byte, CacheStatus) { t.Helper() resp, err := svc.Get(t.Context(), req) if err != nil { - t.Fatalf("Get() of quality %d error = %v", req.Quality, err) + t.Fatalf("Get() of quality %d error = %v", req.Transform.Quality, err) } defer func() { _ = resp.Content.Close() }() body, err := io.ReadAll(resp.Content) if err != nil { - t.Fatalf("reading the image of quality %d: %v", req.Quality, err) + t.Fatalf("reading the image of quality %d: %v", req.Transform.Quality, err) } return body, resp.CacheStatus @@ -58,7 +62,9 @@ func truncateFile(t *testing.T, path string, size int64) { } // cachedSourcePath returns the path of the cached source file req uses. -func cachedSourcePath(t *testing.T, svc *Service, req *ImageRequest) string { +func cachedSourcePath( + t *testing.T, svc *Service, req *imagerequest.Request, +) string { t.Helper() contentHash, _, err := svc.cache.LookupSource(t.Context(), req) @@ -113,16 +119,16 @@ func TestService_Get_FetchesAgainForEmptyCachedSource(t *testing.T) { svc, fixtures := SetupTestService(t) - getImage(t, svc, photoVariant(fixtures, 85, FitCover)) + getImage(t, svc, photoVariant(fixtures, 85, imagerequest.FitCover)) - path := cachedSourcePath(t, svc, photoVariant(fixtures, 85, FitCover)) + path := cachedSourcePath(t, svc, photoVariant(fixtures, 85, imagerequest.FitCover)) wholeSize := fileSize(t, path) truncateFile(t, path, 0) assertWarningNames(t, reconcileAsAtStartup(t, svc.cache), path) - getImage(t, svc, photoVariant(fixtures, 40, FitCover)) - getImage(t, svc, photoVariant(fixtures, 30, FitCover)) + getImage(t, svc, photoVariant(fixtures, 40, imagerequest.FitCover)) + getImage(t, svc, photoVariant(fixtures, 30, imagerequest.FitCover)) fetches := readCacheStatsCounters(t, svc.cache).upstreamFetchCount t.Logf("upstream fetches = %d", fetches) @@ -149,16 +155,16 @@ func TestService_Get_FetchesAgainForPartialCachedSource(t *testing.T) { svc, fixtures := SetupTestService(t) - getImage(t, svc, photoVariant(fixtures, 85, FitCover)) + getImage(t, svc, photoVariant(fixtures, 85, imagerequest.FitCover)) - path := cachedSourcePath(t, svc, photoVariant(fixtures, 85, FitCover)) + path := cachedSourcePath(t, svc, photoVariant(fixtures, 85, imagerequest.FitCover)) wholeSize := fileSize(t, path) truncateFile(t, path, wholeSize/2) assertWarningNames(t, reconcileAsAtStartup(t, svc.cache), path) - getImage(t, svc, photoVariant(fixtures, 40, FitCover)) - getImage(t, svc, photoVariant(fixtures, 30, FitCover)) + getImage(t, svc, photoVariant(fixtures, 40, imagerequest.FitCover)) + getImage(t, svc, photoVariant(fixtures, 30, imagerequest.FitCover)) fetches := readCacheStatsCounters(t, svc.cache).upstreamFetchCount t.Logf("upstream fetches = %d", fetches) @@ -182,11 +188,11 @@ func assertConvertedAgain(t *testing.T, size int64) { t.Helper() svc, fixtures := SetupTestService(t) - req := photoVariant(fixtures, 85, FitCover) + req := photoVariant(fixtures, 85, imagerequest.FitCover) want, _ := getImage(t, svc, req) - path := svc.cache.variants.keyToPath(CacheKey(req)) + path := svc.cache.variants.keyToPath(imagerequest.CacheKey(req)) truncateFile(t, path, size) assertWarningNames(t, reconcileAsAtStartup(t, svc.cache), path) diff --git a/internal/imgcache/pendingcounts_internal_test.go b/internal/imgcache/pendingcounts_internal_test.go index 77dd73e..e8a559a 100644 --- a/internal/imgcache/pendingcounts_internal_test.go +++ b/internal/imgcache/pendingcounts_internal_test.go @@ -6,6 +6,8 @@ import ( "errors" "testing" "time" + + "sneak.berlin/go/pixa/internal/imagerequest" ) // pendingCountsWait is how long a test waits for the database's connection, @@ -92,7 +94,7 @@ func TestService_Get_ReturnsByItsDeadlineWhileTheDatabaseIsBusy(t *testing.T) { deadline, _ := ctx.Deadline() - results := startGet(ctx, svc, photoVariant(fixtures, 85, FitCover)) + results := startGet(ctx, svc, photoVariant(fixtures, 85, imagerequest.FitCover)) // The request has made its database reads by the time it fetches. select { diff --git a/internal/imgcache/service.go b/internal/imgcache/service.go index ced31b4..25d19fa 100644 --- a/internal/imgcache/service.go +++ b/internal/imgcache/service.go @@ -19,6 +19,7 @@ import ( "sneak.berlin/go/pixa/internal/allowlist" "sneak.berlin/go/pixa/internal/httpfetcher" "sneak.berlin/go/pixa/internal/imageprocessor" + "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/magic" "sneak.berlin/go/pixa/internal/signature" ) @@ -139,7 +140,9 @@ var ErrProcessingTimedOut = errors.New( // While its source URL is in the negative cache, a cached variant is still // served and a variant not in the cache is still made from a cached source; // only a request that needs the source fetched is refused. -func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, error) { +func (s *Service) Get( + ctx context.Context, req *imagerequest.Request, +) (*ImageResponse, error) { // Propagate AllowHTTP setting to the request req.AllowHTTP = s.allowHTTP @@ -186,7 +189,7 @@ func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, e } // Warm pre-fetches and caches an image without returning it. -func (s *Service) Warm(ctx context.Context, req *ImageRequest) error { +func (s *Service) Warm(ctx context.Context, req *imagerequest.Request) error { _, err := s.Get(ctx, req) return err @@ -204,7 +207,7 @@ func (s *Service) WaitForProcessing(ctx context.Context) int { } // ValidateRequest validates the request signature if required. -func (s *Service) ValidateRequest(req *ImageRequest) error { +func (s *Service) ValidateRequest(req *imagerequest.Request) error { // Check if host is allowed (no signature required) sourceURL := req.SourceURL() @@ -227,7 +230,7 @@ func (s *Service) ValidateRequest(req *ImageRequest) error { } // Signature required for non-allowed hosts - return s.signer.Verify(signatureRequest(req)) + return s.signer.Verify(req) } // GenerateSignedURL generates a signed URL for the given request. The URL @@ -236,26 +239,22 @@ func (s *Service) ValidateRequest(req *ImageRequest) error { // set to 85 or cover, the values the route uses when a URL has no q or fit. func (s *Service) GenerateSignedURL( baseURL string, - req *ImageRequest, + req *imagerequest.Request, ttl time.Duration, ) (string, error) { - if req.Quality == 0 { - req.Quality = 85 + if req.Transform.Quality == 0 { + req.Transform.Quality = 85 } - if req.FitMode == "" { - req.FitMode = FitCover + if req.Transform.FitMode == "" { + req.Transform.FitMode = imagerequest.FitCover } - sigReq := signatureRequest(req) - path, sig, exp := s.signer.GenerateSignedURL(sigReq, ttl) - - // Propagate the generated signature and expiration back onto the request. - req.Expires = sigReq.Expires - req.Signature = sigReq.Signature + // Sets the request's Expires and Signature + path, sig, exp := s.signer.GenerateSignedURL(req, ttl) return fmt.Sprintf("%s%s?sig=%s&exp=%d&q=%d&fit=%s", - baseURL, path, sig, exp, req.Quality, req.FitMode), nil + baseURL, path, sig, exp, req.Transform.Quality, req.Transform.FitMode), nil } // errPanicked is returned when processing a variant panicked. @@ -268,14 +267,14 @@ var errPanicked = errors.New("panic while processing image") // the first request's cancellation, so the others are still served if that // client goes away, but keeps its deadline. func (s *Service) processOrWait( - ctx context.Context, req *ImageRequest, + ctx context.Context, req *imagerequest.Request, ) (*ImageResponse, error) { // A request that has already ended starts no processing if ctx.Err() != nil { return nil, ctx.Err() } - cacheKey := CacheKey(req) + cacheKey := imagerequest.CacheKey(req) // Closed when this request's own function runs, which singleflight does // only when no other request is processing the variant @@ -293,7 +292,7 @@ func (s *Service) processOrWait( recovered := recover() if recovered != nil { s.log.Error("panic while processing image", - "host", req.SourceHost, "path", req.SourcePath, + "host", req.Source.Host, "path", req.Source.Path, "panic", recovered, "stack", string(debug.Stack())) if hub := sentry.GetHubFromContext(ctx); hub != nil { @@ -393,8 +392,8 @@ func (s *Service) loadCachedSource( // if available. func (s *Service) processFromSourceOrFetch( ctx context.Context, - req *ImageRequest, - cacheKey VariantKey, + req *imagerequest.Request, + cacheKey imagerequest.VariantKey, ) (*processedVariant, error) { // Check if we have cached source content contentHash, _, err := s.cache.LookupSource(ctx, req) @@ -431,8 +430,8 @@ func (s *Service) processFromSourceOrFetch( // source URL is in the negative cache. func (s *Service) fetchAndProcess( ctx context.Context, - req *ImageRequest, - cacheKey VariantKey, + req *imagerequest.Request, + cacheKey imagerequest.VariantKey, ) (*processedVariant, error) { // Check negative cache - skip fetching for recently-failed URLs negHit, err := s.cache.checkNegativeCache(ctx, req) @@ -442,8 +441,8 @@ func (s *Service) fetchAndProcess( if negHit { s.log.Debug("negative cache hit", - "host", req.SourceHost, - "path", req.SourcePath, + "host", req.Source.Host, + "path", req.Source.Path, ) return nil, fmt.Errorf("%w: %w", httpfetcher.ErrUpstreamError, ErrNegativeCached) @@ -493,8 +492,8 @@ func (s *Service) fetchAndProcess( // Log upstream fetch details s.log.Info("upstream fetched", "request_id", middleware.GetReqID(ctx), - "host", req.SourceHost, - "path", req.SourcePath, + "host", req.Source.Host, + "path", req.Source.Path, "bytes", fetchBytes, "fetch_ms", fetchResult.FetchDurationMs, "rate", downloadRate, @@ -510,7 +509,7 @@ func (s *Service) fetchAndProcess( err = magic.ValidateMagicBytes(sourceData, fetchResult.ContentType) if err != nil { s.log.Warn("source refused", "request_id", middleware.GetReqID(ctx), - "host", req.SourceHost, "path", req.SourcePath, + "host", req.Source.Host, "path", req.Source.Path, "content_type", fetchResult.ContentType, "error", err) s.storeNegative(ctx, req, httpStatusUnsupportedMediaType, err) @@ -533,12 +532,12 @@ func (s *Service) fetchAndProcess( // failed with err, so the source is not fetched again until the entry // expires. A failure to store the entry is only logged. func (s *Service) storeNegative( - ctx context.Context, req *ImageRequest, statusCode int, err error, + ctx context.Context, req *imagerequest.Request, statusCode int, err error, ) { storeErr := s.cache.StoreNegative(ctx, req, statusCode, err.Error()) if storeErr != nil { s.log.Warn("failed to store negative cache entry", - "host", req.SourceHost, "path", req.SourcePath, "error", storeErr) + "host", req.Source.Host, "path", req.Source.Path, "error", storeErr) } } @@ -554,24 +553,17 @@ type processedVariant struct { // result. func (s *Service) processAndStore( ctx context.Context, - req *ImageRequest, - cacheKey VariantKey, + req *imagerequest.Request, + cacheKey imagerequest.VariantKey, source io.Reader, fetchBytes int64, ) (*processedVariant, error) { // Process the image processStart := time.Now() - processReq := &imageprocessor.Request{ - Size: imageprocessor.Size{Width: req.Size.Width, Height: req.Size.Height}, - Format: imageprocessor.Format(req.Format), - Quality: req.Quality, - FitMode: imageprocessor.FitMode(req.FitMode), - } - // ctx has the deadline of the request doing the processing, which can // end while a request waiting for its image has time left - processResult, err := s.processor.Process(ctx, source, processReq) + processResult, err := s.processor.Process(ctx, source, &req.Transform) if errors.Is(err, context.DeadlineExceeded) { return nil, fmt.Errorf("%w: %w", ErrProcessingTimedOut, err) } @@ -604,10 +596,10 @@ func (s *Service) processAndStore( s.log.Info("image converted", "request_id", middleware.GetReqID(ctx), - "host", req.SourceHost, - "path", req.SourcePath, + "host", req.Source.Host, + "path", req.Source.Path, "src_format", processResult.InputFormat, - "dst_format", req.Format, + "dst_format", req.Transform.Format, "src_bytes", fetchBytes, "dst_bytes", outputSize, "src_dimensions", fmt.Sprintf("%dx%d", @@ -616,8 +608,8 @@ func (s *Service) processAndStore( processResult.Width, processResult.Height), "size_ratio", fmt.Sprintf("%.1f%%", sizePercent), "convert_ms", processDuration.Milliseconds(), - "quality", req.Quality, - "fit", req.FitMode, + "quality", req.Transform.Quality, + "fit", req.Transform.FitMode, ) // Store variant to cache @@ -636,24 +628,6 @@ func (s *Service) processAndStore( }, nil } -// signatureRequest projects an ImageRequest onto the standalone -// signature.Request type used by the signature package. This keeps the -// import edge one-way: imgcache depends on signature, never the reverse. -func signatureRequest(req *ImageRequest) *signature.Request { - return &signature.Request{ - SourceHost: req.SourceHost, - SourcePath: req.SourcePath, - SourceQuery: req.SourceQuery, - Width: req.Size.Width, - Height: req.Size.Height, - Format: string(req.Format), - Quality: req.Quality, - FitMode: string(req.FitMode), - Signature: req.Signature, - Expires: req.Expires, - } -} - // HTTP status codes for error responses. const ( httpStatusBadGateway = 502 @@ -680,7 +654,7 @@ func extractStatusCode(err error) int { const etagHashLength = 16 // formatETag formats a VariantKey as a quoted ETag value. -func formatETag(key VariantKey) string { +func formatETag(key imagerequest.VariantKey) string { hash := string(key) // Use first 16 characters of hash for a shorter but still unique ETag if len(hash) > etagHashLength { diff --git a/internal/imgcache/service_internal_test.go b/internal/imgcache/service_internal_test.go index e4bbb30..712955c 100644 --- a/internal/imgcache/service_internal_test.go +++ b/internal/imgcache/service_internal_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/magic" "sneak.berlin/go/pixa/internal/signature" ) @@ -25,13 +26,17 @@ func TestService_Get_AllowlistedHost(t *testing.T) { svc, fixtures := SetupTestService(t) ctx := context.Background() - req := &ImageRequest{ - SourceHost: fixtures.GoodHost, - SourcePath: testPathPhoto, - Size: Size{Width: 50, Height: 50}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: fixtures.GoodHost, + Path: testPathPhoto, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } resp, err := svc.Get(ctx, req) @@ -61,13 +66,17 @@ func TestService_Get_NonAllowlistedHost_NoSignature(t *testing.T) { svc, fixtures := SetupTestService(t, WithSigningKey("test-key")) - req := &ImageRequest{ - SourceHost: fixtures.OtherHost, - SourcePath: testPathUpload, - Size: Size{Width: 50, Height: 50}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: fixtures.OtherHost, + Path: testPathUpload, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } // Should fail validation - not allowlisted and no signature @@ -84,19 +93,23 @@ func TestService_Get_NonAllowlistedHost_ValidSignature(t *testing.T) { svc, fixtures := SetupTestService(t, WithSigningKey(signingKey)) ctx := context.Background() - req := &ImageRequest{ - SourceHost: fixtures.OtherHost, - SourcePath: testPathUpload, - Size: Size{Width: 50, Height: 50}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: fixtures.OtherHost, + Path: testPathUpload, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } // Generate a valid signature signer := signature.New(signingKey) req.Expires = time.Now().Add(time.Hour) - req.Signature = signer.Sign(signatureRequest(req)) + req.Signature = signer.Sign(req) // Should pass validation err := svc.ValidateRequest(req) @@ -128,19 +141,23 @@ func TestService_Get_NonAllowlistedHost_ExpiredSignature(t *testing.T) { signingKey := testSigningKey svc, fixtures := SetupTestService(t, WithSigningKey(signingKey)) - req := &ImageRequest{ - SourceHost: fixtures.OtherHost, - SourcePath: testPathUpload, - Size: Size{Width: 50, Height: 50}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: fixtures.OtherHost, + Path: testPathUpload, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } // Generate an expired signature signer := signature.New(signingKey) req.Expires = time.Now().Add(-time.Hour) // Already expired - req.Signature = signer.Sign(signatureRequest(req)) + req.Signature = signer.Sign(req) // Should fail validation err := svc.ValidateRequest(req) @@ -155,13 +172,17 @@ func TestService_Get_NonAllowlistedHost_InvalidSignature(t *testing.T) { signingKey := testSigningKey svc, fixtures := SetupTestService(t, WithSigningKey(signingKey)) - req := &ImageRequest{ - SourceHost: fixtures.OtherHost, - SourcePath: testPathUpload, - Size: Size{Width: 50, Height: 50}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: fixtures.OtherHost, + Path: testPathUpload, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } // Set an invalid signature @@ -191,16 +212,20 @@ func TestService_ValidateRequest_SignatureExactHostMatch(t *testing.T) { signer := signature.New(signingKey) // Sign a request for "cdn.example.com" - signedReq := &ImageRequest{ - SourceHost: testHostCDN, - SourcePath: testPathCat, - Size: Size{Width: 50, Height: 50}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, - Expires: time.Now().Add(time.Hour), + signedReq := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHostCDN, + Path: testPathCat, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, + Expires: time.Now().Add(time.Hour), } - signedReq.Signature = signer.Sign(signatureRequest(signedReq)) + signedReq.Signature = signer.Sign(signedReq) // The original request should pass validation t.Run("exact host passes", func(t *testing.T) { @@ -228,23 +253,22 @@ func TestService_ValidateRequest_SignatureExactHostMatch(t *testing.T) { t.Run(tt.name+" rejected", func(t *testing.T) { t.Parallel() - req := &ImageRequest{ - SourceHost: tt.host, - SourcePath: signedReq.SourcePath, - SourceQuery: signedReq.SourceQuery, - Size: signedReq.Size, - Format: signedReq.Format, - Quality: signedReq.Quality, - FitMode: signedReq.FitMode, - Expires: signedReq.Expires, - Signature: signedReq.Signature, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: tt.host, + Path: signedReq.Source.Path, + Query: signedReq.Source.Query, + }, + Transform: signedReq.Transform, + Expires: signedReq.Expires, + Signature: signedReq.Signature, } err := svc.ValidateRequest(req) if err == nil { t.Errorf( "ValidateRequest() should reject signature for host %q (signed for %q)", - tt.host, signedReq.SourceHost) + tt.host, signedReq.Source.Host) } }) } @@ -280,13 +304,17 @@ func TestService_ValidateRequest_HostWithPortNeedsSignature(t *testing.T) { t.Run(tt.host, func(t *testing.T) { t.Parallel() - req := &ImageRequest{ - SourceHost: tt.host, - SourcePath: testPathCat, - Size: Size{Width: 50, Height: 50}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: tt.host, + Path: testPathCat, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } err := svc.ValidateRequest(req) @@ -320,13 +348,17 @@ func TestService_ValidateRequest_UnbracketedIPv6PortNeedsSignature( t.Run(host, func(t *testing.T) { t.Parallel() - req := &ImageRequest{ - SourceHost: host, - SourcePath: testPathCat, - Size: Size{Width: 50, Height: 50}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: host, + Path: testPathCat, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } err := svc.ValidateRequest(req) @@ -347,14 +379,9 @@ func TestService_Get_InvalidFile(t *testing.T) { svc, fixtures := SetupTestService(t) ctx := context.Background() - req := &ImageRequest{ - SourceHost: fixtures.GoodHost, - SourcePath: "/images/fake.jpg", - Size: Size{Width: 50, Height: 50}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, - } + req := photoVariant(fixtures, 85, imagerequest.FitCover) + req.Source.Path = "/images/fake.jpg" + req.Transform.Size = imagerequest.Size{Width: 50, Height: 50} // Should fail because magic bytes don't match _, err := svc.Get(ctx, req) @@ -369,14 +396,9 @@ func TestService_Get_NotFound(t *testing.T) { svc, fixtures := SetupTestService(t) ctx := context.Background() - req := &ImageRequest{ - SourceHost: fixtures.GoodHost, - SourcePath: "/images/nonexistent.jpg", - Size: Size{Width: 50, Height: 50}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, - } + req := photoVariant(fixtures, 85, imagerequest.FitCover) + req.Source.Path = "/images/nonexistent.jpg" + req.Transform.Size = imagerequest.Size{Width: 50, Height: 50} _, err := svc.Get(ctx, req) if err == nil { @@ -393,25 +415,25 @@ func TestService_Get_FormatConversion(t *testing.T) { tests := []struct { name string sourcePath string - outFormat ImageFormat + outFormat imagerequest.Format wantMIME string }{ { name: "JPEG to PNG", sourcePath: testPathPhoto, - outFormat: FormatPNG, + outFormat: imagerequest.FormatPNG, wantMIME: "image/png", }, { name: "PNG to JPEG", sourcePath: "/images/logo.png", - outFormat: FormatJPEG, + outFormat: imagerequest.FormatJPEG, wantMIME: testContentTypeJPEG, }, { name: "GIF to PNG", sourcePath: "/images/animation.gif", - outFormat: FormatPNG, + outFormat: imagerequest.FormatPNG, wantMIME: "image/png", }, } @@ -420,13 +442,12 @@ func TestService_Get_FormatConversion(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - req := &ImageRequest{ - SourceHost: fixtures.GoodHost, - SourcePath: tt.sourcePath, - Size: Size{Width: 50, Height: 50}, - Format: tt.outFormat, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{Host: fixtures.GoodHost, Path: tt.sourcePath}, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: tt.outFormat, Quality: 85, FitMode: imagerequest.FitCover, + }, } resp, err := svc.Get(ctx, req) @@ -474,13 +495,17 @@ func TestService_Get_Caching(t *testing.T) { svc, fixtures := SetupTestService(t) ctx := context.Background() - req := &ImageRequest{ - SourceHost: fixtures.GoodHost, - SourcePath: testPathPhoto, - Size: Size{Width: 50, Height: 50}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: fixtures.GoodHost, + Path: testPathPhoto, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } // First request - should be a cache miss @@ -544,7 +569,7 @@ func TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo(t *testing.T) { t.Parallel() svc, fixtures := SetupTestService(t) - variant := photoVariant(fixtures, 85, FitCover) + variant := photoVariant(fixtures, 85, imagerequest.FitCover) resp, err := svc.Get(t.Context(), variant) if err != nil { @@ -570,7 +595,7 @@ func TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo(t *testing.T) { } // Another quality is a new variant, made from the cached source - resp, err = svc.Get(t.Context(), photoVariant(fixtures, 40, FitCover)) + resp, err = svc.Get(t.Context(), photoVariant(fixtures, 40, imagerequest.FitCover)) if err != nil { t.Fatalf("Get() of a new variant error = %v", err) } @@ -586,20 +611,20 @@ func TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo(t *testing.T) { // backdateSource sets every time pixa stored for req's cached source to when, // as listed on TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo. func backdateSource( - t *testing.T, cache *Cache, req *ImageRequest, when time.Time, + t *testing.T, cache *Cache, req *imagerequest.Request, when time.Time, ) { t.Helper() - pathHash := HashPath(req.SourcePath + "?" + req.SourceQuery) + pathHash := HashPath(req.Source.Path + "?" + req.Source.Query) - meta, err := cache.srcMetadata.Load(req.SourceHost, pathHash) + meta, err := cache.srcMetadata.Load(req.Source.Host, pathHash) if err != nil { t.Fatalf("reading the source's metadata file: %v", err) } meta.FetchedAt = when.Unix() - err = cache.srcMetadata.Store(req.SourceHost, pathHash, meta) + err = cache.srcMetadata.Store(req.Source.Host, pathHash, meta) if err != nil { t.Fatalf("writing the source's metadata file: %v", err) } @@ -609,7 +634,7 @@ func backdateSource( updateOneRow(t, cache, ` UPDATE source_metadata SET fetched_at = ? WHERE source_host = ? AND source_path = ? AND source_query = ? - `, dbTime, req.SourceHost, req.SourcePath, req.SourceQuery) + `, dbTime, req.Source.Host, req.Source.Path, req.Source.Query) updateOneRow(t, cache, `UPDATE source_content SET fetched_at = ? WHERE content_hash = ?`, dbTime, meta.ContentHash) @@ -617,7 +642,7 @@ func backdateSource( for _, path := range []string{ cache.srcContent.hashToPath(ContentHash(meta.ContentHash)), - cache.srcMetadata.metaPath(req.SourceHost, pathHash), + cache.srcMetadata.metaPath(req.Source.Host, pathHash), } { err = os.Chtimes(path, when, when) if err != nil { @@ -630,11 +655,11 @@ func backdateSource( // for to when, as listed on // TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo. func backdateVariant( - t *testing.T, cache *Cache, req *ImageRequest, when time.Time, + t *testing.T, cache *Cache, req *imagerequest.Request, when time.Time, ) { t.Helper() - key := CacheKey(req) + key := imagerequest.CacheKey(req) content, size, contentType, err := cache.variants.LoadWithMeta(key) if err != nil || contentType == "" { @@ -696,7 +721,7 @@ func TestService_Get_DifferentSizes(t *testing.T) { ctx := context.Background() // Request same image at different sizes - sizes := []Size{ + sizes := []imagerequest.Size{ {Width: 25, Height: 25}, {Width: 50, Height: 50}, {Width: 75, Height: 75}, @@ -705,13 +730,17 @@ func TestService_Get_DifferentSizes(t *testing.T) { responses := make([][]byte, 0, len(sizes)) for _, size := range sizes { - req := &ImageRequest{ - SourceHost: fixtures.GoodHost, - SourcePath: testPathPhoto, - Size: size, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: fixtures.GoodHost, + Path: testPathPhoto, + }, + Transform: imagerequest.Transform{ + Size: size, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } resp, err := svc.Get(ctx, req) @@ -745,13 +774,17 @@ func TestService_ValidateRequest_NoSigningKey(t *testing.T) { // Service with no signing key - all non-allowlisted requests should fail svc, fixtures := SetupTestService(t, WithNoAllowlist()) - req := &ImageRequest{ - SourceHost: fixtures.OtherHost, - SourcePath: testPathUpload, - Size: Size{Width: 50, Height: 50}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: fixtures.OtherHost, + Path: testPathUpload, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } err := svc.ValidateRequest(req) @@ -770,13 +803,17 @@ func TestService_Get_ContextCancellation(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() // Cancel immediately - req := &ImageRequest{ - SourceHost: fixtures.GoodHost, - SourcePath: testPathPhoto, - Size: Size{Width: 50, Height: 50}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: fixtures.GoodHost, + Path: testPathPhoto, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } _, err := svc.Get(ctx, req) @@ -791,13 +828,17 @@ func TestService_Get_ReturnsETag(t *testing.T) { svc, fixtures := SetupTestService(t) ctx := context.Background() - req := &ImageRequest{ - SourceHost: fixtures.GoodHost, - SourcePath: testPathPhoto, - Size: Size{Width: 50, Height: 50}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: fixtures.GoodHost, + Path: testPathPhoto, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } resp, err := svc.Get(ctx, req) @@ -824,13 +865,17 @@ func TestService_Get_ETagConsistency(t *testing.T) { svc, fixtures := SetupTestService(t) ctx := context.Background() - req := &ImageRequest{ - SourceHost: fixtures.GoodHost, - SourcePath: testPathPhoto, - Size: Size{Width: 50, Height: 50}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: fixtures.GoodHost, + Path: testPathPhoto, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } // First request @@ -866,22 +911,30 @@ func TestService_Get_DifferentETagsForDifferentContent(t *testing.T) { ctx := context.Background() // Request same image at different sizes - should get different ETags - req1 := &ImageRequest{ - SourceHost: fixtures.GoodHost, - SourcePath: testPathPhoto, - Size: Size{Width: 25, Height: 25}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + req1 := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: fixtures.GoodHost, + Path: testPathPhoto, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 25, Height: 25}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } - req2 := &ImageRequest{ - SourceHost: fixtures.GoodHost, - SourcePath: testPathPhoto, - Size: Size{Width: 50, Height: 50}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + req2 := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: fixtures.GoodHost, + Path: testPathPhoto, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } resp1, err := svc.Get(ctx, req1) diff --git a/internal/imgcache/stats_internal_test.go b/internal/imgcache/stats_internal_test.go index 2fe9dbb..0ff7dae 100644 --- a/internal/imgcache/stats_internal_test.go +++ b/internal/imgcache/stats_internal_test.go @@ -15,6 +15,7 @@ import ( "sneak.berlin/go/pixa/internal/database" "sneak.berlin/go/pixa/internal/httpfetcher" + "sneak.berlin/go/pixa/internal/imagerequest" ) func setupStatsTestDB(t *testing.T) *sql.DB { @@ -319,13 +320,17 @@ func TestService_Get_CountsStats(t *testing.T) { } for _, step := range steps { - resp, err := svc.Get(t.Context(), &ImageRequest{ - SourceHost: fixtures.GoodHost, - SourcePath: step.path, - Size: Size{Width: step.size, Height: step.size}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + resp, err := svc.Get(t.Context(), &imagerequest.Request{ + Source: imagerequest.Source{ + Host: fixtures.GoodHost, + Path: step.path, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: step.size, Height: step.size}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, }) if (err != nil) != step.wantErr { t.Fatalf("%s: Get() error = %v, want error %t", step.name, err, step.wantErr) @@ -419,13 +424,17 @@ func TestService_Get_CountsInterruptedMisses(t *testing.T) { svc.fetcher = upstream - resp, err := svc.Get(ctx, &ImageRequest{ - SourceHost: fixtures.GoodHost, - SourcePath: testPathPhoto, - Size: Size{Width: 50, Height: 50}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + resp, err := svc.Get(ctx, &imagerequest.Request{ + Source: imagerequest.Source{ + Host: fixtures.GoodHost, + Path: testPathPhoto, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, }) t.Logf("Get() error = %v", err) @@ -453,13 +462,17 @@ func TestService_Get_CountsHitAfterRequestEnds(t *testing.T) { svc, fixtures := SetupTestService(t) - req := &ImageRequest{ - SourceHost: fixtures.GoodHost, - SourcePath: testPathPhoto, - Size: Size{Width: 50, Height: 50}, - Format: FormatJPEG, - Quality: 85, - FitMode: FitCover, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: fixtures.GoodHost, + Path: testPathPhoto, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 50, Height: 50}, + Format: imagerequest.FormatJPEG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, } // A first request caches the variant. diff --git a/internal/imgcache/storage.go b/internal/imgcache/storage.go index de55df4..a243fab 100644 --- a/internal/imgcache/storage.go +++ b/internal/imgcache/storage.go @@ -10,8 +10,9 @@ import ( "log/slog" "os" "path/filepath" - "strings" "time" + + "sneak.berlin/go/pixa/internal/imagerequest" ) // Storage constants. @@ -32,9 +33,6 @@ var ( // ContentHash is a SHA256 hash of file content (hex-encoded). type ContentHash string -// VariantKey is a SHA256 hash identifying a specific image variant (hex-encoded). -type VariantKey string - // PathHash is a SHA256 hash of a URL path (hex-encoded). type PathHash string @@ -403,34 +401,6 @@ func HashPath(path string) PathHash { return PathHash(hex.EncodeToString(h[:])) } -// CacheKey generates a unique key for a request variant. -// Format: sha256(host:path:query:width:height:format:quality:fit_mode) -// The path and query go through escapePercentAndColon, so that a ":" in -// either cannot be taken for the one between them. -func CacheKey(req *ImageRequest) VariantKey { - data := fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%s", - req.SourceHost, - escapePercentAndColon(req.SourcePath), - escapePercentAndColon(req.SourceQuery), - req.Size.Width, - req.Size.Height, - req.Format, - req.Quality, - req.FitMode, - ) - h := sha256.Sum256([]byte(data)) - - return VariantKey(hex.EncodeToString(h[:])) -} - -// escapePercentAndColon returns s with each "%" written as "%25" and each -// ":" as "%3A", so that the result holds no ":". -func escapePercentAndColon(s string) string { - s = strings.ReplaceAll(s, "%", "%25") - - return strings.ReplaceAll(s, ":", "%3A") -} - // VariantStorage handles key-based file storage for processed image variants. // Files are stored at: /// // Metadata is stored at: ///.meta @@ -464,7 +434,7 @@ func NewVariantStorage(baseDir string, log *slog.Logger) (*VariantStorage, error // with writeFileSynced. The .meta file is written first, so that a crash // cannot leave the content in place without it. func (s *VariantStorage) Store( - key VariantKey, r io.Reader, contentType string, + key imagerequest.VariantKey, r io.Reader, contentType string, ) (int64, error) { data, err := io.ReadAll(r) if err != nil { @@ -508,7 +478,7 @@ func (s *VariantStorage) Store( } // Load returns a reader for the content at the given key. -func (s *VariantStorage) Load(key VariantKey) (io.ReadCloser, error) { +func (s *VariantStorage) Load(key imagerequest.VariantKey) (io.ReadCloser, error) { path := s.keyToPath(key) f, err := os.Open(path) //nolint:gosec // path derived from cache key @@ -525,7 +495,9 @@ func (s *VariantStorage) Load(key VariantKey) (io.ReadCloser, error) { // LoadWithSize returns a reader and file size for the content at the // given key. -func (s *VariantStorage) LoadWithSize(key VariantKey) (io.ReadCloser, int64, error) { +func (s *VariantStorage) LoadWithSize( + key imagerequest.VariantKey, +) (io.ReadCloser, int64, error) { path := s.keyToPath(key) f, err := os.Open(path) //nolint:gosec // path derived from cache key @@ -551,7 +523,7 @@ func (s *VariantStorage) LoadWithSize(key VariantKey) (io.ReadCloser, int64, err // the given key. The content type is read from the .meta file, and is // empty when that file is missing or unreadable. func (s *VariantStorage) LoadWithMeta( - key VariantKey, + key imagerequest.VariantKey, ) (io.ReadCloser, int64, string, error) { f, size, err := s.LoadWithSize(key) if err != nil { @@ -574,7 +546,7 @@ func (s *VariantStorage) LoadWithMeta( } // Exists checks if content exists at the given key. -func (s *VariantStorage) Exists(key VariantKey) bool { +func (s *VariantStorage) Exists(key imagerequest.VariantKey) bool { path := s.keyToPath(key) _, err := os.Stat(path) @@ -583,7 +555,7 @@ func (s *VariantStorage) Exists(key VariantKey) bool { // Delete removes the content at the given key together with its .meta // sidecar file. A missing file is not an error. -func (s *VariantStorage) Delete(key VariantKey) error { +func (s *VariantStorage) Delete(key imagerequest.VariantKey) error { path := s.keyToPath(key) err := os.Remove(path) @@ -602,7 +574,7 @@ func (s *VariantStorage) Delete(key VariantKey) error { } // keyToPath converts a key to a file path: /// -func (s *VariantStorage) keyToPath(key VariantKey) string { +func (s *VariantStorage) keyToPath(key imagerequest.VariantKey) string { k := string(key) if len(k) < MinHashLength { return filepath.Join(s.baseDir, k) diff --git a/internal/imgcache/storage_internal_test.go b/internal/imgcache/storage_internal_test.go index 1d3996d..0819b9f 100644 --- a/internal/imgcache/storage_internal_test.go +++ b/internal/imgcache/storage_internal_test.go @@ -9,6 +9,8 @@ import ( "path/filepath" "strings" "testing" + + "sneak.berlin/go/pixa/internal/imagerequest" ) func TestContentStorage_StoreAndLoad(t *testing.T) { @@ -322,147 +324,6 @@ func TestHashPath(t *testing.T) { } } -func TestCacheKey(t *testing.T) { - t.Parallel() - - req1 := &ImageRequest{ - SourceHost: testHostCDN, - SourcePath: testPathCat, - SourceQuery: "", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, - Quality: 85, - FitMode: FitCover, - } - - req2 := &ImageRequest{ - SourceHost: testHostCDN, - SourcePath: testPathCat, - SourceQuery: "", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, - Quality: 85, - FitMode: FitCover, - } - - // Same request should produce same key - key1 := CacheKey(req1) - key2 := CacheKey(req2) - - if key1 != key2 { - t.Errorf("CacheKey() not deterministic: %s vs %s", key1, key2) - } - - // Key should be 64 hex chars - if len(string(key1)) != 64 { - t.Errorf("CacheKey() length = %d, want 64", len(string(key1))) - } - - // Different size should produce different key - req3 := &ImageRequest{ - SourceHost: testHostCDN, - SourcePath: testPathCat, - SourceQuery: "", - Size: Size{Width: 400, Height: 300}, // Different size - Format: FormatWebP, - Quality: 85, - FitMode: FitCover, - } - - key3 := CacheKey(req3) - if key1 == key3 { - t.Error("CacheKey() produced same key for different sizes") - } - - // Different format should produce different key - req4 := &ImageRequest{ - SourceHost: testHostCDN, - SourcePath: testPathCat, - SourceQuery: "", - Size: Size{Width: 800, Height: 600}, - Format: FormatPNG, // Different format - Quality: 85, - FitMode: FitCover, - } - - key4 := CacheKey(req4) - if key1 == key4 { - t.Error("CacheKey() produced same key for different formats") - } - - // Different quality should produce different key - req5 := &ImageRequest{ - SourceHost: testHostCDN, - SourcePath: testPathCat, - SourceQuery: "", - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, - Quality: 50, // Different quality - FitMode: FitCover, - } - - key5 := CacheKey(req5) - if key1 == key5 { - t.Error("CacheKey() produced same key for different quality") - } -} - -// TestCacheKey_ColonOrPercentInPathOrQuery checks that two different source -// URLs whose path or query holds ":" or "%" get different keys. -func TestCacheKey_ColonOrPercentInPathOrQuery(t *testing.T) { - t.Parallel() - - cases := []struct { - name string - path1, query1 string - path2, query2 string - }{ - { - name: "colon moved from path to query", - path1: "/a:b", query1: "", - path2: "/a", query2: "b:", - }, - { - name: "colon in path and its percent-encoding", - path1: "/a:b", query1: "", - path2: "/a%3Ab", query2: "", - }, - } - - for _, tt := range cases { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - key1 := CacheKey(&ImageRequest{ - SourceHost: testHostCDN, - SourcePath: tt.path1, - SourceQuery: tt.query1, - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, - Quality: 85, - FitMode: FitCover, - }) - key2 := CacheKey(&ImageRequest{ - SourceHost: testHostCDN, - SourcePath: tt.path2, - SourceQuery: tt.query2, - Size: Size{Width: 800, Height: 600}, - Format: FormatWebP, - Quality: 85, - FitMode: FitCover, - }) - - t.Logf("CacheKey() of path %q query %q = %s", tt.path1, tt.query1, key1) - t.Logf("CacheKey() of path %q query %q = %s", tt.path2, tt.query2, key2) - - if key1 == key2 { - t.Errorf("CacheKey() gave path %q query %q and path %q query %q "+ - "the same key", tt.path1, tt.query1, tt.path2, tt.query2) - } - }) - } -} - // TestVariantStorage_StoreLogsFailedMetaWrite verifies that a .meta write // that fails is logged at warn and the store still succeeds. func TestVariantStorage_StoreLogsFailedMetaWrite(t *testing.T) { @@ -476,7 +337,9 @@ func TestVariantStorage_StoreLogsFailedMetaWrite(t *testing.T) { t.Fatalf("NewVariantStorage() error = %v", err) } - key := CacheKey(&ImageRequest{SourceHost: testHostCDN, SourcePath: testPathCat}) + key := imagerequest.CacheKey(&imagerequest.Request{ + Source: imagerequest.Source{Host: testHostCDN, Path: testPathCat}, + }) // A directory where the .meta file goes makes the .meta write fail. err = os.MkdirAll(storage.keyToPath(key)+".meta", StorageDirPerm) @@ -497,7 +360,7 @@ func TestVariantStorage_StoreLogsFailedMetaWrite(t *testing.T) { // storeTestVariant stores one variant, with its .meta file, in a new // VariantStorage and returns the storage and the variant's key. -func storeTestVariant(t *testing.T) (*VariantStorage, VariantKey) { +func storeTestVariant(t *testing.T) (*VariantStorage, imagerequest.VariantKey) { t.Helper() storage, err := NewVariantStorage(t.TempDir(), slog.New(slog.DiscardHandler)) @@ -505,7 +368,9 @@ func storeTestVariant(t *testing.T) (*VariantStorage, VariantKey) { t.Fatalf("NewVariantStorage() error = %v", err) } - key := CacheKey(&ImageRequest{SourceHost: testHostCDN, SourcePath: testPathCat}) + key := imagerequest.CacheKey(&imagerequest.Request{ + Source: imagerequest.Source{Host: testHostCDN, Path: testPathCat}, + }) _, err = storage.Store(key, bytes.NewReader([]byte("variant data")), "image/webp") if err != nil { diff --git a/internal/server/computed_side_internal_test.go b/internal/server/computed_side_internal_test.go index ded083f..ad81740 100644 --- a/internal/server/computed_side_internal_test.go +++ b/internal/server/computed_side_internal_test.go @@ -9,7 +9,7 @@ import ( "testing" "sneak.berlin/go/pixa/internal/encurl" - "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/imagerequest" ) // TestImageProxyRefusesComputedSideAboveMaximum requests sizes of a 3000x600 @@ -70,9 +70,12 @@ func TestImageProxyRefusesComputedSideAboveMaximum(t *testing.T) { } for _, tt := range tests { - token, err := generator.Generate(&encurl.Payload{ - SourceHost: upstreamHost, SourcePath: tt.path, - Width: tt.width, Height: tt.height, Format: imgcache.FormatJPEG, + token, err := generator.Generate(&imagerequest.Request{ + Source: imagerequest.Source{Host: upstreamHost, Path: tt.path}, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: tt.width, Height: tt.height}, + Format: imagerequest.FormatJPEG, + }, }) if err != nil { t.Fatalf("encurl Generate: %v", err) diff --git a/internal/server/fit_outside_internal_test.go b/internal/server/fit_outside_internal_test.go index f0ada85..cbc6a80 100644 --- a/internal/server/fit_outside_internal_test.go +++ b/internal/server/fit_outside_internal_test.go @@ -7,7 +7,7 @@ import ( "testing" "sneak.berlin/go/pixa/internal/encurl" - "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/imagerequest" ) // TestImageProxyRefusesFitOutsideAboveMaximum requests sizes with fit=outside @@ -59,10 +59,13 @@ func TestImageProxyRefusesFitOutsideAboveMaximum(t *testing.T) { } for _, tt := range tests { - token, err := generator.Generate(&encurl.Payload{ - SourceHost: upstreamHost, SourcePath: tt.path, - Width: tt.width, Height: tt.height, Format: imgcache.FormatJPEG, - FitMode: imgcache.FitOutside, + token, err := generator.Generate(&imagerequest.Request{ + Source: imagerequest.Source{Host: upstreamHost, Path: tt.path}, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: tt.width, Height: tt.height}, + Format: imagerequest.FormatJPEG, + FitMode: imagerequest.FitOutside, + }, }) if err != nil { t.Fatalf("encurl Generate: %v", err) diff --git a/internal/server/source_url_internal_test.go b/internal/server/source_url_internal_test.go index 27b963b..05e8ef4 100644 --- a/internal/server/source_url_internal_test.go +++ b/internal/server/source_url_internal_test.go @@ -14,6 +14,7 @@ import ( "time" "sneak.berlin/go/pixa/internal/encurl" + "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imgcache" "sneak.berlin/go/pixa/internal/session" "sneak.berlin/go/pixa/internal/signature" @@ -513,12 +514,16 @@ func signedImageURL(t *testing.T, sourceHost string) string { t.Helper() path, sig, exp := signature.New(testSigningKey).GenerateSignedURL( - &signature.Request{ - SourceHost: sourceHost, - SourcePath: ipv6SourcePath, - Format: string(imgcache.FormatPNG), - Quality: 85, - FitMode: string(imgcache.FitCover), + &imagerequest.Request{ + Source: imagerequest.Source{ + Host: sourceHost, + Path: ipv6SourcePath, + }, + Transform: imagerequest.Transform{ + Format: imagerequest.FormatPNG, + Quality: 85, + FitMode: imagerequest.FitCover, + }, }, time.Hour) return path + "?sig=" + sig + "&exp=" + strconv.FormatInt(exp, 10) @@ -534,9 +539,9 @@ func encryptedImageURL(t *testing.T, sourceHost string) string { t.Fatalf("encurl.NewGenerator: %v", err) } - token, err := generator.Generate(&encurl.Payload{ - SourceHost: sourceHost, SourcePath: ipv6SourcePath, - Format: imgcache.FormatPNG, + token, err := generator.Generate(&imagerequest.Request{ + Source: imagerequest.Source{Host: sourceHost, Path: ipv6SourcePath}, + Transform: imagerequest.Transform{Format: imagerequest.FormatPNG}, }) if err != nil { t.Fatalf("encurl Generate: %v", err) diff --git a/internal/server/svg_as_avif_internal_test.go b/internal/server/svg_as_avif_internal_test.go index cd05947..af75b24 100644 --- a/internal/server/svg_as_avif_internal_test.go +++ b/internal/server/svg_as_avif_internal_test.go @@ -8,7 +8,7 @@ import ( "testing" "sneak.berlin/go/pixa/internal/encurl" - "sneak.berlin/go/pixa/internal/imgcache" + "sneak.berlin/go/pixa/internal/imagerequest" ) // svgServedAsAVIF is an SVG whose first bytes pass the first-bytes check for @@ -38,10 +38,13 @@ func TestImageProxyRefusesSVGServedAsAVIF(t *testing.T) { t.Fatalf("encurl.NewGenerator: %v", err) } - encryptedURL := func(format imgcache.ImageFormat) string { - token, err := generator.Generate(&encurl.Payload{ - SourceHost: upstreamHost, SourcePath: "/drawing.avif", - Width: 32, Height: 24, Format: format, + encryptedURL := func(format imagerequest.Format) string { + token, err := generator.Generate(&imagerequest.Request{ + Source: imagerequest.Source{Host: upstreamHost, Path: "/drawing.avif"}, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 32, Height: 24}, + Format: format, + }, }) if err != nil { t.Fatalf("encurl Generate: %v", err) @@ -53,8 +56,8 @@ func TestImageProxyRefusesSVGServedAsAVIF(t *testing.T) { for _, target := range []string{ "/v1/image/" + upstreamHost + "/drawing.avif/32x24.png", "/v1/image/" + upstreamHost + "/drawing.avif/64x48.orig", - encryptedURL(imgcache.FormatPNG), - encryptedURL(imgcache.FormatOriginal), + encryptedURL(imagerequest.FormatPNG), + encryptedURL(imagerequest.FormatOriginal), } { rec := httptest.NewRecorder() s.ServeHTTP(rec, httptest.NewRequestWithContext( diff --git a/internal/signature/escaping_test.go b/internal/signature/escaping_test.go index 550805c..032df24 100644 --- a/internal/signature/escaping_test.go +++ b/internal/signature/escaping_test.go @@ -5,6 +5,7 @@ import ( "testing" "time" + "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/signature" ) @@ -16,32 +17,35 @@ func TestSigner_Verify_ColonCannotMovePathQueryBoundary(t *testing.T) { signer := signature.New(goldenSigningKey) - signed := &signature.Request{ - SourceHost: testHost, - SourcePath: "/a:b", - Width: 800, - Height: 600, - Format: testFormatWebP, - Quality: 85, - FitMode: testFitCover, - Expires: time.Now().Add(1 * time.Hour), + signed := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHost, + Path: "/a:b", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: testFormatWebP, + Quality: 85, + FitMode: testFitCover, + }, + Expires: time.Now().Add(1 * time.Hour), } signed.Signature = signer.Sign(signed) replayed := *signed - replayed.SourcePath = "/a" - replayed.SourceQuery = "b:" + replayed.Source.Path = "/a" + replayed.Source.Query = "b:" t.Logf("signature for path %q query %q: %s", - signed.SourcePath, signed.SourceQuery, signed.Signature) + signed.Source.Path, signed.Source.Query, signed.Signature) t.Logf("signature for path %q query %q: %s", - replayed.SourcePath, replayed.SourceQuery, signer.Sign(&replayed)) + replayed.Source.Path, replayed.Source.Query, signer.Sign(&replayed)) err := signer.Verify(&replayed) if !errors.Is(err, signature.ErrInvalid) { t.Errorf("Verify() of path %q query %q with the signature for "+ "path %q = %v, want %v", - replayed.SourcePath, replayed.SourceQuery, signed.SourcePath, + replayed.Source.Path, replayed.Source.Query, signed.Source.Path, err, signature.ErrInvalid) } } @@ -90,16 +94,19 @@ func TestSigner_EscapedGoldenVectors(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - req := &signature.Request{ - SourceHost: testHost, - SourcePath: tt.path, - SourceQuery: tt.query, - Width: 800, - Height: 600, - Format: testFormatWebP, - Quality: 85, - FitMode: testFitCover, - Expires: time.Unix(goldenExpiresUnix, 0), + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHost, + Path: tt.path, + Query: tt.query, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: testFormatWebP, + Quality: 85, + FitMode: testFitCover, + }, + Expires: time.Unix(goldenExpiresUnix, 0), } gotSignature := signer.Sign(req) diff --git a/internal/signature/golden_test.go b/internal/signature/golden_test.go index 126b05c..bb8eda2 100644 --- a/internal/signature/golden_test.go +++ b/internal/signature/golden_test.go @@ -4,6 +4,7 @@ import ( "testing" "time" + "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/signature" ) @@ -16,7 +17,7 @@ const goldenSigningKey = "golden-test-key" type goldenVector struct { name string - req signature.Request + req imagerequest.Request // wantSignature is the exact base64url (RFC 4648 URL-safe, // padded) HMAC-SHA256 signature for the request with Expires // set to goldenExpiresUnix. @@ -34,15 +35,18 @@ func goldenVectors() []goldenVector { return []goldenVector{ { name: "resized without query", - req: signature.Request{ - SourceHost: testHost, - SourcePath: testPath, - SourceQuery: "", - Width: 800, - Height: 600, - Format: testFormatWebP, - Quality: 85, - FitMode: testFitCover, + req: imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHost, + Path: testPath, + Query: "", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: testFormatWebP, + Quality: 85, + FitMode: testFitCover, + }, }, // Signed data: // "cdn.example.com:/photos/cat.jpg::800:600:webp:1704067200:85:cover" @@ -51,15 +55,18 @@ func goldenVectors() []goldenVector { }, { name: "resized with query string", - req: signature.Request{ - SourceHost: testHost, - SourcePath: testPath, - SourceQuery: "token=abc&v=2", - Width: 800, - Height: 600, - Format: testFormatWebP, - Quality: 85, - FitMode: testFitCover, + req: imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHost, + Path: testPath, + Query: "token=abc&v=2", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: testFormatWebP, + Quality: 85, + FitMode: testFitCover, + }, }, // Signed data: // "cdn.example.com:/photos/cat.jpg:token=abc&v=2:800:600:webp:1704067200:85:cover" @@ -69,15 +76,18 @@ func goldenVectors() []goldenVector { }, { name: "original size without query", - req: signature.Request{ - SourceHost: testHost, - SourcePath: testPath, - SourceQuery: "", - Width: 0, - Height: 0, - Format: testFormatPNG, - Quality: 85, - FitMode: testFitCover, + req: imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHost, + Path: testPath, + Query: "", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 0, Height: 0}, + Format: testFormatPNG, + Quality: 85, + FitMode: testFitCover, + }, }, // Signed data: // "cdn.example.com:/photos/cat.jpg::0:0:png:1704067200:85:cover" @@ -86,15 +96,18 @@ func goldenVectors() []goldenVector { }, { name: "non-default quality and fit", - req: signature.Request{ - SourceHost: testHost, - SourcePath: testPath, - SourceQuery: "", - Width: 800, - Height: 600, - Format: testFormatWebP, - Quality: 40, - FitMode: testFitContain, + req: imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHost, + Path: testPath, + Query: "", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: testFormatWebP, + Quality: 40, + FitMode: testFitContain, + }, }, // Signed data: // "cdn.example.com:/photos/cat.jpg::800:600:webp:1704067200:40:contain" diff --git a/internal/signature/quality_fit_test.go b/internal/signature/quality_fit_test.go index bebbec4..8821b7b 100644 --- a/internal/signature/quality_fit_test.go +++ b/internal/signature/quality_fit_test.go @@ -5,22 +5,26 @@ import ( "testing" "time" + "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/signature" ) // signedQualityFitRequest returns a request signed for quality 85 and fit // mode "cover", the effective defaults the handler applies before // verification. -func signedQualityFitRequest(signer *signature.Signer) *signature.Request { - req := &signature.Request{ - SourceHost: testHost, - SourcePath: testPath, - Width: 800, - Height: 600, - Format: testFormatWebP, - Quality: 85, - FitMode: testFitCover, - Expires: time.Now().Add(1 * time.Hour), +func signedQualityFitRequest(signer *signature.Signer) *imagerequest.Request { + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHost, + Path: testPath, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: testFormatWebP, + Quality: 85, + FitMode: testFitCover, + }, + Expires: time.Now().Add(1 * time.Hour), } req.Signature = signer.Sign(req) @@ -40,15 +44,15 @@ func TestSigner_Verify_QualityAndFitAreSigned(t *testing.T) { cases := []struct { name string - tamper func(r *signature.Request) + tamper func(r *imagerequest.Request) }{ { name: "replayed with different quality", - tamper: func(r *signature.Request) { r.Quality = 40 }, + tamper: func(r *imagerequest.Request) { r.Transform.Quality = 40 }, }, { name: "replayed with different fit mode", - tamper: func(r *signature.Request) { r.FitMode = testFitContain }, + tamper: func(r *imagerequest.Request) { r.Transform.FitMode = testFitContain }, }, } diff --git a/internal/signature/signature.go b/internal/signature/signature.go index a507240..beff9b4 100644 --- a/internal/signature/signature.go +++ b/internal/signature/signature.go @@ -12,6 +12,8 @@ import ( "strconv" "strings" "time" + + "sneak.berlin/go/pixa/internal/imagerequest" ) // Signature errors. @@ -22,37 +24,6 @@ var ( ErrMissingExpiration = errors.New("signature expiration is required") ) -// Request carries the components an image request signature covers. It is a -// standalone type so that this package does not depend on imgcache, keeping -// the import edge one-way (imgcache depends on signature, never the reverse). -type Request struct { - // SourceHost is the origin host (e.g. "cdn.example.com"). - SourceHost string - // SourcePath is the path on the origin (e.g. "/photos/cat.jpg"). - SourcePath string - // SourceQuery is the optional query string for the origin URL. - SourceQuery string - // Width is the requested output width in pixels. - Width int - // Height is the requested output height in pixels. - Height int - // Format is the requested output format (e.g. "webp"). - Format string - // Quality is the requested output quality (1-100) for lossy formats. - // It is the effective value the request resolves to: callers pass the - // default quality when the request omits the parameter, so an omitted - // quality signs identically to that same value stated explicitly. - Quality int - // FitMode is how the image is fit into the requested dimensions - // (e.g. "cover"). Like Quality it is the effective value: callers pass - // the default fit mode when the request omits the parameter. - FitMode string - // Signature is the HMAC signature to verify. - Signature string - // Expires is the signature expiration timestamp. - Expires time.Time -} - // Signer handles HMAC-SHA256 signature generation and verification. type Signer struct { secretKey []byte @@ -67,8 +38,11 @@ func New(secretKey string) *Signer { // Sign generates an HMAC-SHA256 signature for the given request. // The signature covers: host + path + query + width + height + format + -// expiration + quality + fit. -func (s *Signer) Sign(req *Request) string { +// expiration + quality + fit. The quality and fit mode are the effective +// values the request resolves to: callers pass the defaults when the request +// omits those parameters, so an omitted quality or fit mode signs +// identically to that same value stated explicitly. +func (s *Signer) Sign(req *imagerequest.Request) string { data := s.buildSignatureData(req) mac := hmac.New(sha256.New, s.secretKey) mac.Write([]byte(data)) @@ -84,7 +58,7 @@ func (s *Signer) Sign(req *Request) string { // No suffix matching, wildcard matching, or partial matching is supported. // A signature for "cdn.example.com" will NOT verify for "example.com" or // "other.cdn.example.com", and vice versa. -func (s *Signer) Verify(req *Request) error { +func (s *Signer) Verify(req *imagerequest.Request) error { // Check expiration first if req.Expires.IsZero() { return ErrMissingExpiration @@ -108,7 +82,7 @@ func (s *Signer) Verify(req *Request) error { // GenerateSignedURL creates a complete URL with signature and expiration. // Returns the path portion that should be appended to the base URL. func (s *Signer) GenerateSignedURL( - req *Request, ttl time.Duration, + req *imagerequest.Request, ttl time.Duration, ) (string, string, int64) { // Set expiration req.Expires = time.Now().Add(ttl) @@ -120,10 +94,11 @@ func (s *Signer) GenerateSignedURL( // Build the size component var sizeStr string - if req.Width == 0 && req.Height == 0 { + if req.Transform.Size.Width == 0 && req.Transform.Size.Height == 0 { sizeStr = "orig" } else { - sizeStr = fmt.Sprintf("%dx%d", req.Width, req.Height) + sizeStr = fmt.Sprintf("%dx%d", + req.Transform.Size.Width, req.Transform.Size.Height) } // Build the path. A source query follows the source path after a "?". @@ -131,13 +106,13 @@ func (s *Signer) GenerateSignedURL( // written in Go's own percent-encoding of a URL path, which writes "?" // as %3F and "%" as %25, and leaves "/", ";", ",", "=" and "&" as they // are. - sourcePathAndQuery := req.SourcePath - if req.SourceQuery != "" { - sourcePathAndQuery += "?" + req.SourceQuery + sourcePathAndQuery := req.Source.Path + if req.Source.Query != "" { + sourcePathAndQuery += "?" + req.Source.Query } unescapedPath := fmt.Sprintf("/v1/image/%s%s/%s.%s", - req.SourceHost, sourcePathAndQuery, sizeStr, req.Format) + req.Source.Host, sourcePathAndQuery, sizeStr, req.Transform.Format) return (&url.URL{Path: unescapedPath}).EscapedPath(), sig, exp } @@ -150,17 +125,17 @@ func (s *Signer) GenerateSignedURL( // expansion is performed. Quality and fit are the effective transform // values, so replaying a signed URL with a different quality or fit mode // fails verification. -func (s *Signer) buildSignatureData(req *Request) string { +func (s *Signer) buildSignatureData(req *imagerequest.Request) string { return fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%d:%s", - req.SourceHost, - escapePercentAndColon(req.SourcePath), - escapePercentAndColon(req.SourceQuery), - req.Width, - req.Height, - req.Format, + req.Source.Host, + escapePercentAndColon(req.Source.Path), + escapePercentAndColon(req.Source.Query), + req.Transform.Size.Width, + req.Transform.Size.Height, + req.Transform.Format, req.Expires.Unix(), - req.Quality, - req.FitMode, + req.Transform.Quality, + req.Transform.FitMode, ) } diff --git a/internal/signature/signature_test.go b/internal/signature/signature_test.go index 559d19c..119fb14 100644 --- a/internal/signature/signature_test.go +++ b/internal/signature/signature_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/signature" ) @@ -26,14 +27,17 @@ func TestSigner_Sign(t *testing.T) { signer := signature.New("test-secret-key") - req := &signature.Request{ - SourceHost: testHost, - SourcePath: testPath, - SourceQuery: "", - Width: 800, - Height: 600, - Format: testFormatWebP, - Expires: time.Unix(1704067200, 0), // Fixed timestamp for reproducibility + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHost, + Path: testPath, + Query: "", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: testFormatWebP, + }, + Expires: time.Unix(1704067200, 0), // Fixed timestamp for reproducibility } sig1 := signer.Sign(req) @@ -51,14 +55,17 @@ func TestSigner_Sign(t *testing.T) { } // Different input should produce different signature - req2 := &signature.Request{ - SourceHost: testHost, - SourcePath: "/photos/dog.jpg", // Different path - SourceQuery: "", - Width: 800, - Height: 600, - Format: testFormatWebP, - Expires: time.Unix(1704067200, 0), + req2 := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHost, + Path: "/photos/dog.jpg", // Different path + Query: "", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: testFormatWebP, + }, + Expires: time.Unix(1704067200, 0), } sig3 := signer.Sign(req2) @@ -69,20 +76,23 @@ func TestSigner_Sign(t *testing.T) { // validVerifyRequest returns a fully-populated request that verifies // successfully once signed. -func validVerifyRequest() *signature.Request { - return &signature.Request{ - SourceHost: testHost, - SourcePath: testPath, - Width: 800, - Height: 600, - Format: testFormatWebP, - Expires: time.Now().Add(1 * time.Hour), +func validVerifyRequest() *imagerequest.Request { + return &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHost, + Path: testPath, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: testFormatWebP, + }, + Expires: time.Now().Add(1 * time.Hour), } } type verifyCase struct { name string - setup func() *signature.Request + setup func() *imagerequest.Request wantErr error } @@ -90,7 +100,7 @@ func verifyCases(signer *signature.Signer) []verifyCase { return []verifyCase{ { name: "valid signature", - setup: func() *signature.Request { + setup: func() *imagerequest.Request { req := validVerifyRequest() req.Signature = signer.Sign(req) @@ -100,7 +110,7 @@ func verifyCases(signer *signature.Signer) []verifyCase { }, { name: "expired signature", - setup: func() *signature.Request { + setup: func() *imagerequest.Request { req := validVerifyRequest() req.Expires = time.Now().Add(-1 * time.Hour) req.Signature = signer.Sign(req) @@ -111,7 +121,7 @@ func verifyCases(signer *signature.Signer) []verifyCase { }, { name: "invalid signature", - setup: func() *signature.Request { + setup: func() *imagerequest.Request { req := validVerifyRequest() req.Signature = "invalid-signature" @@ -121,7 +131,7 @@ func verifyCases(signer *signature.Signer) []verifyCase { }, { name: "missing expiration", - setup: func() *signature.Request { + setup: func() *imagerequest.Request { req := validVerifyRequest() req.Expires = time.Time{} req.Signature = "some-signature" @@ -132,10 +142,10 @@ func verifyCases(signer *signature.Signer) []verifyCase { }, { name: "tampered request", - setup: func() *signature.Request { + setup: func() *imagerequest.Request { req := validVerifyRequest() req.Signature = signer.Sign(req) - req.SourcePath = "/photos/secret.jpg" + req.Source.Path = "/photos/secret.jpg" return req }, @@ -173,7 +183,7 @@ func TestSigner_Verify(t *testing.T) { type tamperCase struct { name string - tamper func(r *signature.Request) + tamper func(r *imagerequest.Request) } // exactMatchTamperCases mutates one signed component per case; every @@ -182,59 +192,59 @@ func exactMatchTamperCases() []tamperCase { return []tamperCase{ { name: "parent domain does not match subdomain", - tamper: func(r *signature.Request) { r.SourceHost = "example.com" }, + tamper: func(r *imagerequest.Request) { r.Source.Host = "example.com" }, }, { name: "subdomain does not match parent domain", - tamper: func(r *signature.Request) { r.SourceHost = "images.cdn.example.com" }, + tamper: func(r *imagerequest.Request) { r.Source.Host = "images.cdn.example.com" }, }, { name: "sibling subdomain does not match", - tamper: func(r *signature.Request) { r.SourceHost = "images.example.com" }, + tamper: func(r *imagerequest.Request) { r.Source.Host = "images.example.com" }, }, { name: "host with suffix appended does not match", - tamper: func(r *signature.Request) { r.SourceHost = testHost + ".evil.com" }, + tamper: func(r *imagerequest.Request) { r.Source.Host = testHost + ".evil.com" }, }, { name: "host with prefix does not match", - tamper: func(r *signature.Request) { r.SourceHost = "evilcdn.example.com" }, + tamper: func(r *imagerequest.Request) { r.Source.Host = "evilcdn.example.com" }, }, { name: "different path does not match", - tamper: func(r *signature.Request) { r.SourcePath = "/photos/dog.jpg" }, + tamper: func(r *imagerequest.Request) { r.Source.Path = "/photos/dog.jpg" }, }, { name: "path suffix does not match", - tamper: func(r *signature.Request) { r.SourcePath = testPath + "/extra" }, + tamper: func(r *imagerequest.Request) { r.Source.Path = testPath + "/extra" }, }, { name: "path prefix does not match", - tamper: func(r *signature.Request) { r.SourcePath = "/other" + testPath }, + tamper: func(r *imagerequest.Request) { r.Source.Path = "/other" + testPath }, }, { name: "different query does not match", - tamper: func(r *signature.Request) { r.SourceQuery = "token=xyz" }, + tamper: func(r *imagerequest.Request) { r.Source.Query = "token=xyz" }, }, { name: "added query does not match empty query", - tamper: func(r *signature.Request) { r.SourceQuery = "extra=1" }, + tamper: func(r *imagerequest.Request) { r.Source.Query = "extra=1" }, }, { name: "removed query does not match", - tamper: func(r *signature.Request) { r.SourceQuery = "" }, + tamper: func(r *imagerequest.Request) { r.Source.Query = "" }, }, { name: "different width does not match", - tamper: func(r *signature.Request) { r.Width = 801 }, + tamper: func(r *imagerequest.Request) { r.Transform.Size.Width = 801 }, }, { name: "different height does not match", - tamper: func(r *signature.Request) { r.Height = 601 }, + tamper: func(r *imagerequest.Request) { r.Transform.Size.Height = 601 }, }, { name: "different format does not match", - tamper: func(r *signature.Request) { r.Format = testFormatPNG }, + tamper: func(r *imagerequest.Request) { r.Transform.Format = testFormatPNG }, }, } } @@ -248,15 +258,18 @@ func TestSigner_Verify_ExactMatchOnly(t *testing.T) { signer := signature.New("test-secret-key") // Base request that we'll sign, then tamper with individual fields. - baseReq := func() *signature.Request { - req := &signature.Request{ - SourceHost: testHost, - SourcePath: testPath, - SourceQuery: "token=abc", - Width: 800, - Height: 600, - Format: testFormatWebP, - Expires: time.Now().Add(1 * time.Hour), + baseReq := func() *imagerequest.Request { + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHost, + Path: testPath, + Query: "token=abc", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: testFormatWebP, + }, + Expires: time.Now().Add(1 * time.Hour), } req.Signature = signer.Sign(req) @@ -309,14 +322,17 @@ func TestSigner_Sign_ExactHostInData(t *testing.T) { sigs := make(map[string]string) for _, host := range hosts { - req := &signature.Request{ - SourceHost: host, - SourcePath: testPath, - SourceQuery: "", - Width: 800, - Height: 600, - Format: testFormatWebP, - Expires: time.Unix(1704067200, 0), + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: host, + Path: testPath, + Query: "", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: testFormatWebP, + }, + Expires: time.Unix(1704067200, 0), } sig := signer.Sign(req) @@ -334,13 +350,16 @@ func TestSigner_DifferentKeys(t *testing.T) { signer1 := signature.New("secret-key-1") signer2 := signature.New("secret-key-2") - req := &signature.Request{ - SourceHost: testHost, - SourcePath: testPath, - Width: 800, - Height: 600, - Format: testFormatWebP, - Expires: time.Now().Add(1 * time.Hour), + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHost, + Path: testPath, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: testFormatWebP, + }, + Expires: time.Now().Add(1 * time.Hour), } // Sign with key 1 @@ -364,13 +383,16 @@ func TestGenerateSignedURL(t *testing.T) { signer := signature.New("test-secret-key") - req := &signature.Request{ - SourceHost: testHost, - SourcePath: testPath, - SourceQuery: "", - Width: 800, - Height: 600, - Format: testFormatWebP, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHost, + Path: testPath, + Query: "", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: testFormatWebP, + }, } ttl := 1 * time.Hour @@ -405,12 +427,15 @@ func TestGenerateSignedURL_OrigSize(t *testing.T) { signer := signature.New("test-secret-key") - req := &signature.Request{ - SourceHost: testHost, - SourcePath: testPath, - Width: 0, // Original size - Height: 0, - Format: testFormatPNG, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHost, + Path: testPath, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 0, Height: 0}, // Original size + Format: testFormatPNG, + }, } path, _, _ := signer.GenerateSignedURL(req, time.Hour) @@ -426,13 +451,16 @@ func TestGenerateSignedURL_WithQueryString(t *testing.T) { signer := signature.New("test-secret-key-for-testing!") - req := &signature.Request{ - SourceHost: testHost, - SourcePath: testPath, - SourceQuery: "token=abc&v=2", - Width: 800, - Height: 600, - Format: testFormatWebP, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHost, + Path: testPath, + Query: "token=abc&v=2", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: testFormatWebP, + }, } path, _, _ := signer.GenerateSignedURL(req, time.Hour) @@ -460,12 +488,15 @@ func TestGenerateSignedURL_WithoutQueryString(t *testing.T) { signer := signature.New("test-secret-key-for-testing!") - req := &signature.Request{ - SourceHost: testHost, - SourcePath: testPath, - Width: 800, - Height: 600, - Format: testFormatWebP, + req := &imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHost, + Path: testPath, + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: testFormatWebP, + }, } path, _, _ := signer.GenerateSignedURL(req, time.Hour) @@ -485,16 +516,19 @@ func TestGenerateSignedURL_ReadmeQueryExample(t *testing.T) { signer := signature.New("example-signing-key-for-documentation") - req := signature.Request{ - SourceHost: testHost, - SourcePath: "/img", - SourceQuery: "url=https://origin.example.com/my%20cat.jpg", - Width: 800, - Height: 600, - Format: testFormatWebP, - Quality: 85, - FitMode: testFitCover, - Expires: time.Unix(1704067200, 0), + req := imagerequest.Request{ + Source: imagerequest.Source{ + Host: testHost, + Path: "/img", + Query: "url=https://origin.example.com/my%20cat.jpg", + }, + Transform: imagerequest.Transform{ + Size: imagerequest.Size{Width: 800, Height: 600}, + Format: testFormatWebP, + Quality: 85, + FitMode: testFitCover, + }, + Expires: time.Unix(1704067200, 0), } const wantSig = "37aFa9GSVCZM1kRY9SzMOk2fOg_M7Wt5gKNR4xM-FYA="