From d8760c40a1326f7c1ae4d9c7247915954ddf9d80 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 13 Oct 2026 09:44:44 +0200 Subject: [PATCH] Refuse a size whose computed side exceeds the maximum (closes #241) A side given as 0 is computed from the source's proportions, and nothing held it to 8192: 0x8192 of a 3000x600 source was resized to 40960x8192 and encoded. Process now refuses such a size with ErrOutputTooLarge once it knows the source's dimensions, before the resize and the encode, and both image routes answer it 400 with the JSON error body. It is a processing error, not a failed fetch, so it is never negative-cached. imgcache.MaxDimension is now imageprocessor.MaxOutputDimension, so the request limit and the output limit are one value. README.md says it under Routes and Encrypted URLs. Model: opus-5-5 --- README.md | 7 +- TODO.md | 9 ++ internal/handlers/image.go | 8 + internal/handlers/imageenc.go | 4 + .../computed_side_internal_test.go | 52 ++++++ internal/imageprocessor/imageprocessor.go | 43 +++-- internal/imgcache/urlparser.go | 7 +- .../server/computed_side_internal_test.go | 153 ++++++++++++++++++ 8 files changed, 268 insertions(+), 15 deletions(-) create mode 100644 internal/imageprocessor/computed_side_internal_test.go create mode 100644 internal/server/computed_side_internal_test.go diff --git a/README.md b/README.md index 49c74bb..9525dcc 100644 --- a/README.md +++ b/README.md @@ -263,6 +263,10 @@ once, is refused with 400. both optional (values under Signature Specification). Both are part of what is cached, so each value of either is a separate cached image. +A size with one side `0` has that side scaled to keep the source's proportions, +and is refused with 400 when that side would come out above 8192, the most +either side may be. + The source image may be JPEG, PNG, GIF, WebP, AVIF or JPEG XL. The upstream's `Content-Type` must name its format, and the image's first bytes must match it. An upstream answering with any other `Content-Type`, SVG included, is refused @@ -345,7 +349,8 @@ nor change what it asks for. (`POST /generate`). The format is JPEG XL unless another is chosen; a form sent with an empty format, or none, also makes a JPEG XL URL. Width and height both empty or `0` keep the original size; if only one of them is empty - or `0`, that side is scaled to keep the image's proportions. + or `0`, that side is scaled to keep the image's proportions. A URL whose + scaled side would come out above 8192 answers 400. 3. The page shows the URL, `https:///v1/e//img.`, and when it expires. `` is the host the page was opened on, and the URL starts with `http` instead while `debug` is on. The name after the token is ignored diff --git a/TODO.md b/TODO.md index 5a5cf5d..2ec0e84 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,15 @@ P2: security: per-origin rate limiting # Completed Steps +- 2026-10-13 a size with one side `0` no longer makes an output above 8192 on a + side (closes #241): that side is computed from the source's proportions, and + nothing held it to the 8192 a requested side is held to, so `0x8192` of a + 3000x600 source was resized to 40960x8192 and encoded. The image processor now + refuses such a size with `ErrOutputTooLarge` once it knows the source's + dimensions, before resizing, and both image routes answer it 400 with the JSON + error body. The refusal is not cached as a failed fetch, so the same source is + still served at other sizes. `imgcache.MaxDimension` is now the image + processor's `MaxOutputDimension`, so the two cannot differ. - 2026-10-13 the unused cache lifetime is removed (closes #69): `CacheConfig.CacheTTL`, which pixad set to `DefaultCacheTTL` (24 hours), was never read, so cached images never expired. The field and the constant are diff --git a/internal/handlers/image.go b/internal/handlers/image.go index 9978565..aefde8e 100644 --- a/internal/handlers/image.go +++ b/internal/handlers/image.go @@ -255,6 +255,14 @@ func (s *Handlers) respondImageError( return } + if errors.Is(err, imageprocessor.ErrOutputTooLarge) { + s.respondError(w, fmt.Sprintf("size too large for the source: the side "+ + "computed from its proportions is above %d", + imageprocessor.MaxOutputDimension), http.StatusBadRequest) + + return + } + s.respondError(w, "internal error", http.StatusInternalServerError) } diff --git a/internal/handlers/imageenc.go b/internal/handlers/imageenc.go index 816e83f..8da6d47 100644 --- a/internal/handlers/imageenc.go +++ b/internal/handlers/imageenc.go @@ -165,6 +165,10 @@ func (s *Handlers) handleImageError(w http.ResponseWriter, err error) { errors.Is(err, imageprocessor.ErrUnsupportedInputFormat): s.respondError(w, "unsupported source format", http.StatusUnsupportedMediaType) + case errors.Is(err, imageprocessor.ErrOutputTooLarge): + s.respondError(w, fmt.Sprintf("size too large for the source: the side "+ + "computed from its proportions is above %d", + imageprocessor.MaxOutputDimension), http.StatusBadRequest) default: s.log.Error("image request failed", "error", err) s.respondError(w, "internal error", http.StatusInternalServerError) diff --git a/internal/imageprocessor/computed_side_internal_test.go b/internal/imageprocessor/computed_side_internal_test.go new file mode 100644 index 0000000..f5c3d5a --- /dev/null +++ b/internal/imageprocessor/computed_side_internal_test.go @@ -0,0 +1,52 @@ +package imageprocessor + +import ( + "bytes" + "errors" + "testing" +) + +// TestImageProcessor_RefusesComputedSideAboveMaximum verifies that a size +// with one side 0, which Process computes from the input's proportions, is +// served when that side comes out at MaxOutputDimension and refused with +// ErrOutputTooLarge when it would be above. A 4096x1 or 1x4096 input given +// 2 for its short side has 8192 for its long side. +func TestImageProcessor_RefusesComputedSideAboveMaximum(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + inputWidth, inputHeight int + size 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}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + input := createTestJPEG(t, tt.inputWidth, tt.inputHeight) + + result, err := New(Params{}).Process(t.Context(), + bytes.NewReader(input), + &Request{Size: tt.size, Format: FormatJPEG, FitMode: FitCover}) + if result != nil { + t.Logf("Process() made %dx%d", result.Width, result.Height) + + if max(result.Width, result.Height) != MaxOutputDimension { + t.Errorf("Process() made %dx%d, want a side of %d", + result.Width, result.Height, MaxOutputDimension) + } + } + + if !errors.Is(err, tt.wantErr) { + t.Errorf("Process() error = %v, want %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/imageprocessor/imageprocessor.go b/internal/imageprocessor/imageprocessor.go index 9f08387..4ad8007 100644 --- a/internal/imageprocessor/imageprocessor.go +++ b/internal/imageprocessor/imageprocessor.go @@ -133,6 +133,14 @@ var ErrInputTooLarge = errors.New("input image dimensions exceed maximum") // configured byte limit. 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 + +// ErrOutputTooLarge is returned when a size with one side 0 would give that +// side, computed from the input's proportions, more than MaxOutputDimension. +var ErrOutputTooLarge = errors.New("output image dimensions exceed maximum") + // ErrUnsupportedOutputFormat is returned when the requested output format is // not supported. var ErrUnsupportedOutputFormat = errors.New("unsupported output format") @@ -259,8 +267,11 @@ func (p *ImageProcessor) Process( return nil, ErrInputTooLarge } - // Determine target dimensions - targetWidth, targetHeight := targetDimensions(req.Size, origWidth, origHeight) + // Determine target dimensions; too large ones are refused before resizing + targetWidth, targetHeight, err := targetDimensions(req.Size, origWidth, origHeight) + if err != nil { + return nil, err + } // Resize if needed if targetWidth != origWidth || targetHeight != origHeight { @@ -296,21 +307,29 @@ func (p *ImageProcessor) Process( // targetDimensions calculates the output dimensions for a requested size, // scaling proportionally when only one dimension is given and keeping the -// original dimensions when both are zero. -func targetDimensions(size Size, origWidth, origHeight int) (int, int) { +// original dimensions when both are zero. A side scaled proportionally can +// come out above MaxOutputDimension, which is ErrOutputTooLarge. +func targetDimensions(size Size, origWidth, origHeight int) (int, int, error) { + width, height := size.Width, size.Height + switch { - case size.Width == 0 && size.Height == 0: + case width == 0 && height == 0: // Both are 0: keep original size - return origWidth, origHeight - case size.Width == 0: + return origWidth, origHeight, nil + case width == 0: // Only height specified: calculate width proportionally - return origWidth * size.Height / origHeight, size.Height - case size.Height == 0: + width = origWidth * height / origHeight + case height == 0: // Only width specified: calculate height proportionally - return size.Width, origHeight * size.Width / origWidth - default: - return size.Width, size.Height + height = origHeight * width / origWidth } + + if width > MaxOutputDimension || height > MaxOutputDimension { + return 0, 0, fmt.Errorf("%w: %dx%d is above %d", ErrOutputTooLarge, + width, height, MaxOutputDimension) + } + + return width, height, nil } // MIME types for the supported image formats. diff --git a/internal/imgcache/urlparser.go b/internal/imgcache/urlparser.go index b11d80e..0d5e6c5 100644 --- a/internal/imgcache/urlparser.go +++ b/internal/imgcache/urlparser.go @@ -7,6 +7,8 @@ import ( "regexp" "strconv" "strings" + + "sneak.berlin/go/pixa/internal/imageprocessor" ) // URL parsing errors. @@ -20,8 +22,9 @@ var ( ErrPathTraversal = errors.New("path traversal detected") ) -// MaxDimension is the maximum allowed width or height. -const MaxDimension = 8192 +// MaxDimension is the maximum allowed width or height, the same as the image +// processor's maximum for an output image. +const MaxDimension = imageprocessor.MaxOutputDimension // ValidateDimension checks one requested width or height; name ("width" or // "height") appears in the error. 0 means "original size" and is valid. diff --git a/internal/server/computed_side_internal_test.go b/internal/server/computed_side_internal_test.go new file mode 100644 index 0000000..ded083f --- /dev/null +++ b/internal/server/computed_side_internal_test.go @@ -0,0 +1,153 @@ +package server + +import ( + "bytes" + "encoding/json" + "image/jpeg" + "net/http" + "net/http/httptest" + "testing" + + "sneak.berlin/go/pixa/internal/encurl" + "sneak.berlin/go/pixa/internal/imgcache" +) + +// TestImageProxyRefusesComputedSideAboveMaximum requests sizes of a 3000x600 +// and a 600x3000 PNG through both image routes. A size with one side 0, whose +// side computed from the source's proportions would be above 8192, is refused +// with 400 and the JSON error body. The sizes after those refusals are served +// as before, which also shows that a refusal is not cached as a failed fetch of +// the source: orig, a size with both sides given, and a size with one side 0 +// whose computed side stays within 8192. +func TestImageProxyRefusesComputedSideAboveMaximum(t *testing.T) { + t.Parallel() + + // The upstream paths of the two sources + const ( + widePath = "/wide.png" + tallPath = "/tall.png" + ) + + wide := encodeTestPNG(t, 3000, 600) + tall := encodeTestPNG(t, 600, 3000) + + upstream := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "image/png") + + if r.URL.Path == tallPath { + _, _ = w.Write(tall) + + return + } + + _, _ = w.Write(wide) + })) + t.Cleanup(upstream.Close) + + s, _, _ := startImageProxy(t, upstream) + + generator, err := encurl.NewGenerator(testSigningKey) + if err != nil { + t.Fatalf("encurl.NewGenerator: %v", err) + } + + tests := []struct { + path string + // size is the size of the /v1/image/ URL, and width and height are + // those of the encrypted URL + size string + width, height int + // wantWidth and wantHeight are the served image's, 0 for a refusal + wantWidth, wantHeight int + }{ + {widePath, "0x8192", 0, 8192, 0, 0}, + {tallPath, "8192x0", 8192, 0, 0, 0}, + {widePath, "orig", 0, 0, 3000, 600}, + {widePath, "300x200", 300, 200, 300, 200}, + {widePath, "0x300", 0, 300, 1500, 300}, + {tallPath, "300x0", 300, 0, 300, 1500}, + } + + for _, tt := range tests { + token, err := generator.Generate(&encurl.Payload{ + SourceHost: upstreamHost, SourcePath: tt.path, + Width: tt.width, Height: tt.height, Format: imgcache.FormatJPEG, + }) + if err != nil { + t.Fatalf("encurl Generate: %v", err) + } + + for _, target := range []string{ + "/v1/image/" + upstreamHost + tt.path + "/" + tt.size + ".jpeg", + "/v1/e/" + token + "/img.jpg", + } { + rec := httptest.NewRecorder() + s.ServeHTTP(rec, httptest.NewRequestWithContext( + t.Context(), http.MethodGet, target, nil)) + t.Logf("GET %s: %d, Content-Type %s", + target, rec.Code, rec.Header().Get("Content-Type")) + + if tt.wantWidth == 0 { + checkSizeRefused(t, rec) + } else { + checkServedSize(t, rec, tt.wantWidth, tt.wantHeight) + } + } + } +} + +// checkSizeRefused checks that rec answers 400 with the JSON error body that +// refuses a size whose side computed from the source's proportions is above +// 8192. +func checkSizeRefused(t *testing.T, rec *httptest.ResponseRecorder) { + t.Helper() + + const wantError = "size too large for the source: " + + "the side computed from its proportions is above 8192" + + var body struct { + Error string `json:"error"` + Status int `json:"status"` + } + + err := json.Unmarshal(rec.Body.Bytes(), &body) + if err != nil { + t.Errorf("status %d, body is not JSON: %v", rec.Code, err) + + return + } + + if rec.Code != http.StatusBadRequest || body.Status != http.StatusBadRequest || + body.Error != wantError { + t.Errorf("status %d, body %+v, want %d and the error %q", + rec.Code, body, http.StatusBadRequest, wantError) + } +} + +// checkServedSize checks that rec answers 200 with a JPEG of wantWidth by +// wantHeight. +func checkServedSize( + t *testing.T, rec *httptest.ResponseRecorder, wantWidth, wantHeight int, +) { + t.Helper() + + if rec.Code != http.StatusOK { + t.Errorf("status %d, want %d; body %s", + rec.Code, http.StatusOK, rec.Body.String()) + + return + } + + served, err := jpeg.DecodeConfig(bytes.NewReader(rec.Body.Bytes())) + if err != nil { + t.Errorf("decoding the served JPEG: %v", err) + + return + } + + if served.Width != wantWidth || served.Height != wantHeight { + t.Errorf("served %dx%d, want %dx%d", + served.Width, served.Height, wantWidth, wantHeight) + } +}