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