fix: answer the review, and read imagor's path the way imagor does

The push endpoint takes an empty filters segment, ignores an image key behind the box and computes a side given as 0 from the other one, all measured against both imagors and pinned in the parity suite. Plus the review leftovers: no em dashes, the request test on ginkgo, comments that contradicted the code, one stat for Head and generate, one thumbnail handler, the gif fit-in rounding of vips, and the new env names last so they win.
This commit is contained in:
Dominik Schmidt committed 2026-10-19 14:57:35 +02:00
1 parent 85b7cccc02
commit 3572410fc4
27 files changed
+342 -236

No files matched your search

+3
View File
@@ -20,6 +20,9 @@ exclude_paths:
- 'tests/acceptance/expected-failures-*.md'
# written by the search engine parity suite, table rows are as wide as they are
- 'services/search/pkg/parity/README.md'
# thumbnail docs: wide tables and prose that is not hard-wrapped
- 'services/thumbnails/README.md'
- 'services/webdav/README.md'
- 'tests/acceptance/bootstrap/**'
- 'tests/acceptance/TestHelpers/**'
- 'tests/acceptance/scripts/run.sh'
+1
View File
@@ -163,6 +163,7 @@
"STORE_GRPC_ADDR": "127.0.0.1:10460",
"THUMBNAILS_DEBUG_ADDR": "127.0.0.1:10194",
"THUMBNAILS_HTTP_ADDR": "127.0.0.1:10190",
"WEBDAV_THUMBNAIL_GENERATOR_URL": "http://127.0.0.1:10190",
"USERLOG_HTTP_ADDR": "127.0.0.1:10211",
"USERLOG_DEBUG_ADDR": "127.0.0.1:10210",
"USERS_DEBUG_ADDR": "127.0.0.1:10145",
+12 -3
View File
@@ -22,13 +22,22 @@ source file to this endpoint and receives the processed image back.
| `POST /unsafe/fit-in/{width}x{height}/filters:{filters}` | Scale to fit within width x height, preserving aspect ratio and never upscaling |
| `POST /unsafe/stretch/{width}x{height}/filters:{filters}` | Resize to the exact width x height without preserving aspect ratio (distorts) |
The filters segment is optional, like imagor's. Inside it, `format` and
`no_upscale` are the filters this executor acts on, every other filter is
ignored. `format` itself is optional: without it the input's own format is
The path is imagor's. The filters segment is optional and may be empty
(`filters:`), and an image key behind it is accepted and ignored, because the
source arrives in the upload rather than being loaded by key. A bare box with
nothing behind it is the one place this service and imagor part: imagor reads the
last segment as the image key, so `/unsafe/100x100` is a key and nothing is
resized, while this service treats it as the box. Inside the filters segment,
`format` and `no_upscale` are the filters this executor acts on, every other
filter is ignored. `format` itself is optional: without it the input's own format is
preserved, the same way imagor does it. An input this service cannot write
(tiff, bmp) becomes a JPEG; imagor writes tiff back in its own format and only
falls back to JPEG for the ones it cannot write either.
A side given as `0` is computed from the other one, so `/unsafe/0x100/` keeps the
aspect ratio and `/unsafe/0x0/` returns the source at its own size. A free axis
leaves nothing to crop to, so the operation makes no difference to it.
Like real imagor, the default fill resize **upscales** small sources to fill
the box exactly (a 100x100 source requested at `320x320` returns a 320x320
image). Adding the `no_upscale()` filter caps the result at the source size
+3 -7
View File
@@ -28,13 +28,9 @@ type Config struct {
// still rejected, but the enforcement now lives at decode time in this service
// (which owns the image) rather than in webdav.
type Thumbnail struct {
// MaxInputWidth is the maximum width of an input image which is being processed.
MaxInputWidth int `yaml:"max_input_width" env:"THUMBNAILS_MAX_INPUT_WIDTH" desc:"The maximum width of an input image which is being processed." introductionVersion:"1.0.0"`
// MaxInputHeight is the maximum height of an input image which is being processed.
MaxInputWidth int `yaml:"max_input_width" env:"THUMBNAILS_MAX_INPUT_WIDTH" desc:"The maximum width of an input image which is being processed." introductionVersion:"1.0.0"`
MaxInputHeight int `yaml:"max_input_height" env:"THUMBNAILS_MAX_INPUT_HEIGHT" desc:"The maximum height of an input image which is being processed." introductionVersion:"1.0.0"`
// MaxConcurrentRequests limits the number of thumbnail requests that are
// decoded and resized in parallel. 0 (the default) means unlimited, matching
// main's THUMBNAILS_MAX_CONCURRENT_REQUESTS. Requests arriving while the limit
// is reached get HTTP 429 Too Many Requests.
// 0 (the default) means unlimited; a request arriving while the limit is
// reached gets HTTP 429 with a Retry-After.
MaxConcurrentRequests int `yaml:"max_concurrent_requests" env:"THUMBNAILS_MAX_CONCURRENT_REQUESTS" desc:"Maximum number of concurrent thumbnail generation requests. Default is 0 which is unlimited." introductionVersion:"%%NEXT%%"`
}
@@ -0,0 +1,60 @@
package parity
// imagor's path is "/unsafe[/{operation}]/{W}x{H}[/filters:{list}]/{image}": the
// filters may be empty, the image key is the last segment and an upload loader
// has no use for it, and a side given as 0 is computed from the other one.
func grammarGroup() parityGroup {
src := func() source { return pngSource(300, 200) }
return parityGroup{
name: "grammar",
cases: []parityCase{
{
title: "an empty filters segment is still a filters segment", path: "/unsafe/100x100/filters:",
source: src(), want: "200 100x100 png",
},
{
title: "an image key behind the box is ignored", path: "/unsafe/100x100/foo.png",
source: src(), want: "200 100x100 png",
},
{
title: "an image key behind the filters is ignored", path: "/unsafe/100x100/filters:format(png)/foo.png",
source: src(), want: "200 100x100 png",
},
{
title: "a zero width follows the height", path: "/unsafe/0x100/filters:format(png)/",
source: src(), want: "200 150x100 png",
},
{
title: "a zero height follows the width", path: "/unsafe/100x0/filters:format(png)/",
source: src(), want: "200 100x67 png",
},
{
title: "a free axis enlarges", path: "/unsafe/0x400/filters:format(png)/",
source: src(), want: "200 600x400 png",
},
{
title: "a free axis under fit-in is the same, there is nothing to crop to", path: "/unsafe/fit-in/0x100/filters:format(png)/",
source: src(), want: "200 150x100 png",
},
{
title: "a free axis under stretch is the same", path: "/unsafe/stretch/100x0/filters:format(png)/",
source: src(), want: "200 100x67 png",
generatorOverrides: map[string]override{
"imagor": {answers: "200 100x66 png", because: "imagor truncates the computed side under stretch and rounds it everywhere else"},
},
},
{
title: "a zero box leaves the source alone", path: "/unsafe/0x0/filters:format(png)/",
source: src(), want: "200 300x200 png",
},
{
title: "a bare box with no tail at all", path: "/unsafe/100x100",
source: src(), want: "200 100x100 png",
generatorOverrides: map[string]override{
"imagor": {answers: "200 300x200 png", because: "the last segment is imagor's image key, so a bare box is a key and nothing is resized"},
},
},
},
}
}
@@ -76,6 +76,7 @@ func parityGroups() []parityGroup {
limitsGroup(),
iconGroup(),
transparencyGroup(),
grammarGroup(),
}
}
@@ -0,0 +1,27 @@
package svc
import "math"
// 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)
}
// autoAxis resolves imagor's zero axis: a side given as 0 is computed from the
// other one, so the aspect ratio decides it, and two zero sides mean the image
// is returned at its own size. The bool reports whether anything is left to
// resize, and a resolved box is reached by a plain resize for every operation,
// because with a free axis there is nothing to crop to.
func autoAxis(srcW, srcH, boxW, boxH int) (int, int, bool) {
switch {
case boxW == 0 && boxH == 0:
return srcW, srcH, false
case boxW == 0:
return max(int(math.Round(float64(srcW)*float64(boxH)/float64(srcH))), 1), boxH, true
default:
return boxW, max(int(math.Round(float64(srcH)*float64(boxW)/float64(srcW))), 1), true
}
}
@@ -94,7 +94,7 @@ func (s Thumbnails) pushHandler(w http.ResponseWriter, r *http.Request) {
return
}
filtersSegment := chi.URLParam(r, "filters")
filtersSegment := filtersFromTail(chi.URLParam(r, "*"))
ext, hasFormat, err := outputFormatFromFilters(filtersSegment)
if err != nil {
writeInvalid(w, err.Error())
@@ -183,12 +183,26 @@ func (s Thumbnails) pushHandler(w http.ResponseWriter, r *http.Request) {
func parseDim(r *http.Request, name string) (int, error) {
v := chi.URLParam(r, name)
n, err := strconv.Atoi(v)
if err != nil || n <= 0 {
if err != nil || n < 0 {
return 0, fmt.Errorf("invalid %s: %s", name, v)
}
return n, nil
}
// filtersFromTail reads the filters out of imagor's path tail. The tail is the
// optional "filters:..." segment plus an optional image key behind it; only the
// filters segment is ours, and it may be empty.
func filtersFromTail(tail string) string {
segment, _, _ := strings.Cut(tail, "/")
filters, ok := strings.CutPrefix(segment, "filters:")
if !ok {
return ""
}
return filters
}
// 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) {
@@ -13,7 +13,6 @@ import (
"net/textproto"
"github.com/go-chi/chi/v5"
"github.com/go-chi/chi/v5/middleware"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"golang.org/x/image/bmp"
@@ -142,13 +141,8 @@ func multipartBody(source []byte, partType string) (*bytes.Buffer, string) {
// testMux is the production route table with the configured input limits; 0
// disables a bound.
func testMux(maxWidth, maxHeight int) *chi.Mux {
mux := chi.NewRouter()
h := Thumbnails{maxWidth: maxWidth, maxHeight: maxHeight}.pushHandler
mux.Use(middleware.StripSlashes)
mux.Post("/unsafe/{operation}/{width}x{height}/filters:{filters}", h)
mux.Post("/unsafe/{width}x{height}/filters:{filters}", h)
mux.Post("/unsafe/{operation}/{width}x{height}", h)
mux.Post("/unsafe/{width}x{height}", h)
mux := chi.NewMux()
mountPushRoutes(mux, Thumbnails{maxWidth: maxWidth, maxHeight: maxHeight}.pushHandler)
return mux
}
@@ -10,7 +10,6 @@ import (
"image/jpeg"
"image/png"
"io"
"math"
"github.com/kovidgoyal/imaging"
)
@@ -25,10 +24,10 @@ func init() {
// processImageImaging resizes the input using the imaging backend. The operation
// selects the resize/crop mode: fill (center-crop to the box, upscaling by default
// like real imagor's default resize), fit-in (fit within the box without cropping,
// 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.
// like real imagor's default resize), fit-in (fit within the box without cropping
// and without ever enlarging the source, like imagor without its upscale()
// filter), 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, req resizeRequest) (any, error) {
if req.KeepAnimation && isGifReader(r) {
g, err := gif.DecodeAll(r)
@@ -44,6 +43,15 @@ func processImageImaging(r io.Reader, req resizeRequest) (any, error) {
srcBounds := img.Bounds()
if req.Width == 0 || req.Height == 0 {
boxW, boxH, resize := autoAxis(srcBounds.Dx(), srcBounds.Dy(), req.Width, req.Height)
if !resize {
return img, nil
}
return imaging.Resize(img, boxW, boxH, imaging.Lanczos), nil
}
switch req.Operation {
case OpStretch:
return imaging.Resize(img, req.Width, req.Height, imaging.Lanczos), nil
@@ -65,14 +73,6 @@ func processImageImaging(r io.Reader, req resizeRequest) (any, error) {
}
}
// 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)
}
// encodeJPEGImaging encodes the processed image as JPEG using the stdlib encoder.
func encodeJPEGImaging(w io.Writer, processed any) error {
img, ok := processed.(image.Image)
@@ -121,6 +121,44 @@ var _ = Describe("the push endpoint", func() {
})
})
// Measured against shumc/imagor:1.9.11: everything behind the box is imagor's
// tail, and a side given as 0 is computed from the other one.
DescribeTable("reads imagor's path the way imagor reads it",
func(path string, source []byte, want image.Rectangle) {
rec := post(path, source)
Expect(rec.Code).To(Equal(http.StatusOK), rec.Body.String())
Expect(decodedBounds(rec)).To(Equal(want))
},
Entry("an empty filters segment is a filters segment",
"/unsafe/50x50/filters:", pngSource(200, 200), image.Rect(0, 0, 50, 50)),
Entry("an empty filters segment with an operation",
"/unsafe/fit-in/50x50/filters:", pngSource(200, 100), image.Rect(0, 0, 50, 25)),
Entry("an image key behind the box is ignored",
"/unsafe/50x50/foo.png", pngSource(200, 200), image.Rect(0, 0, 50, 50)),
Entry("an image key behind the filters is ignored",
"/unsafe/50x50/filters:format(png)/foo.png", pngSource(200, 200), image.Rect(0, 0, 50, 50)),
Entry("a zero width follows the height",
"/unsafe/0x100/filters:format(png)/", pngSource(300, 200), image.Rect(0, 0, 150, 100)),
Entry("a zero height follows the width",
"/unsafe/100x0/filters:format(png)/", pngSource(300, 200), image.Rect(0, 0, 100, 67)),
Entry("a free axis ignores the operation, there is nothing to crop to",
"/unsafe/fit-in/0x100/filters:format(png)/", pngSource(300, 200), image.Rect(0, 0, 150, 100)),
Entry("a free axis enlarges, like imagor",
"/unsafe/0x200/filters:format(png)/", pngSource(150, 100), image.Rect(0, 0, 300, 200)),
Entry("a zero box leaves the source alone",
"/unsafe/0x0/filters:format(png)/", pngSource(300, 200), image.Rect(0, 0, 300, 200)),
)
It("keeps an animated gif animated on a free axis", func() {
rec := post("/unsafe/0x40/filters:format(gif)/", gifSource(200, 100, 3))
Expect(rec.Code).To(Equal(http.StatusOK), rec.Body.String())
out := decodedGIF(rec)
Expect(out.Image).To(HaveLen(3))
Expect(out.Image[0].Bounds()).To(Equal(image.Rect(0, 0, 80, 40)))
})
DescribeTable("refuses what it cannot answer",
func(path string, source []byte, partType string, want int) {
rec := postTo(testMux(0, 0), path, source, partType)
@@ -129,8 +167,8 @@ var _ = Describe("the push endpoint", func() {
},
Entry("dimensions that are not numbers",
"/unsafe/fit-in/abcxdef/filters:format(jpeg)/", pngSource(100, 100), "image/png", http.StatusBadRequest),
Entry("a zero box",
"/unsafe/fit-in/0x0/filters:format(jpeg)/", pngSource(100, 100), "image/png", http.StatusBadRequest),
Entry("a negative box",
"/unsafe/fit-in/-50x50/filters:format(jpeg)/", pngSource(100, 100), "image/png", http.StatusBadRequest),
Entry("a format nobody writes",
"/unsafe/fit-in/50x50/filters:format(tiff)/", pngSource(100, 100), "image/png", http.StatusBadRequest),
Entry("bytes that are not an image",
@@ -21,10 +21,10 @@ func init() {
// processImageVips resizes the input using libvips. The operation selects the
// resize/crop mode: fill (center-crop to the box, upscaling by default like real
// imagor's default resize), fit-in (fit within the box without cropping, upscaling
// 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.
// imagor's default resize), fit-in (fit within the box without cropping and
// without ever enlarging the source, like imagor without its upscale() filter),
// or stretch (resize to the exact box). noUpscale caps the default fill at the
// source size, mirroring imagor's no_upscale() filter.
func processImageVips(r io.Reader, req resizeRequest) (any, error) {
if req.KeepAnimation && isGifReader(r) {
g, err := gif.DecodeAll(r)
@@ -48,6 +48,23 @@ func processImageVips(r io.Reader, req resizeRequest) (any, error) {
// after exporting. Closing it here would free the underlying C image before the
// export runs (use-after-free).
if req.Width == 0 || req.Height == 0 {
boxW, boxH, resize := autoAxis(m.Width(), m.Height(), req.Width, req.Height)
if resize {
hScale := float64(boxW) / float64(m.Width())
vScale := float64(boxH) / float64(m.Height())
if err := m.ResizeWithVScale(hScale, vScale, vips.KernelLanczos3); err != nil {
return nil, err
}
}
if err := m.RemoveMetadata(); err != nil {
return nil, err
}
return m, nil
}
switch req.Operation {
case OpStretch:
// Resize to the exact box without preserving aspect ratio.
@@ -16,6 +16,15 @@ import (
// https://github.com/willnorris/gifresize.
func resizeGIF(m *gif.GIF, width, height int, operation string, noUpscale bool) *gif.GIF {
srcX, srcY := m.Config.Width, m.Config.Height
if width == 0 || height == 0 {
resolvedW, resolvedH, resize := autoAxis(srcX, srcY, width, height)
if !resize {
return m
}
width, height, operation = resolvedW, resolvedH, OpStretch
}
b := image.Rect(0, 0, srcX, srcY)
tmp := image.NewRGBA(b)
@@ -29,10 +38,12 @@ func resizeGIF(m *gif.GIF, width, height int, operation string, noUpscale bool)
case OpStretch:
processed = imaging.Resize(tmp, width, height, imaging.Lanczos)
case OpFitIn:
if noUpscale && srcX <= width && srcY <= height {
// not imaging.Fit: it truncates the scaled side, vips and imagor round
// it, and fit-in never enlarges whether no_upscale() was sent or not.
if fitW, fitH := fitInBox(srcX, srcY, width, height); fitW >= srcX && fitH >= srcY {
processed = tmp
} else {
processed = imaging.Fit(tmp, width, height, imaging.Lanczos)
processed = imaging.Resize(tmp, fitW, fitH, imaging.Lanczos)
}
default: // OpFill
if noUpscale && srcX <= width && srcY <= height {
@@ -42,19 +42,7 @@ func NewService(opts ...Option) Service {
limiter: newConcurrencyLimiter(limits.MaxConcurrentRequests),
}
// 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 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
// StripSlashes makes the trailing slash optional, the operation and filters
// segments are the four remaining combinations.
m.Use(middleware.StripSlashes)
m.Post("/unsafe/{operation}/{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/{width}x{height}", handler)
mountPushRoutes(m, svc.pushHandler)
_ = 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")
@@ -64,6 +52,21 @@ func NewService(opts ...Option) Service {
return svc
}
// mountPushRoutes puts the imagor-compatible push endpoint on a mux. The
// optional operation segment selects the resize/crop mode; its absence is the
// default center-crop fill. Everything after the box is imagor's tail: an
// optional filters segment, which may be empty, followed by an optional image
// key, which an upload loader has no use for and which is ignored here too.
// StripSlashes makes the trailing slash optional, the operation segment and the
// tail are the four remaining combinations.
func mountPushRoutes(m *chi.Mux, handler http.HandlerFunc) {
m.Use(middleware.StripSlashes)
m.Post("/unsafe/{operation}/{width}x{height}/*", handler)
m.Post("/unsafe/{width}x{height}/*", handler)
m.Post("/unsafe/{operation}/{width}x{height}", handler)
m.Post("/unsafe/{width}x{height}", handler)
}
// Thumbnails implements the business logic for Service.
type Thumbnails struct {
logger log.Logger
+1 -1
View File
@@ -66,7 +66,7 @@ When what a request resolves to contradicts an `a` the client sent, the response
#### Thumbnail Generator
The thumbnails themselves are made by a generator the webdav service POSTs to, configured with `WEBDAV_THUMBNAIL_GENERATOR_URL` (and `WEBDAV_THUMBNAIL_GENERATOR_TIMEOUT`). The built-in thumbnails service speaks the imagor image endpoint, so a real [imagor](https://imagor.net) can be put in its place without changing anything else. A source the generator cannot read answers `404`, one that is too large `403`, and `WEBDAV_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE` caps what is sent at all. A generator that refuses because it is busy answers `429` with its own `Retry-After`, so a client can come back instead of treating it as a missing preview.
The thumbnails themselves are made by a generator the webdav service POSTs to, configured with `WEBDAV_THUMBNAIL_GENERATOR_URL` (and `WEBDAV_THUMBNAIL_GENERATOR_TIMEOUT`). The built-in thumbnails service speaks the imagor image endpoint, so a real [imagor](https://imagor.net) can be put in its place without changing anything else. A source the generator cannot read answers `404`, one that is too large `403`, and `WEBDAV_THUMBNAIL_MAX_INPUT_IMAGE_FILE_SIZE` caps what is sent at all. A generator that refuses because it is busy answers `429` with its own `Retry-After`, so a client can come back instead of treating it as a missing preview.
Three settings say what is made of what. They are merged onto the built-in defaults, so naming one type does not take the answer away from the others:
+2 -2
View File
@@ -29,7 +29,7 @@ type Config struct {
ThumbnailGeneratorURL string `yaml:"thumbnail_generator_url" env:"WEBDAV_THUMBNAIL_GENERATOR_URL" desc:"Base URL of the thumbnail generator service, e.g. http://thumbnails:9186" introductionVersion:"%%NEXT%%"`
ThumbnailGeneratorTimeout string `yaml:"thumbnail_generator_timeout" env:"WEBDAV_THUMBNAIL_GENERATOR_TIMEOUT" desc:"HTTP timeout for requests to the thumbnail generator, e.g. 30s" introductionVersion:"%%NEXT%%"`
// the yaml key stays unprefixed, a config file written against it has to keep working
ThumbnailMaxInputFileSize string `yaml:"max_input_file_size" env:"WEBDAV_THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE;THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE" desc:"Maximum file size of an input image for thumbnail generation. Usable common abbreviations: [KB, KiB, MB, MiB, GB, GiB], example: 50MB" introductionVersion:"%%NEXT%%"`
ThumbnailMaxInputFileSize string `yaml:"max_input_file_size" env:"THUMBNAILS_MAX_INPUT_IMAGE_FILE_SIZE;WEBDAV_THUMBNAIL_MAX_INPUT_IMAGE_FILE_SIZE" desc:"Maximum file size of an input image for thumbnail generation. Usable common abbreviations: [KB, KiB, MB, MiB, GB, GiB], example: 50MB" introductionVersion:"%%NEXT%%"`
ThumbnailCacheBackend string `yaml:"thumbnail_cache_backend" env:"WEBDAV_THUMBNAIL_CACHE_BACKEND" desc:"Cache backend for imagor thumbnails: 'none', 'memory', 'file', or 's3'. Default: file." introductionVersion:"%%NEXT%%"`
ThumbnailCacheDir string `yaml:"thumbnail_cache_dir" env:"WEBDAV_THUMBNAIL_CACHE_DIR" desc:"Directory for file-based thumbnail cache (Default: $OC_BASE_DATA_PATH/thumbnails/files). Only used when ThumbnailCacheBackend is 'file'." introductionVersion:"%%NEXT%%"`
@@ -47,7 +47,7 @@ type Config struct {
ThumbnailTypeMapping []string `yaml:"thumbnail_type_mapping" env:"OC_THUMBNAIL_TYPE_MAPPING;WEBDAV_THUMBNAIL_TYPE_MAPPING" desc:"The type a thumbnail is returned in, per type of the image it is made from, as mime:mime entries, e.g. image/vnd.microsoft.icon:image/png. The target is asked for by the name WEBDAV_THUMBNAIL_GENERATOR_FORMATS gives it. A type that is not listed, and one whose target the generator cannot write, becomes a JPEG." introductionVersion:"%%NEXT%%"`
// the yaml key stays unprefixed, a config file written against it has to keep working
ThumbnailFontMapFile string `yaml:"font_map_file" env:"WEBDAV_THUMBNAILS_TXT_FONTMAP_FILE;THUMBNAILS_TXT_FONTMAP_FILE" desc:"The path to a font map file for txt thumbnails." introductionVersion:"%%NEXT%%"`
ThumbnailFontMapFile string `yaml:"font_map_file" env:"THUMBNAILS_TXT_FONTMAP_FILE;WEBDAV_THUMBNAIL_TXT_FONTMAP_FILE" desc:"The path to a font map file for txt thumbnails." introductionVersion:"%%NEXT%%"`
Context context.Context `yaml:"-"`
}
@@ -0,0 +1,13 @@
package requests_test
import (
"testing"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
func TestRequests(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Requests Suite")
}
@@ -37,7 +37,6 @@ type ThumbnailRequest struct {
Ref *providerv1beta1.Reference
// The file name of the source file including the extension
Filename string
// The file extension
// The requested width of the thumbnail
Width int32
// The requested height of the thumbnail
@@ -1,101 +1,69 @@
package requests
package requests_test
import (
"context"
"net/http"
"net/http/httptest"
"testing"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/opencloud-eu/opencloud/pkg/conversions"
"github.com/opencloud-eu/opencloud/services/webdav/pkg/constants"
"github.com/opencloud-eu/opencloud/services/webdav/pkg/dav/requests"
)
func newThumbnailRequest(t *testing.T, rawQuery string) *http.Request {
t.Helper()
func thumbnailRequest(rawQuery string) *http.Request {
req := httptest.NewRequest("GET", "/files/test.png?"+rawQuery, nil)
ctx := context.WithValue(req.Context(), constants.ContextKeyPath, "/files/test.png")
return req.WithContext(ctx)
return req.WithContext(context.WithValue(req.Context(), constants.ContextKeyPath, "/files/test.png"))
}
var (
trueValue = true
falseValue = false
)
var _ = Describe("ParseThumbnailRequest", func() {
DescribeTable("the legacy a flag",
func(query string, want *bool) {
tr, err := requests.ParseThumbnailRequest(thumbnailRequest(query))
Expect(err).ToNot(HaveOccurred())
if want == nil {
Expect(tr.Aspect).To(BeNil())
func TestParseThumbnailRequestAspect(t *testing.T) {
tests := []struct {
name string
query string
wantA *bool
}{
{"a absent is not set at all", "x=1024&y=1024", nil},
{"a=1 preserves aspect", "x=1024&y=1024&a=1", &trueValue},
{"a=0 fills the box", "x=1024&y=1024&a=0", &falseValue},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r := newThumbnailRequest(t, tt.query)
tr, err := ParseThumbnailRequest(r)
if err != nil {
t.Fatalf("ParseThumbnailRequest() error = %v", err)
}
switch {
case tt.wantA == nil && tr.Aspect != nil:
t.Errorf("Aspect = %v, want it unset", *tr.Aspect)
case tt.wantA != nil && tr.Aspect == nil:
t.Errorf("Aspect unset, want %v", *tt.wantA)
case tt.wantA != nil && *tr.Aspect != *tt.wantA:
t.Errorf("Aspect = %v, want %v", *tr.Aspect, *tt.wantA)
}
})
}
}
func TestParseThumbnailRequestProcessor(t *testing.T) {
r := newThumbnailRequest(t, "x=320&y=320&processor=fit")
tr, err := ParseThumbnailRequest(r)
if err != nil {
t.Fatalf("ParseThumbnailRequest() error = %v", err)
}
if tr.Processor != "fit" {
t.Errorf("Processor = %q, want %q", tr.Processor, "fit")
}
if tr.Width != 320 || tr.Height != 320 {
t.Errorf("dimensions = %dx%d, want 320x320", tr.Width, tr.Height)
}
}
func TestParseThumbnailRequestDimensions(t *testing.T) {
tests := []struct {
name string
query string
wantW, wantH int32
wantErr bool
}{
{name: "both given", query: "x=448&y=448", wantW: 448, wantH: 448},
{name: "neither given keeps the legacy default", query: "preview=1", wantW: DefaultWidth, wantH: DefaultHeight},
{name: "an omitted axis is unconstrained", query: "y=512&processor=fit", wantW: 0, wantH: 512},
{name: "an explicit zero is rejected", query: "x=0&y=512&processor=fit", wantErr: true},
{name: "negative is rejected", query: "x=-1&y=512", wantErr: true},
{name: "an unconstrained axis without a processor is fitted, not rejected", query: "y=512", wantW: 0, wantH: 512},
{name: "cropping to an unconstrained axis is rejected explicitly", query: "y=512&processor=thumbnail", wantErr: true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tr, err := ParseThumbnailRequest(newThumbnailRequest(t, tt.query))
if tt.wantErr {
if err == nil {
t.Fatalf("ParseThumbnailRequest() error = nil, want an error")
}
return
}
if err != nil {
t.Fatalf("ParseThumbnailRequest() error = %v", err)
}
if tr.Width != tt.wantW || tr.Height != tt.wantH {
t.Errorf("dimensions = %dx%d, want %dx%d", tr.Width, tr.Height, tt.wantW, tt.wantH)
}
})
}
}
Expect(tr.Aspect).To(HaveValue(Equal(*want)))
},
Entry("stays unset when the client does not send it", "x=1024&y=1024", nil),
Entry("preserves the aspect ratio with a=1", "x=1024&y=1024&a=1", conversions.ToPointer(true)),
Entry("fills the box with a=0", "x=1024&y=1024&a=0", conversions.ToPointer(false)),
)
It("keeps the processor and both dimensions", func() {
tr, err := requests.ParseThumbnailRequest(thumbnailRequest("x=320&y=320&processor=fit"))
Expect(err).ToNot(HaveOccurred())
Expect(tr.Processor).To(Equal("fit"))
Expect(tr.Width).To(Equal(int32(320)))
Expect(tr.Height).To(Equal(int32(320)))
})
DescribeTable("the requested box",
func(query string, wantWidth, wantHeight int32) {
tr, err := requests.ParseThumbnailRequest(thumbnailRequest(query))
Expect(err).ToNot(HaveOccurred())
Expect(tr.Width).To(Equal(wantWidth))
Expect(tr.Height).To(Equal(wantHeight))
},
Entry("takes both axes when they are given", "x=448&y=448", int32(448), int32(448)),
Entry("keeps the legacy default when neither is given", "preview=1", int32(requests.DefaultWidth), int32(requests.DefaultHeight)),
Entry("leaves an omitted axis unconstrained", "y=512&processor=fit", int32(0), int32(512)),
Entry("fits an unconstrained axis without a processor", "y=512", int32(0), int32(512)),
)
DescribeTable("a request it refuses",
func(query string) {
_, err := requests.ParseThumbnailRequest(thumbnailRequest(query))
Expect(err).To(HaveOccurred())
},
Entry("an explicit zero", "x=0&y=512&processor=fit"),
Entry("a negative size", "x=-1&y=512"),
Entry("cropping to an unconstrained axis", "y=512&processor=thumbnail"),
)
})
+1 -1
View File
@@ -20,7 +20,7 @@ const (
// operationSegment maps an operation to its imagor URL path segment. The fill
// operation is the bare WxH resize (imagor's default), so it contributes no
// leading segment; stretch and fit-in use their imagor names. Note that real
// imagor has no /fill/ route — "fill" is only our internal name for the default
// imagor has no /fill/ route, "fill" is only our internal name for the default
// center-crop resize.
func operationSegment(op string) string {
switch op {
@@ -36,7 +36,6 @@ type Image struct {
Data []byte
}
// FileConverter is the interface for the file converters
// GgsDecoder is a converter for the geogebra slides file
type GgsDecoder struct{ thumbnailpath string }
+6 -16
View File
@@ -112,10 +112,10 @@ func NewService(opts ...Option) (Service, error) {
r.Use(svc.DavUserContext())
if !conf.DisablePreviews {
r.Get("/remote.php/dav/spaces/{id}", svc.SpacesThumbnail)
r.Get("/remote.php/dav/spaces/{id}/*", svc.SpacesThumbnail)
r.Get("/dav/spaces/{id}", svc.SpacesThumbnail)
r.Get("/dav/spaces/{id}/*", svc.SpacesThumbnail)
r.Get("/remote.php/dav/spaces/{id}", svc.Thumbnail)
r.Get("/remote.php/dav/spaces/{id}/*", svc.Thumbnail)
r.Get("/dav/spaces/{id}", svc.Thumbnail)
r.Get("/dav/spaces/{id}/*", svc.Thumbnail)
r.Get("/remote.php/dav/files/{id}", svc.Thumbnail)
r.Get("/remote.php/dav/files/{id}/*", svc.Thumbnail)
@@ -359,18 +359,8 @@ func (g Webdav) writeThumbnail(w http.ResponseWriter, r *http.Request, tr *reque
generator.WriteThumbnailResponse(w, thumb.Data, thumb.ContentType)
}
// SpacesThumbnail is the endpoint for retrieving thumbnails inside of spaces.
func (g Webdav) SpacesThumbnail(w http.ResponseWriter, r *http.Request) {
logger := g.requestLogger(r)
tr, ok := g.parseThumbnailRequest(w, r, logger)
if !ok {
return
}
g.writeThumbnail(w, r, tr, r.Header.Get(revactx.TokenHeader), logger)
}
// Thumbnail implements the Service interface.
// Thumbnail serves a thumbnail for every authenticated DAV path, spaces, files
// and the legacy /webdav root alike: the reference is parsed from the request.
func (g Webdav) Thumbnail(w http.ResponseWriter, r *http.Request) {
logger := g.requestLogger(r)
tr, ok := g.parseThumbnailRequest(w, r, logger)
@@ -45,9 +45,6 @@ func (t SourceTypes) Supports(contentType string) bool {
return supported
}
// All lists the types, so a caller can iterate them.
func (t SourceTypes) All() map[string]struct{} { return t.byType }
// List is the types as a slice, for a configuration that takes one.
func (t SourceTypes) List() []string {
out := make([]string, 0, len(t.byType))
@@ -12,7 +12,7 @@ var _ = Describe("TypeMapping", func() {
m := thumbnail.NewTypeMapping(nil)
It("answers for every type a preview is made of", func() {
for mimeType := range thumbnail.NewSourceTypes(nil).All() {
for _, mimeType := range thumbnail.NewSourceTypes(nil).List() {
Expect(m.TargetFor(mimeType)).ToNot(BeEmpty(), "%s has no output type", mimeType)
}
})
@@ -118,23 +118,14 @@ func fileIsProcessing(info *providerv1beta1.ResourceInfo) bool {
return utils.ReadPlainFromOpaque(info.GetOpaque(), "status") == "processing"
}
// processorID normalizes the requested processor to the identifier used in cache
// keys, matching the legacy behavior where an empty processor resolved to "thumbnail".
func processorID(processor string) string {
if processor == "" {
return "thumbnail"
}
return strings.ToLower(processor)
}
// thumbnailStorageKey builds the cache key: checksum[:2]/checksum[2:4]/checksum[4:]/wxh-<operation>.format.
// The <operation> segment (fit-in, fill, stretch, ...) distinguishes requests that
// share a size but produce different images, so they never share a cache entry.
// The key carries the snapped box, not the requested size, so every request that
// rounds to the same box shares one entry. Caches regenerate when the configured
// resolutions or operations change.
func thumbnailStorageKey(checksum string, w, h int, format, processor string) string {
name := fmt.Sprintf("%dx%d-%s.%s", w, h, processor, format)
func thumbnailStorageKey(checksum string, w, h int, format, operation string) string {
name := fmt.Sprintf("%dx%d-%s.%s", w, h, operation, format)
if len(checksum) >= 6 {
return checksum[:2] + "/" + checksum[2:4] + "/" + checksum[4:] + "/" + name
}
@@ -295,38 +286,48 @@ func (w *ThumbnailWorkflow) Head(ctx context.Context, tr *requests.ThumbnailRequ
return err
}
_, err = w.statSource(ctx, ref, auth, logger)
return err
}
// statSource stats the source and reports whether a thumbnail may be made of it
// at all, which is the same question for a HEAD and for a generation.
func (w *ThumbnailWorkflow) statSource(ctx context.Context, ref *providerv1beta1.Reference, auth string, logger log.Logger) (*providerv1beta1.ResourceInfo, error) {
statRsp, err := w.stater.Stat(ctx, ref, auth)
if err != nil {
logger.Debug().Err(err).Msg("could not stat file")
return fmt.Errorf("stat: %w", err)
return nil, fmt.Errorf("stat: %w", err)
}
info := statRsp.GetInfo()
if fileIsProcessing(info) {
return ErrFileProcessing
return nil, ErrFileProcessing
}
if !info.GetPermissionSet().GetInitiateFileDownload() {
return ErrPermissionDenied
return nil, ErrPermissionDenied
}
if !w.sourceTypes.Supports(info.GetMimeType()) {
return fmt.Errorf("%w: %s", ErrUnsupportedFileType, info.GetMimeType())
return nil, fmt.Errorf("%w: %s", ErrUnsupportedFileType, info.GetMimeType())
}
if w.maxInputSize > 0 && info.GetSize() > w.maxInputSize {
logger.Debug().Uint64("size", info.GetSize()).Uint64("max", w.maxInputSize).Msg("file too large for thumbnail")
return ErrImageTooLarge
return nil, ErrImageTooLarge
}
return nil
return info, nil
}
// resolveReference turns the request's reference into a fully-anchored CS3
// reference, mirroring reva's ocdav split of space-ID-based requests from
// path-only requests:
// - If Ref carries a ResourceId (dav/spaces/{id}/...), it is used directly —
// - If Ref carries a ResourceId (dav/spaces/{id}/...), it is used directly,
// the URL already identifies the space root.
// - Otherwise (path-only request) the absolute CS3 path is resolved to the
// user's home-relative path and, when a SpaceLookup is configured, the
@@ -403,29 +404,9 @@ func (w *ThumbnailWorkflow) absolutizeUserPath(ctx context.Context, p, identifie
}
func (w *ThumbnailWorkflow) generate(ctx context.Context, ref *providerv1beta1.Reference, auth string, tr *requests.ThumbnailRequest, logger log.Logger) (Thumbnail, error) {
statRsp, err := w.stater.Stat(ctx, ref, auth)
info, err := w.statSource(ctx, ref, auth, logger)
if err != nil {
logger.Debug().Err(err).Msg("could not stat file")
return Thumbnail{}, fmt.Errorf("stat: %w", err)
}
info := statRsp.GetInfo()
if fileIsProcessing(info) {
return Thumbnail{}, ErrFileProcessing
}
if !info.GetPermissionSet().GetInitiateFileDownload() {
return Thumbnail{}, ErrPermissionDenied
}
if !w.sourceTypes.Supports(info.GetMimeType()) {
return Thumbnail{}, fmt.Errorf("%w: %s", ErrUnsupportedFileType, info.GetMimeType())
}
if w.maxInputSize > 0 && info.GetSize() > w.maxInputSize {
logger.Debug().Uint64("size", info.GetSize()).Uint64("max", w.maxInputSize).Msg("file too large for thumbnail")
return Thumbnail{}, ErrImageTooLarge
return Thumbnail{}, err
}
checksum := info.GetChecksum().GetSum()
@@ -467,7 +448,7 @@ func (w *ThumbnailWorkflow) generate(ctx context.Context, ref *providerv1beta1.R
// requests that produce the same image share one entry and the number of
// entries per file stays bounded by the ladder. It needs nothing but the
// request, so a hit still returns without touching storage.
cacheKey := thumbnailStorageKey(checksum, box.Dx(), box.Dy(), outputFormat, processorID(operation))
cacheKey := thumbnailStorageKey(checksum, box.Dx(), box.Dy(), outputFormat, operation)
if w.cache != nil {
if cached, err := w.cache.Get(cacheKey); err == nil {
@@ -499,8 +480,10 @@ func (w *ThumbnailWorkflow) generate(ctx context.Context, ref *providerv1beta1.R
}
if thumbType != "" && thumbType != outputType {
// the answer is always the type we cache under, so the same request
// cannot report one type now and another one after a cache hit
logger.Debug().Str("asked", outputType).Str("answered", thumbType).
// cannot report one type now and another one after a cache hit. A
// generator that writes something else than it was asked for means the
// configured formats do not match it, which an operator has to fix.
logger.Warn().Str("asked", outputType).Str("answered", thumbType).
Msg("generator wrote a type we did not ask for")
}
@@ -569,10 +552,9 @@ func (w *ThumbnailWorkflow) sourceImage(ctx context.Context, ref *providerv1beta
// - (none), gif -> stretch (resize for gifs)
// - (none), non-gif -> fill (default = thumbnail = fill)
//
// "thumbnail" and "fit" are legacy aliases. They map onto the imagor operations
// above but IGNORE the legacy "a" and "scalingup" query parameters — callers that
// need aspect/upscaling control should use the imagor names directly ("fill",
// "fit-in", "stretch"). The second return value reports whether upscaling must be
// "thumbnail" and "fit" are legacy aliases for "fill" and "fit-in". Naming any
// processor decides the behavior on its own, so "a" is overruled and "scalingup"
// is ignored either way. The second return value reports whether upscaling must be
// suppressed (no_upscale): only fit-in never enlarges its source, so it is true
// exactly for the fit/fit-in processors; fill, thumbnail and resize all allow the
// generator to enlarge a small source to fill the snapped box. The third return
@@ -29,6 +29,7 @@ import (
"github.com/stretchr/testify/mock"
"google.golang.org/grpc"
"github.com/opencloud-eu/opencloud/pkg/conversions"
"github.com/opencloud-eu/opencloud/pkg/log"
"github.com/opencloud-eu/opencloud/services/webdav/pkg/constants"
"github.com/opencloud-eu/opencloud/services/webdav/pkg/dav/requests"
@@ -55,24 +56,6 @@ func spaceRef(spaceID, relPath string) *providerv1beta1.Reference {
}
}
// ptr is the address of a literal, for the optional flags on a request.
func ptr[T any](v T) *T { return &v }
var _ = Describe("processorID", func() {
It("maps the default (empty) processor to the thumbnail id", func() {
Expect(processorID("")).To(Equal("thumbnail"))
})
It("maps an explicit thumbnail processor to the same id as the default", func() {
Expect(processorID("thumbnail")).To(Equal("thumbnail"))
})
It("gives opt-in processors their own id so they cache separately", func() {
Expect(processorID("fit")).To(Equal("fit"))
Expect(processorID("fill")).To(Equal("fill"))
})
})
var _ = Describe("matchOperation", func() {
w := &ThumbnailWorkflow{}
@@ -139,29 +122,29 @@ var _ = Describe("matchOperation", func() {
It("reports aIgnored when an unconstrained axis overrules a=0", func() {
op, _, aIgnored := w.matchOperation(
&requests.ThumbnailRequest{Width: 0, Height: 512, Aspect: ptr(false)}, "image/jpeg")
&requests.ThumbnailRequest{Width: 0, Height: 512, Aspect: conversions.ToPointer(false)}, "image/jpeg")
Expect(op).To(Equal(generator.OpFitIn))
Expect(aIgnored).To(BeTrue())
})
It("reports aIgnored when an explicit processor overrides the legacy a flag", func() {
// a=0 (Aspect=false) wants fill; fit processor overrides it with fit-in.
op, _, aIgnored := w.matchOperation(&requests.ThumbnailRequest{Processor: "fit", Aspect: ptr(false)}, "image/jpeg")
op, _, aIgnored := w.matchOperation(&requests.ThumbnailRequest{Processor: "fit", Aspect: conversions.ToPointer(false)}, "image/jpeg")
Expect(op).To(Equal(generator.OpFitIn))
Expect(aIgnored).To(BeTrue())
// a=1 (Aspect=true) wants fit-in; fill processor overrides it with fill.
op, _, aIgnored = w.matchOperation(&requests.ThumbnailRequest{Processor: "fill", Aspect: ptr(true)}, "image/jpeg")
op, _, aIgnored = w.matchOperation(&requests.ThumbnailRequest{Processor: "fill", Aspect: conversions.ToPointer(true)}, "image/jpeg")
Expect(op).To(Equal(generator.OpFill))
Expect(aIgnored).To(BeTrue())
// a=0 wants fill; thumbnail processor implies fill -> no contradiction.
op, _, aIgnored = w.matchOperation(&requests.ThumbnailRequest{Processor: "thumbnail", Aspect: ptr(false)}, "image/jpeg")
op, _, aIgnored = w.matchOperation(&requests.ThumbnailRequest{Processor: "thumbnail", Aspect: conversions.ToPointer(false)}, "image/jpeg")
Expect(op).To(Equal(generator.OpFill))
Expect(aIgnored).To(BeFalse())
// resize is neutral: never reports a contradiction.
_, _, aIgnored = w.matchOperation(&requests.ThumbnailRequest{Processor: "resize", Aspect: ptr(false)}, "image/jpeg")
_, _, aIgnored = w.matchOperation(&requests.ThumbnailRequest{Processor: "resize", Aspect: conversions.ToPointer(false)}, "image/jpeg")
Expect(aIgnored).To(BeFalse())
})
})
+1
View File
@@ -88,6 +88,7 @@ export WEB_DEBUG_ADDR=localhost:10104
export WEB_HTTP_ADDR=localhost:10100
export WEBDAV_DEBUG_ADDR=127.0.0.1:10119
export WEBDAV_HTTP_ADDR=127.0.0.1:10115
export WEBDAV_THUMBNAIL_GENERATOR_URL=http://127.0.0.1:10190
export WEBFINGER_DEBUG_ADDR=127.0.0.1:10279
export WEBFINGER_HTTP_ADDR=127.0.0.1:10275
export OC_SERVICE_ACCOUNT_ID=serviceaccount