IMG-23: replace an old imagetype with the new one (#1482)

* imagetype (new) -> imagetype

* Always fallback in C-D, GetTypeMap -> GetTypeByName

* Do not send origin ContentType

config/configurators/configurators.go

@@ -132,48 +132,60 @@ func URLPath(s *string, name string) {
 }
 
 func ImageTypes(it *[]imagetype.Type, name string) error {
-	if env := os.Getenv(name); len(env) > 0 {
-		parts := strings.Split(env, ",")
+	// Get image types from environment variable
+	env := os.Getenv(name)
+	if len(env) == 0 {
+		return nil
+	}
 
-		*it = make([]imagetype.Type, 0, len(parts))
+	parts := strings.Split(env, ",")
+	*it = make([]imagetype.Type, 0, len(parts))
 
-		for _, p := range parts {
-			pt := strings.TrimSpace(p)
-			if t, ok := imagetype.Types[pt]; ok {
-				*it = append(*it, t)
-			} else {
-				return fmt.Errorf("Unknown image format: %s", pt)
-			}
+	for _, p := range parts {
+		part := strings.TrimSpace(p)
+
+		// For every part passed through the environment variable,
+		// check if it matches any of the image types defined in
+		// the imagetype package or return error.
+		t, ok := imagetype.GetTypeByName(part)
+		if !ok {
+			return fmt.Errorf("unknown image format: %s", part)
 		}
+		*it = append(*it, t)
 	}
 
 	return nil
 }
 
 func ImageTypesQuality(m map[imagetype.Type]int, name string) error {
-	if env := os.Getenv(name); len(env) > 0 {
-		parts := strings.Split(env, ",")
+	env := os.Getenv(name)
+	if len(env) == 0 {
+		return nil
+	}
 
-		for _, p := range parts {
-			i := strings.Index(p, "=")
-			if i < 0 {
-				return fmt.Errorf("Invalid format quality string: %s", p)
-			}
+	parts := strings.Split(env, ",")
 
-			imgtypeStr, qStr := strings.TrimSpace(p[:i]), strings.TrimSpace(p[i+1:])
+	for _, p := range parts {
+		i := strings.Index(p, "=")
+		if i < 0 {
+			return fmt.Errorf("invalid format quality string: %s", p)
+		}
 
-			imgtype, ok := imagetype.Types[imgtypeStr]
-			if !ok {
-				return fmt.Errorf("Invalid format: %s", p)
-			}
+		// Split the string into image type and quality
+		imgtypeStr, qStr := strings.TrimSpace(p[:i]), strings.TrimSpace(p[i+1:])
 
-			q, err := strconv.Atoi(qStr)
-			if err != nil || q <= 0 || q > 100 {
-				return fmt.Errorf("Invalid quality: %s", p)
-			}
+		// Check if quality is a valid integer
+		q, err := strconv.Atoi(qStr)
+		if err != nil || q <= 0 || q > 100 {
+			return fmt.Errorf("invalid quality: %s", p)
+		}
 
-			m[imgtype] = q
+		t, ok := imagetype.GetTypeByName(imgtypeStr)
+		if !ok {
+			return fmt.Errorf("unknown image format: %s", imgtypeStr)
 		}
+
+		m[t] = q
 	}
 
 	return nil

httpheaders/cdv.go

@@ -0,0 +1,76 @@
+package httpheaders
+
+import (
+	"fmt"
+	"mime"
+	"path/filepath"
+	"strings"
+)
+
+const (
+	// fallbackStem is used when the stem cannot be determined from the URL.
+	fallbackStem = "image"
+
+	// Content-Disposition header format
+	contentDispositionsHeader = "%s; filename=\"%s%s\""
+
+	// "inline" disposition types
+	inlineDisposition = "inline"
+
+	// "attachment" disposition type
+	attachmentDisposition = "attachment"
+)
+
+// ContentDispositionValue generates the content-disposition header value.
+//
+// It uses the following priorities:
+// 1. By default, it uses the filename and extension from the URL.
+// 2. If `filename` is provided, it overrides the URL filename.
+// 3. If `contentType` is provided, it tries to determine the extension from the content type.
+// 4. If `ext` is provided, it overrides any extension determined from the URL or header.
+// 5. If the filename is still empty, it uses fallback stem.
+func ContentDispositionValue(url, filename, ext, contentType string, returnAttachment bool) string {
+	// By default, let's use the URL filename and extension
+	_, urlFilename := filepath.Split(url)
+	urlExt := filepath.Ext(urlFilename)
+
+	var rStem string
+
+	// Avoid strings.TrimSuffix allocation by using slice operation
+	if urlExt != "" {
+		rStem = urlFilename[:len(urlFilename)-len(urlExt)]
+	} else {
+		rStem = urlFilename
+	}
+
+	var rExt = urlExt
+
+	// If filename is provided explicitly, use it
+	if len(filename) > 0 {
+		rStem = filename
+	}
+
+	// If ext is provided explicitly, use it
+	if len(ext) > 0 {
+		rExt = ext
+	} else if len(contentType) > 0 {
+		exts, err := mime.ExtensionsByType(contentType)
+		if err == nil && len(exts) != 0 {
+			rExt = exts[0]
+		}
+	}
+
+	// If fallback is requested, and filename is still empty, override it with fallbackStem
+	if len(rStem) == 0 {
+		rStem = fallbackStem
+	}
+
+	disposition := inlineDisposition
+
+	// Create the content-disposition header value
+	if returnAttachment {
+		disposition = attachmentDisposition
+	}
+
+	return fmt.Sprintf(contentDispositionsHeader, disposition, strings.ReplaceAll(rStem, `"`, "%22"), rExt)
+}

httpheaders/cdv_test.go

@@ -0,0 +1,127 @@
+package httpheaders
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestContentDispositionValue(t *testing.T) {
+	// Test cases for ContentDispositionValue function that generates content-disposition headers
+	tests := []struct {
+		name             string
+		url              string
+		filename         string
+		ext              string
+		returnAttachment bool
+		expected         string
+		contentType      string
+	}{
+		{
+			name:             "BasicURL",
+			url:              "http://example.com/test.jpg",
+			filename:         "",
+			ext:              "",
+			contentType:      "",
+			returnAttachment: false,
+			expected:         "inline; filename=\"test.jpg\"",
+		},
+		{
+			name:             "EmptyFilename",
+			url:              "http://example.com/path/to/",
+			filename:         "",
+			ext:              "",
+			contentType:      "",
+			returnAttachment: false,
+			expected:         "inline; filename=\"image\"",
+		},
+		{
+			name:             "EmptyFilenameWithExt",
+			url:              "http://example.com/path/to/",
+			filename:         "",
+			ext:              ".png",
+			contentType:      "",
+			returnAttachment: false,
+			expected:         "inline; filename=\"image.png\"",
+		},
+		{
+			name:             "EmptyFilenameWithFilenameAndExt",
+			url:              "http://example.com/path/to/",
+			filename:         "example",
+			ext:              ".png",
+			contentType:      "",
+			returnAttachment: false,
+			expected:         "inline; filename=\"example.png\"",
+		},
+		{
+			name:             "EmptyFilenameWithFilenameOverride",
+			url:              "http://example.com/path/to/",
+			filename:         "example",
+			ext:              ".jpg",
+			contentType:      "",
+			returnAttachment: false,
+			expected:         "inline; filename=\"example.jpg\"",
+		},
+		{
+			name:             "PresentFilenameWithExtOverride",
+			url:              "http://example.com/path/to/face.png",
+			filename:         "",
+			ext:              ".jpg",
+			contentType:      "",
+			returnAttachment: false,
+			expected:         "inline; filename=\"face.jpg\"",
+		},
+		{
+			name:             "PresentFilenameWithFilenameOverride",
+			url:              "http://example.com/path/to/123.png",
+			filename:         "face",
+			ext:              ".jpg",
+			contentType:      "",
+			returnAttachment: false,
+			expected:         "inline; filename=\"face.jpg\"",
+		},
+		{
+			name:             "EmptyFilenameWithFallback",
+			url:              "http://example.com/path/to/",
+			filename:         "",
+			ext:              ".png",
+			contentType:      "",
+			returnAttachment: false,
+			expected:         "inline; filename=\"image.png\"",
+		},
+		{
+			name:             "AttachmentDisposition",
+			url:              "http://example.com/test.jpg",
+			filename:         "",
+			ext:              "",
+			contentType:      "",
+			returnAttachment: true,
+			expected:         "attachment; filename=\"test.jpg\"",
+		},
+		{
+			name:             "FilenameWithQuotes",
+			url:              "http://example.com/test.jpg",
+			filename:         "my\"file",
+			ext:              ".png",
+			returnAttachment: false,
+			contentType:      "",
+			expected:         "inline; filename=\"my%22file.png\"",
+		},
+		{
+			name:             "FilenameWithContentType",
+			url:              "http://example.com/test",
+			filename:         "my\"file",
+			ext:              "",
+			contentType:      "image/png",
+			returnAttachment: false,
+			expected:         "inline; filename=\"my%22file.png\"",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			result := ContentDispositionValue(tc.url, tc.filename, tc.ext, tc.contentType, tc.returnAttachment)
+			require.Equal(t, tc.expected, result)
+		})
+	}
+}

imagetype_new/defs.go → imagetype/defs.go

@@ -1,4 +1,4 @@
-package imagetype_new
+package imagetype
 
 var (
 	JPEG = RegisterType(&TypeDesc{

imagetype_new/errors.go → imagetype/errors.go

@@ -1,4 +1,4 @@
-package imagetype_new
+package imagetype
 
 import (
 	"net/http"

imagetype/imagetype.go

@@ -1,181 +0,0 @@
-// Code generated by gen_imagetype.go; DO NOT EDIT.
-
-package imagetype
-
-import (
-	"fmt"
-	"net/url"
-	"path/filepath"
-	"strings"
-)
-
-type Type int
-
-const (
-	Unknown Type = iota
-	JPEG
-	JXL
-	PNG
-	WEBP
-	GIF
-	ICO
-	SVG
-	HEIC
-	AVIF
-	BMP
-	TIFF
-)
-
-const (
-	contentDispositionFilenameFallback = "image"
-	contentDispositionsFmt             = "%s; filename=\"%s%s\""
-)
-
-var (
-	Types = map[string]Type{
-		"jpeg": JPEG,
-		"jpg":  JPEG,
-		"jxl":  JXL,
-		"png":  PNG,
-		"webp": WEBP,
-		"gif":  GIF,
-		"ico":  ICO,
-		"svg":  SVG,
-		"heic": HEIC,
-		"avif": AVIF,
-		"bmp":  BMP,
-		"tiff": TIFF,
-	}
-
-	mimes = map[Type]string{
-		JPEG: "image/jpeg",
-		JXL:  "image/jxl",
-		PNG:  "image/png",
-		WEBP: "image/webp",
-		GIF:  "image/gif",
-		ICO:  "image/x-icon",
-		SVG:  "image/svg+xml",
-		HEIC: "image/heif",
-		AVIF: "image/avif",
-		BMP:  "image/bmp",
-		TIFF: "image/tiff",
-	}
-
-	extensions = map[Type]string{
-		JPEG: ".jpg",
-		JXL:  ".jxl",
-		PNG:  ".png",
-		WEBP: ".webp",
-		GIF:  ".gif",
-		ICO:  ".ico",
-		SVG:  ".svg",
-		HEIC: ".heic",
-		AVIF: ".avif",
-		BMP:  ".bmp",
-		TIFF: ".tiff",
-	}
-)
-
-func (it Type) String() string {
-	// JPEG has two names, we should use only the full one
-	if it == JPEG {
-		return "jpeg"
-	}
-
-	for k, v := range Types {
-		if v == it {
-			return k
-		}
-	}
-	return ""
-}
-
-func (it Type) Ext() string {
-	if ext, ok := extensions[it]; ok {
-		return ext
-	}
-	return ""
-}
-
-func (it Type) MarshalJSON() ([]byte, error) {
-	for k, v := range Types {
-		if v == it {
-			return []byte(fmt.Sprintf("%q", k)), nil
-		}
-	}
-	return []byte("null"), nil
-}
-
-func (it Type) Mime() string {
-	if mime, ok := mimes[it]; ok {
-		return mime
-	}
-
-	return "application/octet-stream"
-}
-
-func (it Type) ContentDisposition(filename string, returnAttachment bool) string {
-	return ContentDisposition(filename, it.Ext(), returnAttachment)
-}
-
-func (it Type) ContentDispositionFromURL(imageURL string, returnAttachment bool) string {
-	url, err := url.Parse(imageURL)
-	if err != nil {
-		return it.ContentDisposition(contentDispositionFilenameFallback, returnAttachment)
-	}
-
-	_, filename := filepath.Split(url.Path)
-	if len(filename) == 0 {
-		return it.ContentDisposition(contentDispositionFilenameFallback, returnAttachment)
-	}
-
-	return it.ContentDisposition(strings.TrimSuffix(filename, filepath.Ext(filename)), returnAttachment)
-}
-
-func (it Type) IsVector() bool {
-	return it == SVG
-}
-
-func (it Type) SupportsAlpha() bool {
-	return it != JPEG
-}
-
-func (it Type) SupportsAnimationLoad() bool {
-	return it == GIF || it == WEBP || it == JXL
-}
-
-func (it Type) SupportsAnimationSave() bool {
-	return it == GIF || it == WEBP
-}
-
-func (it Type) SupportsColourProfile() bool {
-	return it == JPEG ||
-		it == JXL ||
-		it == PNG ||
-		it == WEBP ||
-		it == HEIC ||
-		it == AVIF
-}
-
-func (it Type) SupportsQuality() bool {
-	return it == JPEG ||
-		it == WEBP ||
-		it == HEIC ||
-		it == AVIF ||
-		it == JXL ||
-		it == TIFF
-}
-
-func (it Type) SupportsThumbnail() bool {
-	return it == HEIC || it == AVIF
-}
-
-func ContentDisposition(filename, ext string, returnAttachment bool) string {
-	disposition := "inline"
-
-	if returnAttachment {
-		disposition = "attachment"
-	}
-
-	return fmt.Sprintf(contentDispositionsFmt, disposition, strings.ReplaceAll(filename, `"`, "%22"), ext)
-}

imagetype_new/registry.go → imagetype/registry.go

@@ -1,4 +1,4 @@
-package imagetype_new
+package imagetype
 
 import (
 	"io"
@@ -31,13 +31,22 @@ type TypeDesc struct {
 type DetectFunc func(r bufreader.ReadPeeker) (Type, error)
 
 // Registry holds the type registry
-type Registry struct {
-	detectors []DetectFunc
-	types     []*TypeDesc
+type registry struct {
+	detectors   []DetectFunc
+	types       []*TypeDesc
+	typesByName map[string]Type // maps type string to Type
 }
 
 // globalRegistry is the default registry instance
-var globalRegistry = &Registry{}
+var globalRegistry = NewRegistry()
+
+// NewRegistry creates a new image type registry.
+func NewRegistry() *registry {
+	return &registry{
+		types:       nil,
+		typesByName: make(map[string]Type),
+	}
+}
 
 // RegisterType registers a new image type in the global registry.
 // It panics if the type already exists (i.e., if a TypeDesc is already registered for this Type).
@@ -51,16 +60,33 @@ func GetTypeDesc(t Type) *TypeDesc {
 	return globalRegistry.GetTypeDesc(t)
 }
 
+// GetTypes returns all registered image types.
+func GetTypeByName(name string) (Type, bool) {
+	return globalRegistry.GetTypeByName(name)
+}
+
 // RegisterType registers a new image type in this registry.
 // It panics if the type already exists (i.e., if a TypeDesc is already registered for this Type).
-func (r *Registry) RegisterType(desc *TypeDesc) Type {
+func (r *registry) RegisterType(desc *TypeDesc) Type {
 	r.types = append(r.types, desc)
-	return Type(len(r.types)) // 0 is unknown
+	typ := Type(len(r.types)) // 0 is unknown
+	r.typesByName[desc.String] = typ
+
+	// NOTE: this is a special case for JPEG. The problem is that JPEG is using
+	// several alternative extensions and processing_options.go is using extension to
+	// find a type by key. There might be not the only case (e.g. ".tif/.tiff").
+	// We need to handle this case in a more generic way.
+	if desc.String == "jpeg" {
+		// JPEG is a special case, we need to alias it
+		r.typesByName["jpg"] = typ
+	}
+
+	return typ
 }
 
 // GetTypeDesc returns the TypeDesc for the given Type.
 // Returns nil if the type is not registered.
-func (r *Registry) GetTypeDesc(t Type) *TypeDesc {
+func (r *registry) GetTypeDesc(t Type) *TypeDesc {
 	if t <= 0 { // This would be "default" type
 		return nil
 	}
@@ -72,6 +98,12 @@ func (r *Registry) GetTypeDesc(t Type) *TypeDesc {
 	return r.types[t-1]
 }
 
+// GetTypeByName returns Type by it's name
+func (r *registry) GetTypeByName(name string) (Type, bool) {
+	typ, ok := r.typesByName[name]
+	return typ, ok
+}
+
 // RegisterDetector registers a custom detector function
 // Detectors are tried in the order they were registered
 func RegisterDetector(detector DetectFunc) {
@@ -91,12 +123,12 @@ func Detect(r io.Reader) (Type, error) {
 }
 
 // RegisterDetector registers a custom detector function on this registry instance
-func (r *Registry) RegisterDetector(detector DetectFunc) {
+func (r *registry) RegisterDetector(detector DetectFunc) {
 	r.detectors = append(r.detectors, detector)
 }
 
 // RegisterMagicBytes registers magic bytes for a specific image type on this registry instance
-func (r *Registry) RegisterMagicBytes(typ Type, signature ...[]byte) {
+func (r *registry) RegisterMagicBytes(typ Type, signature ...[]byte) {
 	r.detectors = append(r.detectors, func(r bufreader.ReadPeeker) (Type, error) {
 		for _, sig := range signature {
 			b, err := r.Peek(len(sig))
@@ -114,7 +146,7 @@ func (r *Registry) RegisterMagicBytes(typ Type, signature ...[]byte) {
 }
 
 // Detect runs image format detection
-func (r *Registry) Detect(re io.Reader) (Type, error) {
+func (r *registry) Detect(re io.Reader) (Type, error) {
 	br := bufreader.New(io.LimitReader(re, maxDetectionLimit))
 
 	for _, fn := range globalRegistry.detectors {

imagetype_new/registry_test.go → imagetype/registry_test.go

@@ -1,4 +1,4 @@
-package imagetype_new
+package imagetype
 
 import (
 	"testing"
@@ -9,7 +9,7 @@ import (
 
 func TestRegisterType(t *testing.T) {
 	// Create a separate registry for testing to avoid conflicts with global registry
-	testRegistry := &Registry{}
+	testRegistry := NewRegistry()
 
 	// Register a custom type
 	customDesc := &TypeDesc{
@@ -141,7 +141,7 @@ func TestTypeProperties(t *testing.T) {
 
 func TestRegisterDetector(t *testing.T) {
 	// Create a test registry to avoid interfering with global state
-	testRegistry := &Registry{}
+	testRegistry := NewRegistry()
 
 	// Create a test detector function
 	testDetector := func(r bufreader.ReadPeeker) (Type, error) {
@@ -165,7 +165,7 @@ func TestRegisterDetector(t *testing.T) {
 
 func TestRegisterMagicBytes(t *testing.T) {
 	// Create a test registry to avoid interfering with global state
-	testRegistry := &Registry{}
+	testRegistry := NewRegistry()
 
 	require.Empty(t, testRegistry.detectors)
 

imagetype_new/svg.go → imagetype/svg.go

@@ -1,4 +1,4 @@
-package imagetype_new
+package imagetype
 
 import (
 	"strings"

imagetype_new/type.go → imagetype/type.go

@@ -1,4 +1,4 @@
-package imagetype_new
+package imagetype
 
 import (
 	"fmt"

imagetype_new/type_test.go → imagetype/type_test.go

@@ -1,4 +1,4 @@
-package imagetype_new
+package imagetype
 
 import (
 	"os"

options/processing_options.go

@@ -570,7 +570,7 @@ func applyFormatQualityOption(po *ProcessingOptions, args []string) error {
 	}
 
 	for i := 0; i < argsLen; i += 2 {
-		f, ok := imagetype.Types[args[i]]
+		f, ok := imagetype.GetTypeByName(args[i])
 		if !ok {
 			return newOptionArgumentError("Invalid image format: %s", args[i])
 		}
@@ -754,7 +754,7 @@ func applyFormatOption(po *ProcessingOptions, args []string) error {
 		return newOptionArgumentError("Invalid format arguments: %v", args)
 	}
 
-	if f, ok := imagetype.Types[args[0]]; ok {
+	if f, ok := imagetype.GetTypeByName(args[0]); ok {
 		po.Format = f
 	} else {
 		return newOptionArgumentError("Invalid image format: %s", args[0])
@@ -775,7 +775,7 @@ func applyCacheBusterOption(po *ProcessingOptions, args []string) error {
 
 func applySkipProcessingFormatsOption(po *ProcessingOptions, args []string) error {
 	for _, format := range args {
-		if f, ok := imagetype.Types[format]; ok {
+		if f, ok := imagetype.GetTypeByName(format); ok {
 			po.SkipProcessingFormats = append(po.SkipProcessingFormats, f)
 		} else {
 			return newOptionArgumentError("Invalid image format in skip processing: %s", format)

processing_handler.go

@@ -158,12 +158,13 @@ func respondWithImage(reqID string, r *http.Request, rw http.ResponseWriter, sta
 		checkErr(r.Context(), "image_data_size", err)
 	}
 
-	var contentDisposition string
-	if len(po.Filename) > 0 {
-		contentDisposition = resultData.Format().ContentDisposition(po.Filename, po.ReturnAttachment)
-	} else {
-		contentDisposition = resultData.Format().ContentDispositionFromURL(originURL, po.ReturnAttachment)
-	}
+	contentDisposition := httpheaders.ContentDispositionValue(
+		originURL,
+		po.Filename,
+		resultData.Format().Ext(),
+		"",
+		po.ReturnAttachment,
+	)
 
 	rw.Header().Set(httpheaders.ContentType, resultData.Format().Mime())
 	rw.Header().Set(httpheaders.ContentDisposition, contentDisposition)

stream.go

@@ -3,9 +3,7 @@ package main
 import (
 	"context"
 	"io"
-	"mime"
 	"net/http"
-	"path/filepath"
 	"strconv"
 	"sync"
 
@@ -13,8 +11,8 @@ import (
 
 	"github.com/imgproxy/imgproxy/v3/config"
 	"github.com/imgproxy/imgproxy/v3/cookies"
+	"github.com/imgproxy/imgproxy/v3/httpheaders"
 	"github.com/imgproxy/imgproxy/v3/imagedata"
-	"github.com/imgproxy/imgproxy/v3/imagetype"
 	"github.com/imgproxy/imgproxy/v3/metrics"
 	"github.com/imgproxy/imgproxy/v3/metrics/stats"
 	"github.com/imgproxy/imgproxy/v3/options"
@@ -92,26 +90,14 @@ func streamOriginImage(ctx context.Context, reqID string, r *http.Request, rw ht
 	}
 
 	if res.StatusCode < 300 {
-		var filename, ext, mimetype string
-
-		_, filename = filepath.Split(req.URL().Path)
-		ext = filepath.Ext(filename)
-
-		if len(po.Filename) > 0 {
-			filename = po.Filename
-		} else {
-			filename = filename[:len(filename)-len(ext)]
-		}
-
-		mimetype = rw.Header().Get("Content-Type")
-
-		if len(ext) == 0 && len(mimetype) > 0 {
-			if exts, err := mime.ExtensionsByType(mimetype); err == nil && len(exts) != 0 {
-				ext = exts[0]
-			}
-		}
-
-		rw.Header().Set("Content-Disposition", imagetype.ContentDisposition(filename, ext, po.ReturnAttachment))
+		contentDisposition := httpheaders.ContentDispositionValue(
+			req.URL().Path,
+			po.Filename,
+			"",
+			rw.Header().Get(httpheaders.ContentType),
+			po.ReturnAttachment,
+		)
+		rw.Header().Set("Content-Disposition", contentDisposition)
 	}
 
 	setCacheControl(rw, po.Expires, res.Header)