diff --git a/services/thumbnails/pkg/service/http/v0/push.go b/services/thumbnails/pkg/service/http/v0/push.go index 65ba78b7e3..f6cdf1e6bb 100644 --- a/services/thumbnails/pkg/service/http/v0/push.go +++ b/services/thumbnails/pkg/service/http/v0/push.go @@ -2,9 +2,9 @@ package svc import ( "bytes" + "errors" "fmt" "image" - "image/gif" "io" "net/http" "strconv" @@ -28,6 +28,10 @@ const ( // errInvalid mirrors imagor's ErrInvalid: a syntactically invalid request. var errInvalid = fmt.Errorf("invalid") +// errFormatUnsupported is what an encoder returns for a format this build has no +// encoder for, as opposed to an encode that went wrong. +var errFormatUnsupported = fmt.Errorf("format not supported by this build") + // pushHandler handles POST requests for the imagor-like push-based thumbnail endpoint. // Path patterns (operation and format are both optional; absent operation is the default // center-crop fill, absent format keeps the input's own format): @@ -73,13 +77,20 @@ func (s Thumbnails) pushHandler(w http.ResponseWriter, r *http.Request) { return } - file, _, err := r.FormFile("image") + file, header, err := r.FormFile("image") if err != nil { writeInvalid(w, "missing image field in form data") return } defer file.Close() + // imagor's upload loader accepts image/* only (UPLOAD_LOADER_ACCEPT), so an + // untyped part is refused here too. + if ct := header.Header.Get("Content-Type"); !strings.HasPrefix(ct, "image/") { + writeNotAcceptable(w, fmt.Sprintf("unsupported part type: %q", ct)) + return + } + imgData, err := io.ReadAll(file) if err != nil { writeInvalid(w, "failed to read image") @@ -117,9 +128,9 @@ func (s Thumbnails) pushHandler(w http.ResponseWriter, r *http.Request) { // The box and operation are chosen by webdav (the sizing brain); this service // is a dumb, imagor-like executor that just fits within the given box. - processed, err := processImage(bytes.NewReader(imgData), width, height, operation, noUpscale) + processed, err := processImage(bytes.NewReader(imgData), width, height, operation, noUpscale, ext == "gif") if err != nil { - writeInvalid(w, "failed to process image") + writeNotAcceptable(w, "failed to process image") return } @@ -135,14 +146,18 @@ func (s Thumbnails) pushHandler(w http.ResponseWriter, r *http.Request) { case "png": contentType = "image/png" encErr = encodePNG(&buf, processed) + case "webp": + contentType = "image/webp" + encErr = encodeWebP(&buf, processed) default: contentType = "image/gif" - g, ok := processed.(*gif.GIF) - if !ok { - writeInvalid(w, "processed image is not a gif") - return - } - encErr = gif.EncodeAll(&buf, g) + encErr = encodeGIF(&buf, processed) + } + if errors.Is(encErr, errFormatUnsupported) { + // 406, not 400: the request is fine, this build just cannot answer it, + // and webdav reads a 400 as its own bug + writeNotAcceptable(w, fmt.Sprintf("this build writes no %s", ext)) + return } if encErr != nil { w.WriteHeader(http.StatusInternalServerError) @@ -164,6 +179,13 @@ func parseDim(r *http.Request, name string) (int, error) { return n, nil } +// writeNotAcceptable responds with imagor's status for a source it cannot read +// (406 Not Acceptable), as opposed to a malformed request. +func writeNotAcceptable(w http.ResponseWriter, msg string) { + w.WriteHeader(http.StatusNotAcceptable) + fmt.Fprintln(w, msg) +} + // writeInvalid responds with imagor's ErrInvalid status (400 Bad Request). func writeInvalid(w http.ResponseWriter, msg string) { w.WriteHeader(http.StatusBadRequest) @@ -246,7 +268,7 @@ func outputFormatFromFilters(segment string) (ext string, hasFormat bool, err er } mapped := mapFormatToExt(args) switch mapped { - case "jpg", "png", "gif": + case "jpg", "png", "gif", "webp": return mapped, true, nil default: return "", false, fmt.Errorf("unsupported output format: %s", args) @@ -321,11 +343,12 @@ func isGifReader(r io.Reader) bool { // Backend-specific function set in init() by push_imaging.go or push_vips.go. // For gif input it returns a *gif.GIF with every frame resized (preserving -// animation); for other inputs it returns a single image.Image. The operation +// animation) when keepAnimation is set; for other inputs it returns a single +// image.Image. The operation // selects the resize/crop mode (fill, fit-in, stretch). noUpscale mirrors // imagor's no_upscale() filter: when set, fill and fit-in must not enlarge the // source (stretch is exact and ignores it). -var processImage func(io.Reader, int, int, string, bool) (any, error) +var processImage func(io.Reader, int, int, string, bool, bool) (any, error) // encodeJPEG and encodePNG write the processed image to w in the requested // format. Set by init() in push_imaging.go / push_vips.go. The vips backend @@ -336,4 +359,9 @@ var processImage func(io.Reader, int, int, string, bool) (any, error) var ( encodeJPEG func(w io.Writer, processed any) error encodePNG func(w io.Writer, processed any) error + // encodeGIF takes an animated gif as it is and palettes a still image. + encodeGIF func(w io.Writer, processed any) error + // encodeWebP is only answered by the libvips build; the pure Go one has no + // webp encoder. + encodeWebP func(w io.Writer, processed any) error ) diff --git a/services/thumbnails/pkg/service/http/v0/push_imaging.go b/services/thumbnails/pkg/service/http/v0/push_imaging.go index 34d421d508..92b6fa6bb9 100644 --- a/services/thumbnails/pkg/service/http/v0/push_imaging.go +++ b/services/thumbnails/pkg/service/http/v0/push_imaging.go @@ -5,19 +5,23 @@ package svc import ( "fmt" "image" + "image/color/palette" "image/draw" "image/gif" "image/jpeg" "image/png" "io" + "math" "github.com/kovidgoyal/imaging" ) func init() { processImage = processImageImaging - encodeJPEG = encodeJPEGImaging - encodePNG = encodePNGImaging + encodeJPEG = encodeJPEGImaging + encodePNG = encodePNGImaging + encodeGIF = encodeGIFImaging + encodeWebP = encodeWebPImaging } // processImageImaging resizes the input using the imaging backend. The operation @@ -26,8 +30,8 @@ func init() { // upscaling small sources by default like real imagor), or stretch (resize to the // exact box). noUpscale caps the // default fill at the source size, mirroring imagor's no_upscale() filter. -func processImageImaging(r io.Reader, width, height int, operation string, noUpscale bool) (any, error) { - if isGifReader(r) { +func processImageImaging(r io.Reader, width, height int, operation string, noUpscale, keepAnimation bool) (any, error) { + if keepAnimation && isGifReader(r) { g, err := gif.DecodeAll(r) if err == nil && len(g.Image) > 0 { return resizeGIF(g, width, height, operation, noUpscale), nil @@ -45,22 +49,14 @@ func processImageImaging(r io.Reader, width, height int, operation string, noUps case OpStretch: return imaging.Resize(img, width, height, imaging.Lanczos), nil case OpFitIn: - // Like real imagor, fit-in fits within the box preserving aspect and - // upscales small sources by default; no_upscale() caps it at the source - // size. imaging.Fit never enlarges, so a small source is scaled up to the - // fitted dimensions explicitly. - if noUpscale && srcBounds.Dx() <= width && srcBounds.Dy() <= height { + // imagor's fit-in only enlarges with the upscale() filter, which webdav + // never sends, so a source smaller than the box is returned as it is. + // Not imaging.Fit: it truncates the scaled side, vips and imagor round it. + w, h := fitInBox(srcBounds.Dx(), srcBounds.Dy(), width, height) + if w >= srcBounds.Dx() && h >= srcBounds.Dy() { return img, nil } - if srcBounds.Dx() <= width && srcBounds.Dy() <= height { - srcAR := float64(srcBounds.Dx()) / float64(srcBounds.Dy()) - boxAR := float64(width) / float64(height) - if srcAR > boxAR { - return imaging.Resize(img, width, int(float64(width)/srcAR), imaging.Lanczos), nil - } - return imaging.Resize(img, int(float64(height)*srcAR), height, imaging.Lanczos), nil - } - return imaging.Fit(img, width, height, imaging.Lanczos), nil + return imaging.Resize(img, w, h, imaging.Lanczos), nil default: // OpFill if noUpscale && srcBounds.Dx() <= width && srcBounds.Dy() <= height { // imagor no_upscale(): never enlarge the source. @@ -70,6 +66,14 @@ func processImageImaging(r io.Reader, width, height int, operation string, noUps } } +// fitInBox is the size an image of srcW x srcH takes inside the box, aspect +// ratio preserved and the scaled side rounded the way vips and imagor round it. +func fitInBox(srcW, srcH, boxW, boxH int) (int, int) { + scale := math.Min(float64(boxW)/float64(srcW), float64(boxH)/float64(srcH)) + + return max(int(math.Round(float64(srcW)*scale)), 1), max(int(math.Round(float64(srcH)*scale)), 1) +} + // resizeGIF resizes every frame of an animated gif while preserving the // animation. It composites each frame onto a running canvas honoring the gif // disposal method, resizes with the requested processor, and re-pallettes the @@ -153,3 +157,24 @@ func encodePNGImaging(w io.Writer, processed any) error { } return png.Encode(w, img) } + +// encodeGIFImaging writes an animated gif as it is and palettes a still image +// with the same dithering the animated path uses. +func encodeGIFImaging(w io.Writer, processed any) error { + if g, ok := processed.(*gif.GIF); ok { + return gif.EncodeAll(w, g) + } + + img, ok := processed.(image.Image) + if !ok { + return fmt.Errorf("cannot encode %T as gif", processed) + } + + return gif.Encode(w, palettedAtOrigin(img, palette.Plan9), nil) +} + +// encodeWebPImaging refuses: there is no webp encoder in the pure Go build, so a +// deployment that needs webp output runs the libvips build or a real imagor. +func encodeWebPImaging(io.Writer, any) error { + return errFormatUnsupported +} diff --git a/services/thumbnails/pkg/service/http/v0/push_test.go b/services/thumbnails/pkg/service/http/v0/push_test.go index 22c7c257a8..62beae9f90 100644 --- a/services/thumbnails/pkg/service/http/v0/push_test.go +++ b/services/thumbnails/pkg/service/http/v0/push_test.go @@ -10,6 +10,7 @@ import ( "mime/multipart" "net/http" "net/http/httptest" + "net/textproto" "testing" "github.com/go-chi/chi/v5" @@ -31,9 +32,20 @@ func createTestImage(width, height int) []byte { } func createMultipartBody(imgBytes []byte) (*bytes.Buffer, string) { + return createMultipartBodyWithType(imgBytes, "image/png") +} + +// createMultipartBodyWithType types the part the way webdav does; the upload +// loader of a real imagor accepts image/* only. +func createMultipartBodyWithType(imgBytes []byte, partType string) (*bytes.Buffer, string) { body := &bytes.Buffer{} writer := multipart.NewWriter(body) - part, err := writer.CreateFormFile("image", "test.png") + h := make(textproto.MIMEHeader) + h.Set("Content-Disposition", `form-data; name="image"; filename="test.png"`) + if partType != "" { + h.Set("Content-Type", partType) + } + part, err := writer.CreatePart(h) if err != nil { panic(err) } @@ -291,9 +303,10 @@ func TestPushEndpoint_FitIn_AspectRatio(t *testing.T) { } // TestPushEndpoint_FitIn_UpscalesByDefault documents that, like real imagor, -// fit-in upscales a small source to fit the box when no_upscale() is absent. A -// 50x50 source requested at 200x200 must come back as exactly 200x200. -func TestPushEndpoint_FitIn_UpscalesByDefault(t *testing.T) { +// fit-in never enlarges a source: imagor only upscales fit-in with the +// upscale() filter, so a 50x50 source requested at 200x200 stays 50x50. +// https://docs.imagor.net/filters +func TestPushEndpoint_FitIn_NeverUpscales(t *testing.T) { mux := newTestMux() imgBytes := createTestImage(50, 50) @@ -314,8 +327,8 @@ func TestPushEndpoint_FitIn_UpscalesByDefault(t *testing.T) { t.Fatalf("failed to decode response image: %v", err) } bounds := img.Bounds() - if bounds.Dx() != 200 || bounds.Dy() != 200 { - t.Errorf("fit-in should upscale 50x50 to fit a 200x200 box as 200x200, got %dx%d", bounds.Dx(), bounds.Dy()) + if bounds.Dx() != 50 || bounds.Dy() != 50 { + t.Errorf("fit-in should leave a 50x50 source in a 200x200 box untouched, got %dx%d", bounds.Dx(), bounds.Dy()) } } @@ -557,7 +570,7 @@ func TestPushEndpoint_UnsupportedFormat(t *testing.T) { imgBytes := createTestImage(100, 100) body, contentType := createMultipartBody(imgBytes) - req := httptest.NewRequest(http.MethodPost, "/unsafe/fit-in/50x50/filters:format(webp)/", body) + req := httptest.NewRequest(http.MethodPost, "/unsafe/fit-in/50x50/filters:format(tiff)/", body) req.Header.Set("Content-Type", contentType) rec := httptest.NewRecorder() @@ -579,8 +592,8 @@ func TestPushEndpoint_InvalidImageData(t *testing.T) { mux.ServeHTTP(rec, req) - if rec.Code != http.StatusBadRequest { - t.Errorf("expected status 400 for invalid image data, got %d: %s", rec.Code, rec.Body.String()) + if rec.Code != http.StatusNotAcceptable { + t.Errorf("expected status 406 for invalid image data, got %d: %s", rec.Code, rec.Body.String()) } } @@ -978,7 +991,7 @@ func TestOutputFormatFromFilters(t *testing.T) { }{ {"format jpeg", "format(jpeg)", "jpg", true, false}, {"extra filters ignored", "no_upscale():format(png)", "png", true, false}, - {"unsupported format", "format(webp)", "", false, true}, + {"unsupported format", "format(tiff)", "", false, true}, {"no format filter", "no_upscale()", "", false, false}, {"empty segment", "", "", false, false}, } diff --git a/services/thumbnails/pkg/service/http/v0/push_vips.go b/services/thumbnails/pkg/service/http/v0/push_vips.go index 217113babb..15ea474f67 100644 --- a/services/thumbnails/pkg/service/http/v0/push_vips.go +++ b/services/thumbnails/pkg/service/http/v0/push_vips.go @@ -5,6 +5,7 @@ package svc import ( "fmt" "image" + "image/color/palette" "image/draw" "image/gif" "io" @@ -15,8 +16,10 @@ import ( func init() { processImage = processImageVips - encodeJPEG = encodeJPEGVips - encodePNG = encodePNGVips + encodeJPEG = encodeJPEGVips + encodePNG = encodePNGVips + encodeGIF = encodeGIFVips + encodeWebP = encodeWebPVips } // processImageVips resizes the input using libvips. The operation selects the @@ -25,8 +28,8 @@ func init() { // small sources by default like real imagor), or stretch (resize to the exact box). // noUpscale caps the upscaling at the source size, mirroring imagor's no_upscale() // filter. -func processImageVips(r io.Reader, width, height int, operation string, noUpscale bool) (any, error) { - if isGifReader(r) { +func processImageVips(r io.Reader, width, height int, operation string, noUpscale, keepAnimation bool) (any, error) { + if keepAnimation && isGifReader(r) { g, err := gif.DecodeAll(r) if err == nil && len(g.Image) > 0 { return resizeGIFVips(g, width, height, operation, noUpscale), nil @@ -57,13 +60,9 @@ func processImageVips(r io.Reader, width, height int, operation string, noUpscal return nil, err } case OpFitIn: - // Like real imagor, fit-in upscales small sources to fit the box by - // default (SizeBoth); no_upscale() caps it at the source size (SizeDown). - size := vips.SizeBoth - if noUpscale { - size = vips.SizeDown - } - if err := m.ThumbnailWithSize(width, height, vips.InterestingNone, size); err != nil { + // imagor's fit-in only enlarges with the upscale() filter, which webdav + // never sends, so it always caps at the source size. + if err := m.ThumbnailWithSize(width, height, vips.InterestingNone, vips.SizeDown); err != nil { return nil, err } default: // OpFill @@ -177,3 +176,42 @@ func encodePNGVips(w io.Writer, processed any) error { _, err = w.Write(buf) return err } + +// encodeGIFVips writes an animated gif as it is; a still libvips image goes +// through the same palette the animated path uses, as libvips has no gif export. +func encodeGIFVips(w io.Writer, processed any) error { + if g, ok := processed.(*gif.GIF); ok { + return gif.EncodeAll(w, g) + } + + m, ok := processed.(*vips.ImageRef) + if !ok { + return fmt.Errorf("cannot encode %T as gif", processed) + } + defer m.Close() + + img, err := m.ToImage(vips.NewDefaultPNGExportParams()) + if err != nil { + return err + } + + return gif.Encode(w, palettedAtOrigin(img, palette.Plan9), nil) +} + +// encodeWebPVips exports the processed libvips image as webp. +func encodeWebPVips(w io.Writer, processed any) error { + m, ok := processed.(*vips.ImageRef) + if !ok { + return fmt.Errorf("cannot encode %T as webp", processed) + } + defer m.Close() + + buf, _, err := m.ExportWebp(vips.NewWebpExportParams()) + if err != nil { + return err + } + + _, err = w.Write(buf) + + return err +} diff --git a/services/thumbnails/pkg/service/http/v0/service.go b/services/thumbnails/pkg/service/http/v0/service.go index 066c76288f..c9151ef5ef 100644 --- a/services/thumbnails/pkg/service/http/v0/service.go +++ b/services/thumbnails/pkg/service/http/v0/service.go @@ -43,13 +43,18 @@ func NewService(opts ...Option) Service { // Push-based thumbnail generation endpoint (imagor-compatible). The optional // operation segment selects the resize/crop mode; its absence is the default - // center-crop fill. The filters segment is captured whole and parsed by the - // handler so any set/order of imagor filters is accepted. + // center-crop fill. The filters segment is optional too, like imagor's, and + // is captured whole and parsed by the handler so any set and order of imagor + // filters is accepted. handler := svc.pushHandler m.Post("/unsafe/{operation}/{width}x{height}/filters:{filters}/", handler) m.Post("/unsafe/{operation}/{width}x{height}/filters:{filters}", handler) m.Post("/unsafe/{width}x{height}/filters:{filters}/", handler) m.Post("/unsafe/{width}x{height}/filters:{filters}", handler) + m.Post("/unsafe/{operation}/{width}x{height}/", handler) + m.Post("/unsafe/{operation}/{width}x{height}", handler) + m.Post("/unsafe/{width}x{height}/", handler) + m.Post("/unsafe/{width}x{height}", handler) _ = chi.Walk(m, func(method string, route string, _ http.Handler, middlewares ...func(http.Handler) http.Handler) error { options.Logger.Debug().Str("method", method).Str("route", route).Int("middlewares", len(middlewares)).Msg("serving endpoint")