From de467d7dd87d150364112eecada48e7ca57b95e1 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Sat, 17 Oct 2026 00:54:26 +0200 Subject: [PATCH] feat(webdav): write a thumbnail in the type the source maps to A source type maps onto the type its thumbnail is returned in, and that type is asked for by the name the generator knows it under. The answer carries that type whether it was just generated or came from the cache, which is also what the cache key says. --- services/webdav/pkg/config/config.go | 3 + .../pkg/config/defaults/defaultconfig.go | 4 + services/webdav/pkg/dav/requests/thumbnail.go | 2 - services/webdav/pkg/generator/extensions.go | 70 ---- .../pkg/generator/generator_suite_test.go | 13 + .../webdav/pkg/generator/generator_test.go | 232 ++++-------- services/webdav/pkg/generator/handler.go | 6 +- services/webdav/pkg/generator/url.go | 53 ++- services/webdav/pkg/preprocessor/errors.go | 4 +- services/webdav/pkg/service/v0/service.go | 14 +- .../pkg/thumbnail/thumbnail_suite_test.go | 13 + services/webdav/pkg/thumbnail/typemapping.go | 56 +++ .../webdav/pkg/thumbnail/typemapping_test.go | 61 +++ .../pkg/thumbnail/workflow/encode_imaging.go | 26 +- .../pkg/thumbnail/workflow/encode_vips.go | 18 +- .../webdav/pkg/thumbnail/workflow/workflow.go | 81 ++-- .../pkg/thumbnail/workflow/workflow_test.go | 347 ++++++++++-------- 17 files changed, 554 insertions(+), 449 deletions(-) delete mode 100644 services/webdav/pkg/generator/extensions.go create mode 100644 services/webdav/pkg/generator/generator_suite_test.go create mode 100644 services/webdav/pkg/thumbnail/thumbnail_suite_test.go create mode 100644 services/webdav/pkg/thumbnail/typemapping.go create mode 100644 services/webdav/pkg/thumbnail/typemapping_test.go diff --git a/services/webdav/pkg/config/config.go b/services/webdav/pkg/config/config.go index 0c0e5c1645..3cc9f8d242 100644 --- a/services/webdav/pkg/config/config.go +++ b/services/webdav/pkg/config/config.go @@ -39,6 +39,9 @@ type Config struct { ThumbnailCacheS3Endpoint string `yaml:"thumbnail_cache_s3_endpoint" env:"WEBDAV_THUMBNAIL_CACHE_S3_ENDPOINT" desc:"S3 endpoint URL (for MinIO or compatible services) when ThumbnailCacheBackend is 's3'." introductionVersion:"%%NEXT%%"` ThumbnailCacheS3AccessKey string `yaml:"thumbnail_cache_s3_access_key" env:"WEBDAV_THUMBNAIL_CACHE_S3_ACCESS_KEY" desc:"S3 access key for thumbnail cache authentication." introductionVersion:"%%NEXT%%"` ThumbnailCacheS3SecretKey string `yaml:"thumbnail_cache_s3_secret_key" env:"WEBDAV_THUMBNAIL_CACHE_S3_SECRET_KEY" desc:"S3 secret key for thumbnail cache authentication." introductionVersion:"%%NEXT%%"` + ThumbnailGeneratorFormats []string `yaml:"thumbnail_generator_formats" env:"OC_THUMBNAIL_GENERATOR_FORMATS;WEBDAV_THUMBNAIL_GENERATOR_FORMATS" desc:"The formats the thumbnail generator writes, as mime:format entries, e.g. image/webp:webp. The format is the name the generator's own format filter uses. A type that is not listed is one the generator cannot write, and a thumbnail mapped to it becomes a JPEG instead." introductionVersion:"%%NEXT%%"` + ThumbnailTypeMapping []string `yaml:"thumbnail_type_mapping" env:"OC_THUMBNAIL_TYPE_MAPPING;WEBDAV_THUMBNAIL_TYPE_MAPPING" desc:"The type a thumbnail is written 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%%"` diff --git a/services/webdav/pkg/config/defaults/defaultconfig.go b/services/webdav/pkg/config/defaults/defaultconfig.go index 962b24522d..8932a30f2c 100644 --- a/services/webdav/pkg/config/defaults/defaultconfig.go +++ b/services/webdav/pkg/config/defaults/defaultconfig.go @@ -8,6 +8,8 @@ import ( "github.com/opencloud-eu/opencloud/pkg/shared" "github.com/opencloud-eu/opencloud/pkg/structs" "github.com/opencloud-eu/opencloud/services/webdav/pkg/config" + "github.com/opencloud-eu/opencloud/services/webdav/pkg/generator" + "github.com/opencloud-eu/opencloud/services/webdav/pkg/thumbnail" ) // FullDefaultConfig returns a fully initialized default configuration @@ -48,6 +50,8 @@ func DefaultConfig() *config.Config { ThumbnailGeneratorURL: "http://127.0.0.1:9186", ThumbnailGeneratorTimeout: "30s", ThumbnailMaxInputFileSize: "50MB", + ThumbnailGeneratorFormats: generator.DefaultFormats, + ThumbnailTypeMapping: thumbnail.DefaultTypeMapping, ThumbnailCacheBackend: "file", ThumbnailCacheDir: path.Join(cored.BaseDataPath(), "thumbnails", "files"), ThumbnailResolutions: []string{ diff --git a/services/webdav/pkg/dav/requests/thumbnail.go b/services/webdav/pkg/dav/requests/thumbnail.go index 9f97971231..e81c803e88 100644 --- a/services/webdav/pkg/dav/requests/thumbnail.go +++ b/services/webdav/pkg/dav/requests/thumbnail.go @@ -38,7 +38,6 @@ type ThumbnailRequest struct { // The file name of the source file including the extension Filename string // The file extension - Extension string // The requested width of the thumbnail Width int32 // The requested height of the thumbnail @@ -114,7 +113,6 @@ func ParseThumbnailRequest(r *http.Request) (*ThumbnailRequest, error) { return &ThumbnailRequest{ Ref: ref, Filename: filepath.Base(fp), - Extension: filepath.Ext(fp), Width: int32(width), Height: int32(height), Processor: q.Get("processor"), diff --git a/services/webdav/pkg/generator/extensions.go b/services/webdav/pkg/generator/extensions.go deleted file mode 100644 index 3cfbf16db2..0000000000 --- a/services/webdav/pkg/generator/extensions.go +++ /dev/null @@ -1,70 +0,0 @@ -package generator - -import ( - "mime" - "strings" -) - -// ExtensionInfo holds the output format and content type for a given file extension. -type ExtensionInfo struct { - OutputFormat string // generator output format (gif, png, jpeg) - ContentType string // HTTP Content-Type header value -} - -var extMap = map[string]ExtensionInfo{ - "gif": {OutputFormat: "gif", ContentType: "image/gif"}, - "png": {OutputFormat: "png", ContentType: "image/png"}, - "jpeg": {OutputFormat: "jpeg", ContentType: "image/jpeg"}, - "jpg": {OutputFormat: "jpeg", ContentType: "image/jpeg"}, -} - -var defaultExtInfo = ExtensionInfo{OutputFormat: "jpeg", ContentType: "image/jpeg"} - -// GetExtensionInfo returns the extension info for a given file extension. -func GetExtensionInfo(ext string) ExtensionInfo { - if info, ok := extMap[strings.ToLower(strings.TrimLeft(ext, "."))]; ok { - return info - } - return defaultExtInfo -} - -// OutputFormat returns the generator output format for a given file extension. -func OutputFormat(ext string) string { - return GetExtensionInfo(ext).OutputFormat -} - -// ContentType returns the HTTP Content-Type for a given file extension. -func ContentType(ext string) string { - return GetExtensionInfo(ext).ContentType -} - -// MimeToExt maps a resource mime type to the file extension used for the on-disk -// thumbnail cache key. Unknown image types fall back to jpg, matching main's -// behavior (only png/gif are treated specially). -func MimeToExt(mimeType string) string { - switch mimeType { - case "image/png": - return "png" - case "image/gif": - return "gif" - default: - return "jpg" - } -} - -// ExtForMime mirrors main's GetExtForMime: returns the output extension for a -// source mime, or "" when the source has no dedicated image type (txt, audio, -// bmp, tiff) so the caller falls back to the requested extension. -func ExtForMime(mimeType string) string { - m, _, _ := mime.ParseMediaType(mimeType) - switch m { - case "image/jpeg", "image/webp": - return "jpg" - case "image/png": - return "png" - case "image/gif": - return "gif" - default: - return "" - } -} diff --git a/services/webdav/pkg/generator/generator_suite_test.go b/services/webdav/pkg/generator/generator_suite_test.go new file mode 100644 index 0000000000..23e617d83a --- /dev/null +++ b/services/webdav/pkg/generator/generator_suite_test.go @@ -0,0 +1,13 @@ +package generator_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestGenerator(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Generator Suite") +} diff --git a/services/webdav/pkg/generator/generator_test.go b/services/webdav/pkg/generator/generator_test.go index 093342993f..7385ecf611 100644 --- a/services/webdav/pkg/generator/generator_test.go +++ b/services/webdav/pkg/generator/generator_test.go @@ -1,168 +1,84 @@ -package generator +package generator_test import ( - "testing" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/opencloud-eu/opencloud/services/webdav/pkg/generator" ) -func TestParseMaxInputFileSize(t *testing.T) { - tests := []struct { - name string - input string - want uint64 - wantErr bool - }{ - {"empty means no limit", "", 0, false}, - {"plain bytes", "1234", 1234, false}, - {"KB is decimal", "50KB", 50_000, false}, - {"KiB is binary", "50KiB", 50 * 1024, false}, - {"MB is decimal", "5MB", 5_000_000, false}, - {"MiB is binary", "5MiB", 5 * 1024 * 1024, false}, - {"GB is decimal", "2GB", 2_000_000_000, false}, - {"GiB is binary", "2GiB", 2 * 1024 * 1024 * 1024, false}, - {"TB", "2TB", 2_000_000_000_000, false}, - {"whitespace around value", " 5MB ", 5_000_000, false}, - {"space before suffix", "5 MB", 5_000_000, false}, - {"lowercase unit", "50kB", 0, true}, - {"invalid input", "abc", 0, true}, - {"negative number", "-5MB", 0, true}, - } +var _ = Describe("ParseMaxInputFileSize", func() { + DescribeTable("reads a size the way the thumbnails service did", + func(input string, want uint64, wantErr bool) { + got, err := generator.ParseMaxInputFileSize(input) + if wantErr { + Expect(err).To(HaveOccurred()) - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := ParseMaxInputFileSize(tt.input) - if (err != nil) != tt.wantErr { - t.Errorf("ParseMaxInputFileSize() error = %v, wantErr %v", err, tt.wantErr) return } - if got != tt.want { - t.Errorf("ParseMaxInputFileSize() = %v, want %v", got, tt.want) - } - }) - } -} -func TestGetExtensionInfo(t *testing.T) { - tests := []struct { - ext string - format string - ct string - }{ - {"gif", "gif", "image/gif"}, - {".gif", "gif", "image/gif"}, - {"png", "png", "image/png"}, - {"jpeg", "jpeg", "image/jpeg"}, - {"jpg", "jpeg", "image/jpeg"}, - {"webp", "jpeg", "image/jpeg"}, - {"", "jpeg", "image/jpeg"}, - } - - for _, tt := range tests { - t.Run(tt.ext, func(t *testing.T) { - info := GetExtensionInfo(tt.ext) - if info.OutputFormat != tt.format { - t.Errorf("OutputFormat = %q, want %q", info.OutputFormat, tt.format) - } - if info.ContentType != tt.ct { - t.Errorf("ContentType = %q, want %q", info.ContentType, tt.ct) - } - }) - } -} - -func TestOutputFormat(t *testing.T) { - if got := OutputFormat("png"); got != "png" { - t.Errorf("OutputFormat(\"png\") = %q, want \"png\"", got) - } - if got := OutputFormat("jpg"); got != "jpeg" { - t.Errorf("OutputFormat(\"jpg\") = %q, want \"jpeg\"", got) - } -} - -func TestContentType(t *testing.T) { - if got := ContentType("gif"); got != "image/gif" { - t.Errorf("ContentType(\"gif\") = %q, want \"image/gif\"", got) - } - if got := ContentType("unknown"); got != "image/jpeg" { - t.Errorf("ContentType(\"unknown\") = %q, want \"image/jpeg\"", got) - } -} - -func TestMimeToExt(t *testing.T) { - tests := []struct { - mime string - want string - }{ - {"image/jpeg", "jpg"}, - {"image/png", "png"}, - {"image/gif", "gif"}, - {"application/octet-stream", "jpg"}, - } - - for _, tt := range tests { - t.Run(tt.mime, func(t *testing.T) { - if got := MimeToExt(tt.mime); got != tt.want { - t.Errorf("MimeToExt(%q) = %q, want %q", tt.mime, got, tt.want) - } - }) - } -} - -func TestBuildURL(t *testing.T) { - tests := []struct { - name string - base string - width int32 - height int32 - operation string - ext string - noUpscale bool - want string - }{ - { - name: "fill operation center-crops to the box", - base: "http://generator", - width: 128, - height: 128, - operation: OpFill, - ext: "jpeg", - want: "http://generator/unsafe/128x128/filters:format(jpeg)/", + Expect(err).ToNot(HaveOccurred()) + Expect(got).To(Equal(want)) }, - { - name: "fill with no_upscale filter caps the resize at source size", - base: "http://generator", - width: 128, - height: 128, - operation: OpFill, - ext: "jpeg", - noUpscale: true, - want: "http://generator/unsafe/128x128/filters:no_upscale():format(jpeg)/", - }, - { - name: "fit-in preserves aspect ratio within the box without upscaling", - base: "http://generator", - width: 1024, - height: 1024, - operation: OpFitIn, - ext: "jpeg", - want: "http://generator/unsafe/fit-in/1024x1024/filters:format(jpeg)/", - }, - { - name: "stretch resizes to the exact box", - base: "http://generator/", - width: 64, - height: 32, - operation: OpStretch, - ext: "png", - want: "http://generator/unsafe/stretch/64x32/filters:format(png)/", - }, - } + Entry("empty means no limit", "", uint64(0), false), + Entry("plain bytes", "1234", uint64(1234), false), + Entry("KB is decimal", "50KB", uint64(50_000), false), + Entry("KiB is binary", "50KiB", uint64(50*1024), false), + Entry("MB is decimal", "5MB", uint64(5_000_000), false), + Entry("MiB is binary", "5MiB", uint64(5*1024*1024), false), + Entry("GB is decimal", "2GB", uint64(2_000_000_000), false), + Entry("GiB is binary", "2GiB", uint64(2*1024*1024*1024), false), + Entry("TB", "2TB", uint64(2_000_000_000_000), false), + Entry("whitespace around the value", " 5MB ", uint64(5_000_000), false), + Entry("space before the suffix", "5 MB", uint64(5_000_000), false), + Entry("a lowercase unit", "50kB", uint64(0), true), + Entry("not a size", "abc", uint64(0), true), + Entry("a negative size", "-5MB", uint64(0), true), + ) +}) - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := BuildURL(tt.base, tt.width, tt.height, tt.operation, tt.ext, tt.noUpscale) - if got != tt.want { - t.Errorf("BuildURL() = %q, want %q", got, tt.want) - } - }) - } -} +var _ = Describe("Formats", func() { + DescribeTable("names the format the generator's filter uses for a type", + func(contentType, want string) { + Expect(generator.NewFormats(nil).For(contentType)).To(Equal(want)) + }, + Entry("png", "image/png", "png"), + Entry("gif", "image/gif", "gif"), + Entry("jpeg", "image/jpeg", "jpg"), + Entry("a type with parameters", "image/jpeg; q=1", "jpg"), + Entry("webp, which the built-in generator cannot write", "image/webp", ""), + Entry("an icon, which no generator writes", "image/vnd.microsoft.icon", ""), + Entry("nonsense", "not a mime type", ""), + ) + + It("takes a format the built-in generator cannot write, for a generator that can", func() { + formats := generator.NewFormats([]string{" Image/WEBP : WebP "}) + + Expect(formats.For("image/webp")).To(Equal("webp")) + Expect(formats.For("image/gif")).To(Equal("gif"), "a default nobody touched still answers") + }) + + It("takes an empty format as no format", func() { + Expect(generator.NewFormats([]string{"image/png:"}).For("image/png")).To(BeEmpty()) + }) +}) + +var _ = Describe("BuildURL", func() { + DescribeTable("renders the generator's image endpoint", + func(base string, width, height int32, operation, format string, noUpscale bool, want string) { + Expect(generator.BuildURL(base, width, height, operation, format, noUpscale)).To(Equal(want)) + }, + Entry("fill center-crops to the box", + "http://generator", int32(128), int32(128), generator.OpFill, "jpeg", false, + "http://generator/unsafe/128x128/filters:format(jpeg)/"), + Entry("fill with no_upscale caps the resize at the source size", + "http://generator", int32(128), int32(128), generator.OpFill, "jpeg", true, + "http://generator/unsafe/128x128/filters:no_upscale():format(jpeg)/"), + Entry("fit-in preserves the aspect ratio within the box", + "http://generator", int32(1024), int32(1024), generator.OpFitIn, "jpeg", false, + "http://generator/unsafe/fit-in/1024x1024/filters:format(jpeg)/"), + Entry("stretch resizes to the exact box", + "http://generator/", int32(64), int32(32), generator.OpStretch, "png", false, + "http://generator/unsafe/stretch/64x32/filters:format(png)/"), + ) +}) diff --git a/services/webdav/pkg/generator/handler.go b/services/webdav/pkg/generator/handler.go index c678b6931f..6b33cc282b 100644 --- a/services/webdav/pkg/generator/handler.go +++ b/services/webdav/pkg/generator/handler.go @@ -7,11 +7,11 @@ import ( "strconv" ) -// WriteThumbnailResponse writes the thumbnail bytes to the HTTP response with correct Content-Type. +// WriteThumbnailResponse writes the thumbnail bytes to the HTTP response. // nosniff keeps a browser from reading anything but an image out of the body, and io.Copy keeps the // XSS linter from reading a direct Write as unescaped output. -func WriteThumbnailResponse(w http.ResponseWriter, data []byte, ext string) { - w.Header().Set("Content-Type", ContentType(ext)) +func WriteThumbnailResponse(w http.ResponseWriter, data []byte, contentType string) { + w.Header().Set("Content-Type", contentType) w.Header().Set("Content-Length", strconv.Itoa(len(data))) w.Header().Set("X-Content-Type-Options", "nosniff") w.WriteHeader(http.StatusOK) diff --git a/services/webdav/pkg/generator/url.go b/services/webdav/pkg/generator/url.go index 810a7abe9a..589b63ab00 100644 --- a/services/webdav/pkg/generator/url.go +++ b/services/webdav/pkg/generator/url.go @@ -2,6 +2,7 @@ package generator import ( "fmt" + "mime" "strings" ) @@ -38,14 +39,14 @@ func operationSegment(op string) string { // an imagor filters segment. Like real imagor, the default (fill) resize upscales // small sources to fill the box exactly; pass noUpscale=true to emit the // no_upscale() filter so the generator caps the result at the source size. -func BuildURL(baseURL string, width, height int32, operation, outputExt string, noUpscale bool) string { +func BuildURL(baseURL string, width, height int32, operation, format string, noUpscale bool) string { base := strings.TrimRight(baseURL, "/") box := fmt.Sprintf("%dx%d", width, height) - filters := "format(" + outputExt + ")" + filters := "format(" + format + ")" if noUpscale { - filters = "no_upscale():format(" + outputExt + ")" + filters = "no_upscale():format(" + format + ")" } filterSegment := "filters:" + filters @@ -55,3 +56,49 @@ func BuildURL(baseURL string, width, height int32, operation, outputExt string, } return fmt.Sprintf("%s/unsafe/%s/%s/%s/", base, segment, box, filterSegment) } + +// DefaultFormats are the types the built-in generator writes, by the name its +// format filter uses for them. A generator behind the URL that writes more than +// that is told so through the configuration. +var DefaultFormats = []string{ + "image/png:png", + "image/gif:gif", + "image/jpeg:jpg", +} + +// Formats says which format the generator is asked for to get a given type. +type Formats struct { + byType map[string]string +} + +// NewFormats parses "mime:format" entries on top of DefaultFormats. +func NewFormats(entries []string) Formats { + return Formats{byType: ParsePairs(DefaultFormats, entries)} +} + +// For is the name the generator's format filter uses for a type, "" for a type +// it cannot write. It guards the configured type mapping: a target nobody can +// write leaves the caller with its default. +func (f Formats) For(contentType string) string { + m, _, err := mime.ParseMediaType(contentType) + if err != nil { + return "" + } + + return f.byType[m] +} + +// ParsePairs reads "key:value" configuration entries on top of the defaults, +// lowercased and trimmed, so one configured entry does not take the answer away +// from all the others. +func ParsePairs(defaults, entries []string) map[string]string { + out := map[string]string{} + for _, entry := range append(append([]string{}, defaults...), entries...) { + key, value, _ := strings.Cut(strings.ToLower(strings.TrimSpace(entry)), ":") + if key = strings.TrimSpace(key); key != "" { + out[key] = strings.TrimSpace(value) + } + } + + return out +} diff --git a/services/webdav/pkg/preprocessor/errors.go b/services/webdav/pkg/preprocessor/errors.go index 72625bdd2f..e3a1ec48ad 100644 --- a/services/webdav/pkg/preprocessor/errors.go +++ b/services/webdav/pkg/preprocessor/errors.go @@ -3,7 +3,7 @@ package preprocessor import "errors" var ( - ErrNoImageFromAudioFile = errors.New("preprocessor: could not extract image from audio file") - ErrNoConverterForExtractedImageFromGgsFile = errors.New("preprocessor: could not find converter for image extracted from ggs file") + ErrNoImageFromAudioFile = errors.New("preprocessor: could not extract image from audio file") + ErrNoConverterForExtractedImageFromGgsFile = errors.New("preprocessor: could not find converter for image extracted from ggs file") ErrNoConverterForExtractedImageFromAudioFile = errors.New("preprocessor: could not find converter for image extracted from audio file") ) diff --git a/services/webdav/pkg/service/v0/service.go b/services/webdav/pkg/service/v0/service.go index 5b5acf8004..bb1eda3a50 100644 --- a/services/webdav/pkg/service/v0/service.go +++ b/services/webdav/pkg/service/v0/service.go @@ -194,6 +194,8 @@ func newThumbnailWorkflow(conf *config.Config, logger log.Logger, gatewaySelecto workflow.WithHTTPClient(httpClient), workflow.WithMaxInputSize(maxInputSize), workflow.WithSizes(sizes), + workflow.WithFormats(generator.NewFormats(conf.ThumbnailGeneratorFormats)), + workflow.WithTypeMapping(thumbnail.NewTypeMapping(conf.ThumbnailTypeMapping)), workflow.WithWebdavNamespace(conf.WebdavNamespace), workflow.WithFontMapFile(conf.ThumbnailFontMapFile), workflow.WithLogger(logger), @@ -292,13 +294,13 @@ func (g Webdav) SpacesThumbnail(w http.ResponseWriter, r *http.Request) { } auth := r.Header.Get(revactx.TokenHeader) - data, ext, aIgnored, err := g.workflow.Execute(r.Context(), tr, auth, logger) + data, contentType, aIgnored, err := g.workflow.Execute(r.Context(), tr, auth, logger) if err != nil { g.handleWorkflowError(w, r, err, tr, logger) return } setAspectIgnoredHeader(w, aIgnored) - generator.WriteThumbnailResponse(w, data, ext) + generator.WriteThumbnailResponse(w, data, contentType) } func whoami(gatewayClient gatewayv1beta1.GatewayAPIClient, ctx context.Context, token string) (*userv1beta1.User, error) { @@ -325,13 +327,13 @@ func (g Webdav) Thumbnail(w http.ResponseWriter, r *http.Request) { } auth := r.Header.Get(revactx.TokenHeader) - data, ext, aIgnored, err := g.workflow.Execute(r.Context(), tr, auth, logger) + data, contentType, aIgnored, err := g.workflow.Execute(r.Context(), tr, auth, logger) if err != nil { g.handleWorkflowError(w, r, err, tr, logger) return } setAspectIgnoredHeader(w, aIgnored) - generator.WriteThumbnailResponse(w, data, ext) + generator.WriteThumbnailResponse(w, data, contentType) } func (g Webdav) PublicThumbnail(w http.ResponseWriter, r *http.Request) { @@ -349,13 +351,13 @@ func (g Webdav) PublicThumbnail(w http.ResponseWriter, r *http.Request) { return } - data, ext, aIgnored, err := g.workflow.ExecutePublic(r.Context(), tr, auth, logger) + data, contentType, aIgnored, err := g.workflow.ExecutePublic(r.Context(), tr, auth, logger) if err != nil { g.handleWorkflowError(w, r, err, tr, logger) return } setAspectIgnoredHeader(w, aIgnored) - generator.WriteThumbnailResponse(w, data, ext) + generator.WriteThumbnailResponse(w, data, contentType) } func (g Webdav) PublicThumbnailHead(w http.ResponseWriter, r *http.Request) { diff --git a/services/webdav/pkg/thumbnail/thumbnail_suite_test.go b/services/webdav/pkg/thumbnail/thumbnail_suite_test.go new file mode 100644 index 0000000000..7217b6a146 --- /dev/null +++ b/services/webdav/pkg/thumbnail/thumbnail_suite_test.go @@ -0,0 +1,13 @@ +package thumbnail_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestThumbnail(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Thumbnail Suite") +} diff --git a/services/webdav/pkg/thumbnail/typemapping.go b/services/webdav/pkg/thumbnail/typemapping.go new file mode 100644 index 0000000000..23e808815a --- /dev/null +++ b/services/webdav/pkg/thumbnail/typemapping.go @@ -0,0 +1,56 @@ +package thumbnail + +import ( + "mime" + + "github.com/opencloud-eu/opencloud/services/webdav/pkg/generator" +) + +// DefaultTypeMapping answers for every type in SupportedMimeTypes, so the +// format a thumbnail is written in can be read off here instead of being +// guessed from a file name. Types that carry alpha or animation keep their own +// format, everything else becomes a JPEG. An icon is the one type no generator +// writes back: it has to become a PNG, or JPEG would flatten its transparent +// pixels onto black. +var DefaultTypeMapping = []string{ + "image/png:image/png", + "image/gif:image/gif", + "image/jpeg:image/jpeg", + "image/jpg:image/jpeg", + "image/webp:image/jpeg", + "image/bmp:image/jpeg", + "image/x-ms-bmp:image/jpeg", + "image/tiff:image/jpeg", + "image/vnd.microsoft.icon:image/png", + "image/x-icon:image/png", + "text/plain:image/jpeg", + "audio/flac:image/jpeg", + "audio/mpeg:image/jpeg", + "audio/ogg:image/jpeg", + "application/vnd.geogebra.slides:image/jpeg", + "application/vnd.geogebra.pinboard:image/jpeg", +} + +// TypeMapping says which type a thumbnail of an image is written in, for the +// types where the image's own is not the answer. +type TypeMapping struct { + byType map[string]string +} + +// NewTypeMapping parses "mime:mime" entries on top of DefaultTypeMapping, so a +// configuration that names one type does not take the answer away from all the +// others. +func NewTypeMapping(entries []string) TypeMapping { + return TypeMapping{byType: generator.ParsePairs(DefaultTypeMapping, entries)} +} + +// TargetFor is the type a thumbnail of this type is written in, "" when nothing +// is mapped and the caller keeps what it had. +func (m TypeMapping) TargetFor(contentType string) string { + parsed, _, err := mime.ParseMediaType(contentType) + if err != nil { + return "" + } + + return m.byType[parsed] +} diff --git a/services/webdav/pkg/thumbnail/typemapping_test.go b/services/webdav/pkg/thumbnail/typemapping_test.go new file mode 100644 index 0000000000..8ab901f56b --- /dev/null +++ b/services/webdav/pkg/thumbnail/typemapping_test.go @@ -0,0 +1,61 @@ +package thumbnail_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/opencloud-eu/opencloud/services/webdav/pkg/generator" + "github.com/opencloud-eu/opencloud/services/webdav/pkg/thumbnail" +) + +var _ = Describe("TypeMapping", func() { + m := thumbnail.NewTypeMapping(nil) + + It("answers for every type a preview is made of", func() { + for mimeType := range thumbnail.SupportedMimeTypes { + Expect(m.TargetFor(mimeType)).ToNot(BeEmpty(), "%s has no output type", mimeType) + } + }) + + It("keeps a format that carries alpha or animation", func() { + Expect(m.TargetFor("image/png")).To(Equal("image/png")) + Expect(m.TargetFor("image/gif")).To(Equal("image/gif")) + }) + + It("writes an icon as a png, since no generator writes icons", func() { + Expect(m.TargetFor("image/vnd.microsoft.icon")).To(Equal("image/png")) + Expect(m.TargetFor("image/x-icon")).To(Equal("image/png")) + }) + + It("reads a type with parameters", func() { + Expect(m.TargetFor("image/png; charset=binary")).To(Equal("image/png")) + }) + + It("leaves a type nobody mapped to the caller", func() { + Expect(m.TargetFor("application/pdf")).To(BeEmpty()) + Expect(m.TargetFor("not a mime type")).To(BeEmpty()) + }) +}) + +var _ = Describe("TypeMapping from the configuration", func() { + It("overrides a default and keeps the rest", func() { + m := thumbnail.NewTypeMapping([]string{" Image/TIFF : Image/PNG "}) + + Expect(m.TargetFor("image/tiff")).To(Equal("image/png")) + Expect(m.TargetFor("image/png")).To(Equal("image/png"), "a default nobody touched still answers") + Expect(m.TargetFor("image/bmp")).To(Equal("image/jpeg")) + }) + + It("hands on a target the generator cannot write, for the caller to reject", func() { + m := thumbnail.NewTypeMapping([]string{"image/png:image/vnd.microsoft.icon"}) + + Expect(m.TargetFor("image/png")).To(Equal("image/vnd.microsoft.icon")) + Expect(generator.NewFormats(nil).For("image/vnd.microsoft.icon")).To(BeEmpty()) + }) + + It("takes an empty target as back to the default", func() { + m := thumbnail.NewTypeMapping([]string{"image/png:"}) + + Expect(m.TargetFor("image/png")).To(BeEmpty()) + }) +}) diff --git a/services/webdav/pkg/thumbnail/workflow/encode_imaging.go b/services/webdav/pkg/thumbnail/workflow/encode_imaging.go index e732dd9d25..ed7fb10758 100644 --- a/services/webdav/pkg/thumbnail/workflow/encode_imaging.go +++ b/services/webdav/pkg/thumbnail/workflow/encode_imaging.go @@ -9,34 +9,30 @@ import ( "image/gif" "image/jpeg" "image/png" - - "github.com/opencloud-eu/opencloud/services/webdav/pkg/generator" ) // encodeForUpload turns the value produced by the preprocessing step into -// bytes for upload to the generator. Raw image bytes pass through unchanged; -// converted values are encoded in-process. +// bytes for upload to the generator, and says which type they are in. func encodeForUpload(v any, mimeType string) ([]byte, string, error) { switch data := v.(type) { - case []byte: - return data, generator.MimeToExt(mimeType), nil case image.Image: - var ( - buf bytes.Buffer - err error - ) + var buf bytes.Buffer if mimeType == "image/jpeg" || mimeType == "image/jpg" { - err = jpeg.Encode(&buf, data, &jpeg.Options{Quality: 85}) - return buf.Bytes(), "jpg", err + err := jpeg.Encode(&buf, data, &jpeg.Options{Quality: 85}) + + return buf.Bytes(), "image/jpeg", err } - err = png.Encode(&buf, data) - return buf.Bytes(), "png", err + + err := png.Encode(&buf, data) + + return buf.Bytes(), "image/png", err case *gif.GIF: var buf bytes.Buffer if err := gif.EncodeAll(&buf, data); err != nil { return nil, "", fmt.Errorf("encode gif: %w", err) } - return buf.Bytes(), "gif", nil + + return buf.Bytes(), "image/gif", nil default: return nil, "", fmt.Errorf("unsupported converted type %T", v) } diff --git a/services/webdav/pkg/thumbnail/workflow/encode_vips.go b/services/webdav/pkg/thumbnail/workflow/encode_vips.go index d3bce98a54..2e83a56fbd 100644 --- a/services/webdav/pkg/thumbnail/workflow/encode_vips.go +++ b/services/webdav/pkg/thumbnail/workflow/encode_vips.go @@ -8,17 +8,12 @@ import ( "image/gif" vips "github.com/davidbyttow/govips/v2/vips" - - "github.com/opencloud-eu/opencloud/services/webdav/pkg/generator" ) // encodeForUpload turns the value produced by the preprocessing step into -// bytes for upload to the generator. Raw image bytes pass through unchanged; -// converted values are encoded in-process via libvips. +// bytes for upload to the generator, and says which type they are in. func encodeForUpload(v any, mimeType string) ([]byte, string, error) { switch data := v.(type) { - case []byte: - return data, generator.MimeToExt(mimeType), nil case image.Image: img, err := vips.NewImageFromGoImage(data) if err != nil { @@ -28,14 +23,18 @@ func encodeForUpload(v any, mimeType string) ([]byte, string, error) { if mimeType == "image/jpeg" || mimeType == "image/jpg" { buf, _, err := img.ExportJpeg(&vips.JpegExportParams{Quality: 85}) - return buf, "jpg", err + + return buf, "image/jpeg", err } + buf, _, err := img.ExportPng(&vips.PngExportParams{}) - return buf, "png", err + + return buf, "image/png", err case *gif.GIF: if len(data.Image) == 0 { return nil, "", fmt.Errorf("gif has no frames") } + img, err := vips.NewImageFromGoImage(data.Image[0]) if err != nil { return nil, "", fmt.Errorf("create vips image: %w", err) @@ -43,7 +42,8 @@ func encodeForUpload(v any, mimeType string) ([]byte, string, error) { defer img.Close() buf, _, err := img.ExportGIF(&vips.GifExportParams{}) - return buf, "gif", err + + return buf, "image/gif", err default: return nil, "", fmt.Errorf("unsupported converted type %T", v) } diff --git a/services/webdav/pkg/thumbnail/workflow/workflow.go b/services/webdav/pkg/thumbnail/workflow/workflow.go index efc396e4ef..960e72669f 100644 --- a/services/webdav/pkg/thumbnail/workflow/workflow.go +++ b/services/webdav/pkg/thumbnail/workflow/workflow.go @@ -116,14 +116,14 @@ func processorID(processor string) string { return strings.ToLower(processor) } -// thumbnailStorageKey builds the cache key: checksum[:2]/checksum[2:4]/checksum[4:]/wxh-.ext. +// thumbnailStorageKey builds the cache key: checksum[:2]/checksum[2:4]/checksum[4:]/wxh-.format. // The segment (fit-in, fill, stretch, ...) distinguishes requests that // share a size but produce different images, so they never share a cache entry. // The requested resolution is used (not the matched one) so the key can be // computed before downloading the source. Caches regenerate when the configured // resolutions or operations change. -func thumbnailStorageKey(checksum string, w, h int, ext, processor string) string { - name := fmt.Sprintf("%dx%d-%s.%s", w, h, processor, ext) +func thumbnailStorageKey(checksum string, w, h int, format, processor string) string { + name := fmt.Sprintf("%dx%d-%s.%s", w, h, processor, format) if len(checksum) >= 6 { return checksum[:2] + "/" + checksum[2:4] + "/" + checksum[4:] + "/" + name } @@ -138,6 +138,8 @@ type ThumbnailWorkflow struct { httpClient *http.Client maxInputSize uint64 sizes *thumbnail.Sizes + typeMapping thumbnail.TypeMapping + formats generator.Formats webdavNS string fontMapFile string log log.Logger @@ -207,6 +209,16 @@ func WithWebdavNamespace(ns string) Option { return func(w *ThumbnailWorkflow) { w.webdavNS = ns } } +// WithFormats sets the formats the generator writes, by type. +func WithFormats(f generator.Formats) Option { + return func(w *ThumbnailWorkflow) { w.formats = f } +} + +// WithTypeMapping sets the format a thumbnail of a given image type is written in. +func WithTypeMapping(m thumbnail.TypeMapping) Option { + return func(w *ThumbnailWorkflow) { w.typeMapping = m } +} + // WithFontMapFile sets the font map file used for text thumbnail rendering. func WithFontMapFile(file string) Option { return func(w *ThumbnailWorkflow) { w.fontMapFile = file } @@ -241,25 +253,25 @@ func WithUserResolver(r UserResolver) Option { } // Execute handles a user thumbnail request (GET). -func (w *ThumbnailWorkflow) Execute(ctx context.Context, tr *requests.ThumbnailRequest, auth string, logger log.Logger) (data []byte, ext string, aIgnored bool, err error) { +func (w *ThumbnailWorkflow) Execute(ctx context.Context, tr *requests.ThumbnailRequest, auth string, logger log.Logger) (data []byte, contentType string, aIgnored bool, err error) { ref, err := w.resolveReference(ctx, tr, auth) if err != nil { return nil, "", false, err } - data, ext, aIgnored, err = w.generate(ctx, ref, auth, tr, logger) - return data, ext, aIgnored, err + data, contentType, aIgnored, err = w.generate(ctx, ref, auth, tr, logger) + return data, contentType, aIgnored, err } // ExecutePublic handles a public link thumbnail request (GET). -func (w *ThumbnailWorkflow) ExecutePublic(ctx context.Context, tr *requests.ThumbnailRequest, auth string, logger log.Logger) (data []byte, ext string, aIgnored bool, err error) { +func (w *ThumbnailWorkflow) ExecutePublic(ctx context.Context, tr *requests.ThumbnailRequest, auth string, logger log.Logger) (data []byte, contentType string, aIgnored bool, err error) { ref, err := w.resolveReference(ctx, tr, auth) if err != nil { return nil, "", false, err } - data, ext, aIgnored, err = w.generate(ctx, ref, auth, tr, logger) - return data, ext, aIgnored, err + data, contentType, aIgnored, err = w.generate(ctx, ref, auth, tr, logger) + return data, contentType, aIgnored, err } // Head performs a cache-only check for a public link thumbnail (HEAD). @@ -412,12 +424,14 @@ func (w *ThumbnailWorkflow) generate(ctx context.Context, ref *providerv1beta1.R } mimeType := info.GetMimeType() - // The output type follows the source mime (like main's GetExtForMime); when - // the source has no dedicated image type (txt, audio) it falls back to the - // requested extension. - outputExt := generator.OutputFormat(tr.Extension) - if ext := generator.ExtForMime(mimeType); ext != "" { - outputExt = ext + // The thumbnail is written in the type the source type maps to, asked for by + // the name the generator's filter uses. A type nobody mapped, and one whose + // target the generator cannot write, becomes a JPEG. + outputType, outputFormat := "image/jpeg", "jpg" + if target := w.typeMapping.TargetFor(mimeType); target != "" { + if format := w.formats.For(target); format != "" { + outputType, outputFormat = target, format + } } // The operation is resolved up front so it can be part of the cache key: a @@ -440,11 +454,11 @@ 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(), outputExt, processorID(operation)) + cacheKey := thumbnailStorageKey(checksum, box.Dx(), box.Dy(), outputFormat, processorID(operation)) if w.cache != nil { if cached, err := w.cache.Get(cacheKey); err == nil { - return cached, outputExt, aIgnored, nil + return cached, outputType, aIgnored, nil } } @@ -452,7 +466,7 @@ func (w *ThumbnailWorkflow) generate(ctx context.Context, ref *providerv1beta1.R // generator fill that box. Only fit-in suppresses upscaling (noUpscale); for // fill/thumbnail/resize the generator may enlarge a small source (e.g. a text // file rendered at a fixed canvas) to fill the snapped box, matching main. - genURL := generator.BuildURL(w.generatorURL, int32(box.Dx()), int32(box.Dy()), operation, outputExt, noUpscale) + genURL := generator.BuildURL(w.generatorURL, int32(box.Dx()), int32(box.Dy()), operation, outputFormat, noUpscale) // Produce the source image to send to the generator, never decoding an image // in webdav: real images (incl. gif) are streamed straight through undecoded; @@ -465,11 +479,17 @@ func (w *ThumbnailWorkflow) generate(ctx context.Context, ref *providerv1beta1.R } defer cleanup() - thumbBytes, err := w.postToGenerator(ctx, genURL, imgStream, tr.Filename, srcType) + thumbBytes, thumbType, err := w.postToGenerator(ctx, genURL, imgStream, tr.Filename, srcType) if err != nil { logger.Error().Err(err).Msg("could not generate thumbnail") return nil, "", false, fmt.Errorf("generate: %w", err) } + 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). + Msg("generator wrote a type we did not ask for") + } if w.cache != nil { if err := w.cache.Put(cacheKey, thumbBytes); err != nil { @@ -477,7 +497,7 @@ func (w *ThumbnailWorkflow) generate(ctx context.Context, ref *providerv1beta1.R } } - return thumbBytes, outputExt, aIgnored, nil + return thumbBytes, outputType, aIgnored, nil } // sourceImage returns a reader over the source image to send to the generator, @@ -516,11 +536,12 @@ func (w *ThumbnailWorkflow) sourceImage(ctx context.Context, ref *providerv1beta return nil, "", nil, fmt.Errorf("%w: could not get image", ErrNotFound) } - data, ext, err := encodeForUpload(img, mimeType) + data, contentType, err := encodeForUpload(img, mimeType) if err != nil { return nil, "", nil, fmt.Errorf("encode converted image: %w", err) } - return bytes.NewReader(data), generator.ContentType(ext), func() {}, nil + + return bytes.NewReader(data), contentType, func() {}, nil } // matchOperation resolves the generator resize/crop operation for a request from @@ -593,7 +614,7 @@ var quoteEscaper = strings.NewReplacer("\\", "\\\\", `"`, "\\\"") // the form part on demand. No Content-Length is set, so the request uses chunked // transfer-encoding; both imagor and the thumbnailer parse the multipart body // without needing it (imagor enforces its size limit after reading the part). -func (w *ThumbnailWorkflow) postToGenerator(ctx context.Context, url string, img io.Reader, filename, contentType string) ([]byte, error) { +func (w *ThumbnailWorkflow) postToGenerator(ctx context.Context, url string, img io.Reader, filename, contentType string) ([]byte, string, error) { pr, pw := io.Pipe() writer := multipart.NewWriter(pw) @@ -625,7 +646,7 @@ func (w *ThumbnailWorkflow) postToGenerator(ctx context.Context, url string, img req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, pr) if err != nil { pr.Close() - return nil, fmt.Errorf("create generator request: %w", err) + return nil, "", fmt.Errorf("create generator request: %w", err) } req.Header.Set("Content-Type", writer.FormDataContentType()) // Forward the request ID so the generator's logs can be correlated with this @@ -638,13 +659,13 @@ func (w *ThumbnailWorkflow) postToGenerator(ctx context.Context, url string, img httpRsp, err := w.httpClient.Do(req) if err != nil { pr.Close() - return nil, fmt.Errorf("generator request: %w", err) + return nil, "", fmt.Errorf("generator request: %w", err) } defer httpRsp.Body.Close() if writeErr := <-errCh; writeErr != nil { io.Copy(io.Discard, httpRsp.Body) - return nil, writeErr + return nil, "", writeErr } if httpRsp.StatusCode < 200 || httpRsp.StatusCode >= 300 { @@ -654,17 +675,17 @@ func (w *ThumbnailWorkflow) postToGenerator(ctx context.Context, url string, img // legacy "image is too large" error so the DAV layer returns 403, matching // the pre-refactor behavior where webdav enforced the dimension limit. if httpRsp.StatusCode == http.StatusUnprocessableEntity { - return nil, ErrImageTooLarge + return nil, "", ErrImageTooLarge } - return nil, fmt.Errorf("generator returned status %d: %s", httpRsp.StatusCode, string(respBody)) + return nil, "", fmt.Errorf("generator returned status %d: %s", httpRsp.StatusCode, string(respBody)) } rspData, err := io.ReadAll(httpRsp.Body) if err != nil { - return nil, fmt.Errorf("read generator response: %w", err) + return nil, "", fmt.Errorf("read generator response: %w", err) } - return rspData, nil + return rspData, httpRsp.Header.Get("Content-Type"), nil } // gatewaySelector is the interface for selecting a gateway client. diff --git a/services/webdav/pkg/thumbnail/workflow/workflow_test.go b/services/webdav/pkg/thumbnail/workflow/workflow_test.go index 34eeeb1c37..65ec7c7a2a 100644 --- a/services/webdav/pkg/thumbnail/workflow/workflow_test.go +++ b/services/webdav/pkg/thumbnail/workflow/workflow_test.go @@ -33,6 +33,7 @@ import ( "github.com/opencloud-eu/opencloud/services/webdav/pkg/constants" "github.com/opencloud-eu/opencloud/services/webdav/pkg/dav/requests" "github.com/opencloud-eu/opencloud/services/webdav/pkg/generator" + "github.com/opencloud-eu/opencloud/services/webdav/pkg/thumbnail" "github.com/opencloud-eu/opencloud/services/webdav/pkg/thumbnail/cache" ) @@ -187,6 +188,8 @@ func setupPreprocessTest( WithCache(testCache), WithHTTPClient(&http.Client{}), WithWebdavNamespace("/users/{{.Id.OpaqueId}}"), + WithFormats(generator.NewFormats(nil)), + WithTypeMapping(thumbnail.NewTypeMapping(nil)), WithLogger(logger), WithStater(NewGatewayStater(pool.GetSelector[gatewayv1beta1.GatewayAPIClient]( "GatewaySelector", "test.gateway", @@ -221,7 +224,7 @@ func setupPreprocessTest( PermissionSet: &providerv1beta1.ResourcePermissions{InitiateFileDownload: true}, MimeType: mimeType, Size: uint64(len(downloadBody)), - Checksum: &providerv1beta1.ResourceChecksum{Sum: "ppchecksum"}, + Checksum: &providerv1beta1.ResourceChecksum{Sum: "1f0c9f1b6e4b4b2a9c1d7f3e8a5b6c7d8e9f0a1b"}, }, }, nil) @@ -300,6 +303,8 @@ var _ = Describe("ThumbnailWorkflow", func() { WithCache(testCache), WithHTTPClient(&http.Client{}), WithWebdavNamespace("/users/{{.Id.OpaqueId}}"), + WithFormats(generator.NewFormats(nil)), + WithTypeMapping(thumbnail.NewTypeMapping(nil)), WithLogger(logger), WithStater(NewGatewayStater(gatewaySelector)), WithFileDownloader(NewGatewayFileDownloader(gatewaySelector, &http.Client{})), @@ -392,18 +397,17 @@ var _ = Describe("ThumbnailWorkflow", func() { wf.generatorURL = generatorSrv.URL tr := &requests.ThumbnailRequest{ - Ref: spaceRef("storageid$spaceid!opaqueid", "test.jpg"), - Filename: "test.jpg", - Extension: ".jpg", - Width: 128, - Height: 128, - Aspect: true, + Ref: spaceRef("storageid$spaceid!opaqueid", "test.jpg"), + Filename: "test.jpg", + Width: 128, + Height: 128, + Aspect: true, } - data, ext, _, err := wf.Execute(context.Background(), tr, testToken, logger) + data, contentType, _, err := wf.Execute(context.Background(), tr, testToken, logger) Expect(err).ToNot(HaveOccurred()) Expect(data).To(Equal(thumbBytes)) - Expect(ext).To(Equal("jpg")) + Expect(contentType).To(Equal("image/jpeg")) cached, err := testCache.Get("cb/56/752477cae6405f85b131872c60d21b967c6a/128x128-fill.jpg") Expect(err).ToNot(HaveOccurred()) @@ -470,16 +474,15 @@ var _ = Describe("ThumbnailWorkflow", func() { wf.generatorURL = generatorSrv.URL tr := &requests.ThumbnailRequest{ - Ref: &providerv1beta1.Reference{Path: "photo.jpeg"}, - Filename: "photo.jpeg", - Extension: ".jpeg", - Width: 36, - Height: 36, + Ref: &providerv1beta1.Reference{Path: "photo.jpeg"}, + Filename: "photo.jpeg", + Width: 36, + Height: 36, } - data, ext, _, err := wf.Execute(context.Background(), tr, testToken, logger) + data, contentType, _, err := wf.Execute(context.Background(), tr, testToken, logger) Expect(err).ToNot(HaveOccurred()) - Expect(ext).To(Equal("jpg")) + Expect(contentType).To(Equal("image/jpeg")) Expect(data).To(Equal([]byte("webdav-thumbnail"))) // No username in the URL: the token owner (test) is resolved via WhoAmI. @@ -539,7 +542,6 @@ var _ = Describe("ThumbnailWorkflow", func() { tr := &requests.ThumbnailRequest{ Ref: &providerv1beta1.Reference{Path: "parent.txt"}, Filename: "parent.txt", - Extension: ".txt", Width: 32, Height: 32, Identifier: "Alice", @@ -604,16 +606,15 @@ var _ = Describe("ThumbnailWorkflow", func() { wf.generatorURL = generatorSrv.URL tr := &requests.ThumbnailRequest{ - Ref: spaceRef(ssid, "photo.jpeg"), - Filename: "photo.jpeg", - Extension: ".jpeg", - Width: 36, - Height: 36, + Ref: spaceRef(ssid, "photo.jpeg"), + Filename: "photo.jpeg", + Width: 36, + Height: 36, } - data, ext, _, err := wf.Execute(context.Background(), tr, testToken, logger) + data, contentType, _, err := wf.Execute(context.Background(), tr, testToken, logger) Expect(err).ToNot(HaveOccurred()) - Expect(ext).To(Equal("jpg")) + Expect(contentType).To(Equal("image/jpeg")) Expect(data).To(Equal([]byte("space-thumbnail"))) Expect(downloadRef.GetResourceId().StorageId).To(Equal(storage)) @@ -683,16 +684,15 @@ var _ = Describe("ThumbnailWorkflow", func() { wf.generatorURL = generatorSrv.URL tr := &requests.ThumbnailRequest{ - Ref: spaceRef(ssid, "lorem.txt"), - Filename: "lorem.txt", - Extension: ".txt", - Width: 36, - Height: 36, + Ref: spaceRef(ssid, "lorem.txt"), + Filename: "lorem.txt", + Width: 36, + Height: 36, } - data, ext, _, err := wf.Execute(context.Background(), tr, testToken, logger) + data, contentType, _, err := wf.Execute(context.Background(), tr, testToken, logger) Expect(err).ToNot(HaveOccurred()) - Expect(ext).To(Equal("jpg")) + Expect(contentType).To(Equal("image/jpeg")) Expect(data).To(Equal([]byte("share-thumbnail"))) // The shares space ResourceId is used verbatim; no lookup was needed. @@ -766,15 +766,14 @@ var _ = Describe("ThumbnailWorkflow", func() { tr := &requests.ThumbnailRequest{ Ref: &providerv1beta1.Reference{Path: "photo.jpeg"}, Filename: "photo.jpeg", - Extension: ".jpeg", Width: 36, Height: 36, Identifier: "alice", } - data, ext, _, err := wf.Execute(context.Background(), tr, testToken, logger) + data, contentType, _, err := wf.Execute(context.Background(), tr, testToken, logger) Expect(err).ToNot(HaveOccurred()) - Expect(ext).To(Equal("jpg")) + Expect(contentType).To(Equal("image/jpeg")) Expect(data).To(Equal([]byte("user-thumbnail"))) // The username from the URL (alice) is resolved via GetUserByClaim and @@ -794,11 +793,10 @@ var _ = Describe("ThumbnailWorkflow", func() { wf2, captured := setupPreprocessTest(GinkgoT(), gatewayClient, logger, testCache, "image/png", "img.png", pngBytes) tr := &requests.ThumbnailRequest{ - Ref: spaceRef("storageid$spaceid!opaqueid", "img.png"), - Filename: "img.png", - Extension: ".png", - Width: 64, - Height: 64, + Ref: spaceRef("storageid$spaceid!opaqueid", "img.png"), + Filename: "img.png", + Width: 64, + Height: 64, } _, _, _, err = wf2.Execute(context.Background(), tr, testToken, logger) @@ -813,11 +811,10 @@ var _ = Describe("ThumbnailWorkflow", func() { wf2, captured := setupPreprocessTest(GinkgoT(), gatewayClient, logger, testCache, "audio/mpeg", "song.mp3", mp3Bytes) tr := &requests.ThumbnailRequest{ - Ref: spaceRef("storageid$spaceid!opaqueid", "song.mp3"), - Filename: "song.mp3", - Extension: ".mp3", - Width: 64, - Height: 64, + Ref: spaceRef("storageid$spaceid!opaqueid", "song.mp3"), + Filename: "song.mp3", + Width: 64, + Height: 64, } _, _, _, err = wf2.Execute(context.Background(), tr, testToken, logger) @@ -833,11 +830,10 @@ var _ = Describe("ThumbnailWorkflow", func() { wf2, captured := setupPreprocessTest(GinkgoT(), gatewayClient, logger, testCache, "application/vnd.geogebra.slides", "deck.ggs", ggsBytes) tr := &requests.ThumbnailRequest{ - Ref: spaceRef("storageid$spaceid!opaqueid", "deck.ggs"), - Filename: "deck.ggs", - Extension: ".ggs", - Width: 64, - Height: 64, + Ref: spaceRef("storageid$spaceid!opaqueid", "deck.ggs"), + Filename: "deck.ggs", + Width: 64, + Height: 64, } _, _, _, err = wf2.Execute(context.Background(), tr, testToken, logger) @@ -852,11 +848,10 @@ var _ = Describe("ThumbnailWorkflow", func() { wf2, captured := setupPreprocessTest(GinkgoT(), gatewayClient, logger, testCache, "text/plain", "notes.txt", text) tr := &requests.ThumbnailRequest{ - Ref: spaceRef("storageid$spaceid!opaqueid", "notes.txt"), - Filename: "notes.txt", - Extension: ".txt", - Width: 64, - Height: 64, + Ref: spaceRef("storageid$spaceid!opaqueid", "notes.txt"), + Filename: "notes.txt", + Width: 64, + Height: 64, } _, _, _, err := wf2.Execute(context.Background(), tr, testToken, logger) @@ -872,11 +867,10 @@ var _ = Describe("ThumbnailWorkflow", func() { wf2, captured := setupPreprocessTest(GinkgoT(), gatewayClient, logger, testCache, "image/gif", "anim.gif", gifBytes) tr := &requests.ThumbnailRequest{ - Ref: spaceRef("storageid$spaceid!opaqueid", "anim.gif"), - Filename: "anim.gif", - Extension: ".gif", - Width: 64, - Height: 64, + Ref: spaceRef("storageid$spaceid!opaqueid", "anim.gif"), + Filename: "anim.gif", + Width: 64, + Height: 64, } _, _, _, err = wf2.Execute(context.Background(), tr, testToken, logger) @@ -890,11 +884,10 @@ var _ = Describe("ThumbnailWorkflow", func() { wf2, _ := setupPreprocessTest(GinkgoT(), gatewayClient, logger, testCache, "audio/mpeg", "bad.mp3", []byte("not a real audio file")) tr := &requests.ThumbnailRequest{ - Ref: spaceRef("storageid$spaceid!opaqueid", "bad.mp3"), - Filename: "bad.mp3", - Extension: ".mp3", - Width: 64, - Height: 64, + Ref: spaceRef("storageid$spaceid!opaqueid", "bad.mp3"), + Filename: "bad.mp3", + Width: 64, + Height: 64, } _, _, _, err := wf2.Execute(context.Background(), tr, testToken, logger) @@ -904,6 +897,73 @@ var _ = Describe("ThumbnailWorkflow", func() { }) }) + Describe("output format", func() { + DescribeTable("asks for the format the source type maps to", + func(mimeType, filename, wantFilter, wantType, wantKey string) { + wf2, captured := setupPreprocessTest(GinkgoT(), gatewayClient, logger, testCache, mimeType, filename, []byte("source-bytes")) + + tr := &requests.ThumbnailRequest{ + Ref: spaceRef("storageid$spaceid!opaqueid", filename), + Filename: filename, + Width: 64, + Height: 64, + } + + _, contentType, _, err := wf2.Execute(context.Background(), tr, testToken, logger) + Expect(err).ToNot(HaveOccurred()) + Expect(captured.path).To(ContainSubstring(wantFilter)) + Expect(contentType).To(Equal(wantType)) + + _, err = testCache.Get(wantKey) + Expect(err).ToNot(HaveOccurred(), "the format belongs in the cache key") + }, + Entry("a png keeps its alpha channel", + "image/png", "img.png", "filters:format(png)", "image/png", + "1f/0c/9f1b6e4b4b2a9c1d7f3e8a5b6c7d8e9f0a1b/64x64-fill.png"), + Entry("a gif keeps its animation", + "image/gif", "anim.gif", "filters:format(gif)", "image/gif", + "1f/0c/9f1b6e4b4b2a9c1d7f3e8a5b6c7d8e9f0a1b/64x64-stretch.gif"), + Entry("a bmp becomes a jpeg", + "image/bmp", "scan.bmp", "filters:format(jpg)", "image/jpeg", + "1f/0c/9f1b6e4b4b2a9c1d7f3e8a5b6c7d8e9f0a1b/64x64-fill.jpg"), + ) + + It("follows a configured chain into a format the built-in generator cannot write", func() { + wf2, captured := setupPreprocessTest(GinkgoT(), gatewayClient, logger, testCache, "image/png", "img.png", []byte("source-bytes")) + wf2.typeMapping = thumbnail.NewTypeMapping([]string{"image/png:image/webp"}) + wf2.formats = generator.NewFormats([]string{"image/webp:webp"}) + + tr := &requests.ThumbnailRequest{ + Ref: spaceRef("storageid$spaceid!opaqueid", "img.png"), + Filename: "img.png", + Width: 64, + Height: 64, + } + + _, contentType, _, err := wf2.Execute(context.Background(), tr, testToken, logger) + Expect(err).ToNot(HaveOccurred()) + Expect(captured.path).To(ContainSubstring("filters:format(webp)")) + Expect(contentType).To(Equal("image/webp")) + }) + + It("falls back to a jpeg when the generator cannot write the mapped target", func() { + wf2, captured := setupPreprocessTest(GinkgoT(), gatewayClient, logger, testCache, "image/png", "img.png", []byte("source-bytes")) + wf2.typeMapping = thumbnail.NewTypeMapping([]string{"image/png:image/vnd.microsoft.icon"}) + + tr := &requests.ThumbnailRequest{ + Ref: spaceRef("storageid$spaceid!opaqueid", "img.png"), + Filename: "img.png", + Width: 64, + Height: 64, + } + + _, contentType, _, err := wf2.Execute(context.Background(), tr, testToken, logger) + Expect(err).ToNot(HaveOccurred()) + Expect(captured.path).To(ContainSubstring("filters:format(jpg)")) + Expect(contentType).To(Equal("image/jpeg")) + }) + }) + Describe("cache hit shortcut", func() { It("should return cached thumbnail without calling generator or downloading the file", func() { cachedThumb := []byte("cached-thumbnail") @@ -913,18 +973,17 @@ var _ = Describe("ThumbnailWorkflow", func() { // No download expectations are set up. If the workflow tried to // download the source on a cache hit, the gateway mock would fail. tr := &requests.ThumbnailRequest{ - Ref: spaceRef("storageid$spaceid!opaqueid", "test.jpg"), - Filename: "test.jpg", - Extension: ".jpg", - Width: 128, - Height: 128, - Aspect: true, + Ref: spaceRef("storageid$spaceid!opaqueid", "test.jpg"), + Filename: "test.jpg", + Width: 128, + Height: 128, + Aspect: true, } - data, ext, _, err := wf.Execute(context.Background(), tr, testToken, logger) + data, contentType, _, err := wf.Execute(context.Background(), tr, testToken, logger) Expect(err).ToNot(HaveOccurred()) Expect(data).To(Equal(cachedThumb)) - Expect(ext).To(Equal("jpg")) + Expect(contentType).To(Equal("image/jpeg")) }) }) @@ -944,11 +1003,10 @@ var _ = Describe("ThumbnailWorkflow", func() { }, nil) tr := &requests.ThumbnailRequest{ - Ref: spaceRef("storageid$spaceid!opaqueid", "document.pdf"), - Filename: "document.pdf", - Extension: ".pdf", - Width: 64, - Height: 64, + Ref: spaceRef("storageid$spaceid!opaqueid", "document.pdf"), + Filename: "document.pdf", + Width: 64, + Height: 64, } _, _, _, err := wf.Execute(context.Background(), tr, testToken, logger) @@ -969,11 +1027,10 @@ var _ = Describe("ThumbnailWorkflow", func() { }, nil) tr := &requests.ThumbnailRequest{ - Ref: spaceRef("storageid$spaceid!opaqueid", "myfolder/"), - Filename: "myfolder", - Extension: "", - Width: 64, - Height: 64, + Ref: spaceRef("storageid$spaceid!opaqueid", "myfolder/"), + Filename: "myfolder", + Width: 64, + Height: 64, } _, _, _, err := wf.Execute(context.Background(), tr, testToken, logger) @@ -1025,11 +1082,10 @@ var _ = Describe("ThumbnailWorkflow", func() { }, nil) tr := &requests.ThumbnailRequest{ - Ref: spaceRef("storageid$spaceid!opaqueid", "large.jpg"), - Filename: "large.jpg", - Extension: ".jpg", - Width: 64, - Height: 64, + Ref: spaceRef("storageid$spaceid!opaqueid", "large.jpg"), + Filename: "large.jpg", + Width: 64, + Height: 64, } _, _, _, err = wfWithLimit.Execute(context.Background(), tr, testToken, logger) @@ -1065,11 +1121,10 @@ var _ = Describe("ThumbnailWorkflow", func() { wf.generatorURL = generatorSrv.URL tr := &requests.ThumbnailRequest{ - Ref: spaceRef("storageid$spaceid!opaqueid", "test.jpg"), - Filename: "test.jpg", - Extension: ".jpg", - Width: 100, - Height: 100, + Ref: spaceRef("storageid$spaceid!opaqueid", "test.jpg"), + Filename: "test.jpg", + Width: 100, + Height: 100, } _, _, _, err := wf.Execute(context.Background(), tr, testToken, logger) @@ -1105,11 +1160,10 @@ var _ = Describe("ThumbnailWorkflow", func() { wf.generatorURL = generatorSrv.URL tr := &requests.ThumbnailRequest{ - Ref: spaceRef("storageid$spaceid!opaqueid", "test.jpg"), - Filename: "test.jpg", - Extension: ".jpg", - Width: 64, - Height: 64, + Ref: spaceRef("storageid$spaceid!opaqueid", "test.jpg"), + Filename: "test.jpg", + Width: 64, + Height: 64, } _, _, _, err := wf.Execute(context.Background(), tr, testToken, logger) @@ -1180,18 +1234,17 @@ var _ = Describe("ThumbnailWorkflow", func() { Expect(err).ToNot(HaveOccurred()) tr := &requests.ThumbnailRequest{ - Ref: &providerv1beta1.Reference{Path: "/public/" + publicLinkToken + "/folder/photo.jpg"}, - Filename: "photo.jpg", - Extension: ".jpg", - Width: 64, - Height: 64, - Aspect: true, + Ref: &providerv1beta1.Reference{Path: "/public/" + publicLinkToken + "/folder/photo.jpg"}, + Filename: "photo.jpg", + Width: 64, + Height: 64, + Aspect: true, } - data, ext, _, err := wf.ExecutePublic(req.Context(), tr, auth, logger) + data, contentType, _, err := wf.ExecutePublic(req.Context(), tr, auth, logger) Expect(err).ToNot(HaveOccurred()) Expect(data).To(Equal(thumbBytes)) - Expect(ext).To(Equal("jpg")) + Expect(contentType).To(Equal("image/jpeg")) cached, err := testCache.Get("5b/a1/b086d95a7e0fd8ff4323caee0c412ab07fc3/64x64-fill.jpg") Expect(err).ToNot(HaveOccurred()) @@ -1258,11 +1311,10 @@ var _ = Describe("ThumbnailWorkflow", func() { Expect(err).ToNot(HaveOccurred()) tr := &requests.ThumbnailRequest{ - Ref: &providerv1beta1.Reference{Path: "/public/" + publicLinkToken + "/photo.jpg"}, - Filename: "photo.jpg", - Extension: ".jpg", - Width: 64, - Height: 64, + Ref: &providerv1beta1.Reference{Path: "/public/" + publicLinkToken + "/photo.jpg"}, + Filename: "photo.jpg", + Width: 64, + Height: 64, } data, _, _, err := wf.ExecutePublic(req.Context(), tr, auth, logger) @@ -1333,11 +1385,10 @@ var _ = Describe("ThumbnailWorkflow", func() { Expect(err).ToNot(HaveOccurred()) tr := &requests.ThumbnailRequest{ - Ref: &providerv1beta1.Reference{Path: "/public/" + publicLinkToken + "/folder/photo.jpg"}, - Filename: "photo.jpg", - Extension: ".jpg", - Width: 64, - Height: 64, + Ref: &providerv1beta1.Reference{Path: "/public/" + publicLinkToken + "/folder/photo.jpg"}, + Filename: "photo.jpg", + Width: 64, + Height: 64, } err = wf.Head(req.Context(), tr, auth, logger) @@ -1367,11 +1418,10 @@ var _ = Describe("ThumbnailWorkflow", func() { Expect(err).ToNot(HaveOccurred()) tr := &requests.ThumbnailRequest{ - Ref: &providerv1beta1.Reference{Path: "/public/" + publicLinkToken + "/nonexistent.jpg"}, - Filename: "nonexistent.jpg", - Extension: ".jpg", - Width: 64, - Height: 64, + Ref: &providerv1beta1.Reference{Path: "/public/" + publicLinkToken + "/nonexistent.jpg"}, + Filename: "nonexistent.jpg", + Width: 64, + Height: 64, } err = wf.Head(req.Context(), tr, auth, logger) @@ -1407,11 +1457,10 @@ var _ = Describe("ThumbnailWorkflow", func() { Expect(err).ToNot(HaveOccurred()) tr := &requests.ThumbnailRequest{ - Ref: &providerv1beta1.Reference{Path: "/public/" + publicLinkToken + "/document.pdf"}, - Filename: "document.pdf", - Extension: ".pdf", - Width: 64, - Height: 64, + Ref: &providerv1beta1.Reference{Path: "/public/" + publicLinkToken + "/document.pdf"}, + Filename: "document.pdf", + Width: 64, + Height: 64, } err = wf.Head(req.Context(), tr, auth, logger) @@ -1436,11 +1485,10 @@ var _ = Describe("ThumbnailWorkflow", func() { }, nil) tr := &requests.ThumbnailRequest{ - Ref: spaceRef("storageid$spaceid!opaqueid", "restricted.jpg"), - Filename: "restricted.jpg", - Extension: ".jpg", - Width: 64, - Height: 64, + Ref: spaceRef("storageid$spaceid!opaqueid", "restricted.jpg"), + Filename: "restricted.jpg", + Width: 64, + Height: 64, } _, _, _, err := wf.Execute(context.Background(), tr, testToken, logger) @@ -1475,11 +1523,10 @@ var _ = Describe("ThumbnailWorkflow", func() { Expect(err).ToNot(HaveOccurred()) tr := &requests.ThumbnailRequest{ - Ref: &providerv1beta1.Reference{Path: "/public/" + publicLinkToken + "/folder/restricted.jpg"}, - Filename: "restricted.jpg", - Extension: ".jpg", - Width: 64, - Height: 64, + Ref: &providerv1beta1.Reference{Path: "/public/" + publicLinkToken + "/folder/restricted.jpg"}, + Filename: "restricted.jpg", + Width: 64, + Height: 64, } err = wf.Head(req.Context(), tr, auth, logger) @@ -1508,11 +1555,10 @@ var _ = Describe("ThumbnailWorkflow", func() { }, nil) tr := &requests.ThumbnailRequest{ - Ref: spaceRef("storageid$spaceid!opaqueid", "processing.jpg"), - Filename: "processing.jpg", - Extension: ".jpg", - Width: 64, - Height: 64, + Ref: spaceRef("storageid$spaceid!opaqueid", "processing.jpg"), + Filename: "processing.jpg", + Width: 64, + Height: 64, } _, _, _, err := wf.Execute(context.Background(), tr, testToken, logger) @@ -1552,11 +1598,10 @@ var _ = Describe("ThumbnailWorkflow", func() { Expect(err).ToNot(HaveOccurred()) tr := &requests.ThumbnailRequest{ - Ref: &providerv1beta1.Reference{Path: "/public/" + publicLinkToken + "/folder/photo.jpg"}, - Filename: "photo.jpg", - Extension: ".jpg", - Width: 64, - Height: 64, + Ref: &providerv1beta1.Reference{Path: "/public/" + publicLinkToken + "/folder/photo.jpg"}, + Filename: "photo.jpg", + Width: 64, + Height: 64, } err = wf.Head(req.Context(), tr, auth, logger) @@ -1573,11 +1618,10 @@ var _ = Describe("ThumbnailWorkflow", func() { }, nil) tr := &requests.ThumbnailRequest{ - Ref: spaceRef("storageid$spaceid!opaqueid", "otherusersfile.txt"), - Filename: "otherusersfile.txt", - Extension: ".txt", - Width: 64, - Height: 64, + Ref: spaceRef("storageid$spaceid!opaqueid", "otherusersfile.txt"), + Filename: "otherusersfile.txt", + Width: 64, + Height: 64, } _, _, _, err := wf.Execute(context.Background(), tr, testToken, logger) @@ -1602,9 +1646,10 @@ var _ = Describe("postToGenerator", func() { defer srv.Close() wf := &ThumbnailWorkflow{httpClient: srv.Client()} - out, err := wf.postToGenerator(context.Background(), srv.URL, strings.NewReader("payload"), `we"ird.png`, "image/png") + out, outType, err := wf.postToGenerator(context.Background(), srv.URL, strings.NewReader("payload"), `we"ird.png`, "image/png") Expect(err).ToNot(HaveOccurred()) Expect(string(out)).To(Equal("thumb")) + Expect(outType).To(HavePrefix("text/plain"), "the generator's own answer, whatever it says") Expect(partType).To(Equal("image/png"), "application/octet-stream is rejected by imagor") Expect(partDisposition).To(Equal(`form-data; name="image"; filename="we\"ird.png"`)) })