diff --git a/services/thumbnails/README.md b/services/thumbnails/README.md index a6a3b356c9..af9af79c82 100644 --- a/services/thumbnails/README.md +++ b/services/thumbnails/README.md @@ -65,10 +65,12 @@ its first frame, and a still source asked for `format(gif)` is palettised. Reading an ICO needs libvips with its magick loader; the pure Go build answers `406` for one. -The answers are imagor's: `400` for a malformed request, `406` for a source it -cannot read, for a format this build cannot write and for an upload part that -is not typed `image/*`, `422` for an image above the configured maximum -dimensions, `429` when the concurrency limit is reached. +The answers are imagor's: `400` for a malformed request, which includes a +`format` nobody writes (anything but `jpg`, `png`, `gif` and `webp`); `406` for a +source it cannot read, for an upload part that is not typed `image/*`, and for a +format this build has no encoder for (`webp` without libvips); `422` for an image +above the configured maximum dimensions; `429` when the concurrency limit is +reached. ## Configuration diff --git a/services/thumbnails/pkg/service/http/v0/push.go b/services/thumbnails/pkg/service/http/v0/push.go index da79f01f70..c6b7f9a79d 100644 --- a/services/thumbnails/pkg/service/http/v0/push.go +++ b/services/thumbnails/pkg/service/http/v0/push.go @@ -13,8 +13,8 @@ import ( "github.com/go-chi/chi/v5" ) -// gifMagic is the leading signature of a GIF file (GIF87a or GIF89a). It guards -// against gif.DecodeAll panicking on non-GIF input. +// gifMagic is the leading signature of a GIF file (GIF87a or GIF89a). It decides +// whether a request takes the animation path. var gifMagic = []byte("GIF8") // Operations understood by the push endpoint. They map to imagor's resize/crop @@ -36,9 +36,11 @@ var errFormatUnsupported = fmt.Errorf("format not supported by this build") // // The filters segment is captured whole and parsed by outputFormatFromFilters; only the // format filter is meaningful to this executor, other filters are ignored. When no format -// filter is present the input's own format is preserved (detected via imaging), matching -// imagor. It returns only imagor's defined status codes: 400 for invalid requests or a -// file exceeding the max size, 422 for an image exceeding the max resolution. +// filter is present the input's own format is preserved, as imagor does it, and the +// backend that will decode the image says which format that is. The answers are imagor's: +// 400 for a malformed request, 406 for a source it cannot read or a format it cannot +// write, 422 for an image above the configured maximum dimensions, 429 when the +// concurrency limit is reached. func (s Thumbnails) pushHandler(w http.ResponseWriter, r *http.Request) { ok, retryAfter := s.limiter.acquire() if !ok { @@ -217,7 +219,7 @@ func (s Thumbnails) serveAnimation(w http.ResponseWriter, imgData []byte, req re } var buf bytes.Buffer - if err := gif.EncodeAll(&buf, resizeGIF(g, req.Width, req.Height, req.Operation, req.NoUpscale)); err != nil { + if err := gif.EncodeAll(&buf, resizeGIF(g, req)); err != nil { s.logger.Error().Err(err).Msg("could not encode the gif") w.WriteHeader(http.StatusInternalServerError) fmt.Fprintln(w, "failed to encode image") diff --git a/services/thumbnails/pkg/service/http/v0/push_imaging.go b/services/thumbnails/pkg/service/http/v0/push_imaging.go index 744f46477e..9743c22634 100644 --- a/services/thumbnails/pkg/service/http/v0/push_imaging.go +++ b/services/thumbnails/pkg/service/http/v0/push_imaging.go @@ -15,8 +15,8 @@ import ( ) // The backend this build resizes with. The implementations keep their own names -// so both builds' versions stay reachable by symbol, the variables are what the -// shared handler calls. +// so a symbol search finds both builds' versions side by side, the variables are +// what the shared handler calls. var ( loadSource = loadSourceImaging extForSourceFormat = extForSourceFormatImaging diff --git a/services/thumbnails/pkg/service/http/v0/push_vips.go b/services/thumbnails/pkg/service/http/v0/push_vips.go index 00684b2ebc..d407354c0c 100644 --- a/services/thumbnails/pkg/service/http/v0/push_vips.go +++ b/services/thumbnails/pkg/service/http/v0/push_vips.go @@ -11,8 +11,8 @@ import ( ) // The backend this build resizes with. The implementations keep their own names -// so both builds' versions stay reachable by symbol, the variables are what the -// shared handler calls. +// so a symbol search finds both builds' versions side by side, the variables are +// what the shared handler calls. // // libvips is demand-driven: loading a buffer parses the header and computes // pixels only when something pulls them, so the dimension guard has its say diff --git a/services/thumbnails/pkg/service/http/v0/resizegif.go b/services/thumbnails/pkg/service/http/v0/resizegif.go index 50e3f44dfd..71adb5ae06 100644 --- a/services/thumbnails/pkg/service/http/v0/resizegif.go +++ b/services/thumbnails/pkg/service/http/v0/resizegif.go @@ -14,8 +14,9 @@ import ( // imaging either way, because neither libvips nor the pure Go path has anything // better for a paletted animation. Code adapted from // https://github.com/willnorris/gifresize. -func resizeGIF(m *gif.GIF, width, height int, operation string, noUpscale bool) *gif.GIF { +func resizeGIF(m *gif.GIF, req resizeRequest) *gif.GIF { srcX, srcY := m.Config.Width, m.Config.Height + width, height, operation, noUpscale := req.Width, req.Height, req.Operation, req.NoUpscale if width == 0 || height == 0 { resolvedW, resolvedH, resize := autoAxis(srcX, srcY, width, height) diff --git a/services/webdav/pkg/config/defaults/defaultconfig.go b/services/webdav/pkg/config/defaults/defaultconfig.go index 962b24522d..783419590d 100644 --- a/services/webdav/pkg/config/defaults/defaultconfig.go +++ b/services/webdav/pkg/config/defaults/defaultconfig.go @@ -1,10 +1,8 @@ package defaults import ( - "path" "strings" - cored "github.com/opencloud-eu/opencloud/pkg/config/defaults" "github.com/opencloud-eu/opencloud/pkg/shared" "github.com/opencloud-eu/opencloud/pkg/structs" "github.com/opencloud-eu/opencloud/services/webdav/pkg/config" @@ -49,7 +47,6 @@ func DefaultConfig() *config.Config { ThumbnailGeneratorTimeout: "30s", ThumbnailMaxInputFileSize: "50MB", ThumbnailCacheBackend: "file", - ThumbnailCacheDir: path.Join(cored.BaseDataPath(), "thumbnails", "files"), ThumbnailResolutions: []string{ "16", "32", "64", "128", "192", "256", "320", "384", "448", "512", "640", "768", "1024", "1280", "1920", "2560", "3840", "7680", @@ -78,4 +75,10 @@ func Sanitize(cfg *config.Config) { if cfg.HTTP.Root != "/" { cfg.HTTP.Root = strings.TrimSuffix(cfg.HTTP.Root, "/") } + + // a yaml configuration that joined the resolutions into one entry kept + // working in the thumbnails service, so it keeps working here + if len(cfg.ThumbnailResolutions) == 1 && strings.Contains(cfg.ThumbnailResolutions[0], ",") { + cfg.ThumbnailResolutions = strings.Split(cfg.ThumbnailResolutions[0], ",") + } } diff --git a/services/webdav/pkg/dav/requests/thumbnail.go b/services/webdav/pkg/dav/requests/thumbnail.go index 2f71446220..086865763d 100644 --- a/services/webdav/pkg/dav/requests/thumbnail.go +++ b/services/webdav/pkg/dav/requests/thumbnail.go @@ -181,9 +181,11 @@ func aspect(q url.Values) *bool { // unconstrained axis cannot describe. Only fit-in inscribes instead. func targetsExactBox(processor string) bool { switch strings.ToLower(strings.TrimSpace(processor)) { - case "fit", "fit-in": - return false - default: + case "fill", "thumbnail", "resize": return true + default: + // fit, fit-in, and any name nobody knows, which the workflow resolves + // the same way it resolves a request without a processor + return false } } diff --git a/services/webdav/pkg/dav/requests/thumbnail_test.go b/services/webdav/pkg/dav/requests/thumbnail_test.go index 531ee79711..35b2d19a02 100644 --- a/services/webdav/pkg/dav/requests/thumbnail_test.go +++ b/services/webdav/pkg/dav/requests/thumbnail_test.go @@ -55,6 +55,7 @@ var _ = Describe("ParseThumbnailRequest", func() { 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)), + Entry("treats a processor nobody knows like none at all", "y=512&processor=bogus", int32(0), int32(512)), ) DescribeTable("a request it refuses", diff --git a/services/webdav/pkg/service/v0/previews_test.go b/services/webdav/pkg/service/v0/previews_test.go new file mode 100644 index 0000000000..3dde2a43d4 --- /dev/null +++ b/services/webdav/pkg/service/v0/previews_test.go @@ -0,0 +1,27 @@ +package svc + +import ( + "net/http" + "net/http/httptest" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/opencloud-eu/opencloud/pkg/log" + "github.com/opencloud-eu/opencloud/services/webdav/pkg/config" +) + +var _ = Describe("with previews disabled", func() { + // the routes stay registered, so a missing one cannot let the REPORT + // patterns answer a thumbnail request with 405 + It("answers a thumbnail request with 404 before anything is parsed", func() { + g := Webdav{config: &config.Config{DisablePreviews: true}} + rec := httptest.NewRecorder() + + tr, ok := g.parseThumbnailRequest(rec, httptest.NewRequest(http.MethodGet, "/dav/files/einstein/photo.png?x=32&y=32&preview=1", nil), log.NopLogger()) + + Expect(ok).To(BeFalse()) + Expect(tr).To(BeNil()) + Expect(rec.Code).To(Equal(http.StatusNotFound)) + }) +}) diff --git a/services/webdav/pkg/service/v0/service.go b/services/webdav/pkg/service/v0/service.go index 7c91cfdb26..34b96b6d13 100644 --- a/services/webdav/pkg/service/v0/service.go +++ b/services/webdav/pkg/service/v0/service.go @@ -111,17 +111,15 @@ func NewService(opts ...Option) (Service, error) { r.Group(func(r chi.Router) { r.Use(svc.DavUserContext()) - if !conf.DisablePreviews { - 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/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) - r.Get("/dav/files/{id}", svc.Thumbnail) - r.Get("/dav/files/{id}/*", svc.Thumbnail) - } + r.Get("/remote.php/dav/files/{id}", svc.Thumbnail) + r.Get("/remote.php/dav/files/{id}/*", svc.Thumbnail) + r.Get("/dav/files/{id}", svc.Thumbnail) + r.Get("/dav/files/{id}/*", svc.Thumbnail) r.MethodFunc("REPORT", "/remote.php/dav/spaces*", svc.Search) r.MethodFunc("REPORT", "/dav/spaces*", svc.Search) @@ -129,25 +127,21 @@ func NewService(opts ...Option) (Service, error) { r.MethodFunc("REPORT", "/dav/files*", svc.Search) }) - if !conf.DisablePreviews { - r.Group(func(r chi.Router) { - r.Use(svc.DavPublicContext()) + r.Group(func(r chi.Router) { + r.Use(svc.DavPublicContext()) - r.Head("/remote.php/dav/public-files/{token}/*", svc.PublicThumbnailHead) - r.Head("/dav/public-files/{token}/*", svc.PublicThumbnailHead) + r.Head("/remote.php/dav/public-files/{token}/*", svc.PublicThumbnailHead) + r.Head("/dav/public-files/{token}/*", svc.PublicThumbnailHead) - r.Get("/remote.php/dav/public-files/{token}/*", svc.PublicThumbnail) - r.Get("/dav/public-files/{token}/*", svc.PublicThumbnail) - }) - } + r.Get("/remote.php/dav/public-files/{token}/*", svc.PublicThumbnail) + r.Get("/dav/public-files/{token}/*", svc.PublicThumbnail) + }) r.Group(func(r chi.Router) { r.Use(svc.WebDAVContext()) - if !conf.DisablePreviews { - r.Get("/remote.php/webdav/*", svc.Thumbnail) - r.Get("/webdav/*", svc.Thumbnail) - } + r.Get("/remote.php/webdav/*", svc.Thumbnail) + r.Get("/webdav/*", svc.Thumbnail) r.MethodFunc("REPORT", "/remote.php/webdav*", svc.Search) r.MethodFunc("REPORT", "/webdav*", svc.Search) @@ -201,6 +195,8 @@ func newThumbnailWorkflow(conf *config.Config, sourceTypes thumbnail.SourceTypes conf.ThumbnailCacheS3SecretKey, ) + // the new setting wins, the deprecated one is the fallback, and an empty + // result lets the cache use its own default directory cacheDir := conf.ThumbnailCacheDir if cacheDir == "" { cacheDir = conf.ThumbnailFilesystemStorageRoot @@ -336,6 +332,15 @@ func (g Webdav) requestLogger(r *http.Request) log.Logger { // parseThumbnailRequest reads the request every thumbnail endpoint starts with. // It reports false when it already answered with a 400. func (g Webdav) parseThumbnailRequest(w http.ResponseWriter, r *http.Request, logger log.Logger) (*requests.ThumbnailRequest, bool) { + if g.config.DisablePreviews { + // every thumbnail path answers 404, which is what the setting promises. + // The routes stay registered: the REPORT patterns match the same paths, + // so without them chi would answer a thumbnail request with 405. + http.NotFound(w, r) + + return nil, false + } + tr, err := requests.ParseThumbnailRequest(r) if err != nil { logger.Debug().Err(err).Msg("could not create Request")