fix(thumbnails): answer like imagor, and write what imagor writes

The upload part has to be typed, a malformed request is a 400 while a
source it cannot read is a 406, an animated gif flattens when the output
is still, a still source can be written as a gif, libvips writes webp, and
fit-in rounds the way vips and imagor round it.

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
 )

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
+}

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},
 	}

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
+}

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")