IMG-44: move skip to processing, remove imath.Min/Max (#1485)

* Removed imath.Min/Max

* Skip goes to processing

* no svg_processing error category

* should report SVG

bufpool/bufpool.go

@@ -9,7 +9,6 @@ import (
 	"sync/atomic"
 
 	"github.com/imgproxy/imgproxy/v3/config"
-	"github.com/imgproxy/imgproxy/v3/imath"
 	"github.com/imgproxy/imgproxy/v3/metrics"
 )
 
@@ -219,7 +218,7 @@ func (p *Pool) Get(size int, grow bool) *bytes.Buffer {
 
 	growSize := p.defaultSize
 	if grow {
-		growSize = imath.Max(p.normalizeCap(size), growSize)
+		growSize = max(p.normalizeCap(size), growSize)
 	}
 
 	// Grow the buffer only if we know the requested size and it is smaller than
@@ -278,7 +277,7 @@ func (p *Pool) normalizeCap(cap int) int {
 	}
 
 	ind := index(cap)
-	return imath.Max(cap, minSize<<ind)
+	return max(cap, minSize<<ind)
 }
 
 func saveEntry(e *entry) {

imagedata/image_data.go

@@ -23,12 +23,16 @@ var (
 // Please note that this interface can be backed by any reader, including lazy AsyncBuffer.
 // There is no other way to guarantee that the data is read without errors except reading it till EOF.
 type ImageData interface {
-	io.Closer                     // Close closes the image data and releases any resources held by it
-	Reader() io.ReadSeeker        // Reader returns a new ReadSeeker for the image data
-	Format() imagetype.Type       // Format returns the image format from the metadata (shortcut)
-	Size() (int, error)           // Size returns the size of the image data in bytes
-	AddCancel(context.CancelFunc) // AddCancel attaches a cancel function to the image data
-	Error() error                 // Error returns any error that occurred during reading data from source
+	io.Closer               // Close closes the image data and releases any resources held by it
+	Reader() io.ReadSeeker  // Reader returns a new ReadSeeker for the image data
+	Format() imagetype.Type // Format returns the image format from the metadata (shortcut)
+	Size() (int, error)     // Size returns the size of the image data in bytes
+	Error() error           // Error returns any error that occurred during reading data from source
+
+	// AddCancel attaches a cancel function to the image data.
+	// Please note that Cancel functions must be idempotent: for instance, an implementation
+	// could wrap cancel into sync.Once.
+	AddCancel(context.CancelFunc)
 }
 
 // imageDataBytes represents image data stored in a byte slice in memory

imath/imath.go

@@ -2,20 +2,6 @@ package imath
 
 import "math"
 
-func Max(a, b int) int {
-	if a > b {
-		return a
-	}
-	return b
-}
-
-func Min(a, b int) int {
-	if a < b {
-		return a
-	}
-	return b
-}
-
 func MinNonZero(a, b int) int {
 	switch {
 	case a == 0:
@@ -24,7 +10,7 @@ func MinNonZero(a, b int) int {
 		return a
 	}
 
-	return Min(a, b)
+	return min(a, b)
 }
 
 func Round(a float64) int {

metrics/cloudwatch/cloudwatch.go

@@ -14,7 +14,6 @@ import (
 	"github.com/sirupsen/logrus"
 
 	"github.com/imgproxy/imgproxy/v3/config"
-	"github.com/imgproxy/imgproxy/v3/imath"
 	"github.com/imgproxy/imgproxy/v3/metrics/stats"
 )
 
@@ -115,8 +114,8 @@ func ObserveBufferSize(t string, size int) {
 
 		stats.count += 1
 		stats.sum += sizef
-		stats.min = imath.Min(stats.min, sizef)
-		stats.max = imath.Max(stats.max, sizef)
+		stats.min = min(stats.min, sizef)
+		stats.max = max(stats.max, sizef)
 	}
 }
 

processing/calc_position.go

@@ -59,8 +59,8 @@ func calcPosition(width, height, innerWidth, innerHeight int, gravity *options.G
 		minY, maxY = 0, height-innerHeight
 	}
 
-	left = imath.Max(minX, imath.Min(left, maxX))
-	top = imath.Max(minY, imath.Min(top, maxY))
+	left = max(minX, min(left, maxX))
+	top = max(minY, min(top, maxY))
 
 	return
 }

processing/fix_size.go

@@ -5,7 +5,6 @@ import (
 
 	"github.com/imgproxy/imgproxy/v3/imagedata"
 	"github.com/imgproxy/imgproxy/v3/imagetype"
-	"github.com/imgproxy/imgproxy/v3/imath"
 	"github.com/imgproxy/imgproxy/v3/options"
 	"github.com/imgproxy/imgproxy/v3/vips"
 	log "github.com/sirupsen/logrus"
@@ -20,7 +19,7 @@ const (
 )
 
 func fixWebpSize(img *vips.Image) error {
-	webpLimitShrink := float64(imath.Max(img.Width(), img.Height())) / webpMaxDimension
+	webpLimitShrink := float64(max(img.Width(), img.Height())) / webpMaxDimension
 
 	if webpLimitShrink <= 1.0 {
 		return nil
@@ -37,7 +36,7 @@ func fixWebpSize(img *vips.Image) error {
 }
 
 func fixHeifSize(img *vips.Image) error {
-	heifLimitShrink := float64(imath.Max(img.Width(), img.Height())) / heifMaxDimension
+	heifLimitShrink := float64(max(img.Width(), img.Height())) / heifMaxDimension
 
 	if heifLimitShrink <= 1.0 {
 		return nil
@@ -56,7 +55,7 @@ func fixHeifSize(img *vips.Image) error {
 func fixGifSize(img *vips.Image) error {
 	gifMaxResolution := float64(vips.GifResolutionLimit())
 	gifResLimitShrink := float64(img.Width()*img.Height()) / gifMaxResolution
-	gifDimLimitShrink := float64(imath.Max(img.Width(), img.Height())) / gifMaxDimension
+	gifDimLimitShrink := float64(max(img.Width(), img.Height())) / gifMaxDimension
 
 	gifLimitShrink := math.Max(gifResLimitShrink, gifDimLimitShrink)
 
@@ -75,7 +74,7 @@ func fixGifSize(img *vips.Image) error {
 }
 
 func fixIcoSize(img *vips.Image) error {
-	icoLimitShrink := float64(imath.Max(img.Width(), img.Height())) / icoMaxDimension
+	icoLimitShrink := float64(max(img.Width(), img.Height())) / icoMaxDimension
 
 	if icoLimitShrink <= 1.0 {
 		return nil

processing/prepare.go

@@ -48,7 +48,7 @@ func calcCropSize(orig int, crop float64) int {
 	case crop >= 1.0:
 		return int(crop)
 	default:
-		return imath.Max(1, imath.Scale(orig, crop))
+		return max(1, imath.Scale(orig, crop))
 	}
 }
 
@@ -215,11 +215,11 @@ func (pctx *pipelineContext) limitScale(widthToScale, heightToScale int, po *opt
 	outHeight := imath.MinNonZero(pctx.scaledHeight, pctx.resultCropHeight)
 
 	if po.Extend.Enabled {
-		outWidth = imath.Max(outWidth, pctx.targetWidth)
-		outHeight = imath.Max(outHeight, pctx.targetHeight)
+		outWidth = max(outWidth, pctx.targetWidth)
+		outHeight = max(outHeight, pctx.targetHeight)
 	} else if po.ExtendAspectRatio.Enabled {
-		outWidth = imath.Max(outWidth, pctx.extendAspectRatioWidth)
-		outHeight = imath.Max(outHeight, pctx.extendAspectRatioHeight)
+		outWidth = max(outWidth, pctx.extendAspectRatioWidth)
+		outHeight = max(outHeight, pctx.extendAspectRatioHeight)
 	}
 
 	if po.Padding.Enabled {
@@ -228,7 +228,7 @@ func (pctx *pipelineContext) limitScale(widthToScale, heightToScale int, po *opt
 	}
 
 	if maxresultDim > 0 && (outWidth > maxresultDim || outHeight > maxresultDim) {
-		downScale := float64(maxresultDim) / float64(imath.Max(outWidth, outHeight))
+		downScale := float64(maxresultDim) / float64(max(outWidth, outHeight))
 
 		pctx.wscale *= downScale
 		pctx.hscale *= downScale

processing/processing.go

@@ -4,16 +4,17 @@ import (
 	"context"
 	"errors"
 	"runtime"
+	"slices"
 
 	log "github.com/sirupsen/logrus"
 
 	"github.com/imgproxy/imgproxy/v3/config"
 	"github.com/imgproxy/imgproxy/v3/imagedata"
 	"github.com/imgproxy/imgproxy/v3/imagetype"
-	"github.com/imgproxy/imgproxy/v3/imath"
 	"github.com/imgproxy/imgproxy/v3/options"
 	"github.com/imgproxy/imgproxy/v3/router"
 	"github.com/imgproxy/imgproxy/v3/security"
+	"github.com/imgproxy/imgproxy/v3/svg"
 	"github.com/imgproxy/imgproxy/v3/vips"
 )
 
@@ -103,7 +104,7 @@ func transformAnimated(ctx context.Context, img *vips.Image, po *options.Process
 	}
 
 	imgWidth := img.Width()
-	framesCount := imath.Min(img.Pages(), po.SecurityOptions.MaxAnimationFrames)
+	framesCount := min(img.Pages(), po.SecurityOptions.MaxAnimationFrames)
 
 	frameHeight, err := img.GetInt("page-height")
 	if err != nil {
@@ -293,10 +294,41 @@ func ProcessImage(ctx context.Context, imgdata imagedata.ImageData, po *options.
 		return nil, err
 	}
 
+	// Let's check if we should skip standard processing
+	if shouldSkipStandardProcessing(imgdata.Format(), po) {
+		// Even in this case, SVG is an exception
+		if imgdata.Format() == imagetype.SVG && config.SanitizeSvg {
+			sanitized, err := svg.Sanitize(imgdata)
+			if err != nil {
+				return nil, err
+			}
+
+			return &Result{
+				OutData:      sanitized,
+				OriginWidth:  originWidth,
+				OriginHeight: originHeight,
+				ResultWidth:  originWidth,
+				ResultHeight: originHeight,
+			}, nil
+		}
+
+		// Return the original image
+		return &Result{
+			OutData:      imgdata,
+			OriginWidth:  originWidth,
+			OriginHeight: originHeight,
+			ResultWidth:  originWidth,
+			ResultHeight: originHeight,
+		}, nil
+	}
+
 	animated := img.IsAnimated()
 	expectAlpha := !po.Flatten && (img.HasAlpha() || po.Padding.Enabled || po.Extend.Enabled)
 
 	switch {
+	case po.Format == imagetype.SVG:
+		// At this point we can't allow requested format to be SVG as we can't save SVGs
+		return nil, newSaveFormatError(po.Format)
 	case po.Format == imagetype.Unknown:
 		switch {
 		case po.PreferJxl && !animated:
@@ -380,3 +412,26 @@ func ProcessImage(ctx context.Context, imgdata imagedata.ImageData, po *options.
 		ResultHeight: img.Height(),
 	}, nil
 }
+
+// Returns true if image should not be processed as usual
+func shouldSkipStandardProcessing(inFormat imagetype.Type, po *options.ProcessingOptions) bool {
+	outFormat := po.Format
+	skipProcessingInFormatEnabled := slices.Contains(po.SkipProcessingFormats, inFormat)
+
+	if inFormat == imagetype.SVG {
+		isOutUnknown := outFormat == imagetype.Unknown
+
+		switch {
+		case outFormat == imagetype.SVG:
+			return true
+		case isOutUnknown && !config.AlwaysRasterizeSvg:
+			return true
+		case isOutUnknown && config.AlwaysRasterizeSvg && skipProcessingInFormatEnabled:
+			return true
+		default:
+			return false
+		}
+	} else {
+		return skipProcessingInFormatEnabled && (inFormat == outFormat || outFormat == imagetype.Unknown)
+	}
+}

processing/scale_on_load.go

@@ -107,10 +107,10 @@ func scaleOnLoad(pctx *pipelineContext, img *vips.Image, po *options.ProcessingO
 	// We should crop before scaling, but we scaled the image on load,
 	// so we need to adjust crop options
 	if pctx.cropWidth > 0 {
-		pctx.cropWidth = imath.Max(1, imath.Shrink(pctx.cropWidth, wpreshrink))
+		pctx.cropWidth = max(1, imath.Shrink(pctx.cropWidth, wpreshrink))
 	}
 	if pctx.cropHeight > 0 {
-		pctx.cropHeight = imath.Max(1, imath.Shrink(pctx.cropHeight, hpreshrink))
+		pctx.cropHeight = max(1, imath.Shrink(pctx.cropHeight, hpreshrink))
 	}
 	if pctx.cropGravity.Type != options.GravityFocusPoint {
 		// Adjust only when crop gravity offsets are absolute

processing/watermark.go

@@ -32,8 +32,8 @@ func prepareWatermark(wm *vips.Image, wmData imagedata.ImageData, opts *options.
 	po.Format = wmData.Format()
 
 	if opts.Scale > 0 {
-		po.Width = imath.Max(imath.ScaleToEven(imgWidth, opts.Scale), 1)
-		po.Height = imath.Max(imath.ScaleToEven(imgHeight, opts.Scale), 1)
+		po.Width = max(imath.ScaleToEven(imgWidth, opts.Scale), 1)
+		po.Height = max(imath.ScaleToEven(imgHeight, opts.Scale), 1)
 	}
 
 	if opts.ShouldReplicate() {

processing_handler.go

@@ -7,7 +7,6 @@ import (
 	"io"
 	"net/http"
 	"net/url"
-	"slices"
 	"strconv"
 	"strings"
 	"time"
@@ -24,14 +23,12 @@ import (
 	"github.com/imgproxy/imgproxy/v3/imagedata"
 	"github.com/imgproxy/imgproxy/v3/imagefetcher"
 	"github.com/imgproxy/imgproxy/v3/imagetype"
-	"github.com/imgproxy/imgproxy/v3/imath"
 	"github.com/imgproxy/imgproxy/v3/metrics"
 	"github.com/imgproxy/imgproxy/v3/metrics/stats"
 	"github.com/imgproxy/imgproxy/v3/options"
 	"github.com/imgproxy/imgproxy/v3/processing"
 	"github.com/imgproxy/imgproxy/v3/router"
 	"github.com/imgproxy/imgproxy/v3/security"
-	"github.com/imgproxy/imgproxy/v3/svg"
 	"github.com/imgproxy/imgproxy/v3/vips"
 )
 
@@ -75,7 +72,7 @@ func setCacheControl(rw http.ResponseWriter, force *time.Time, originHeaders htt
 	}
 
 	if force != nil && (ttl < 0 || force.Before(time.Now().Add(time.Duration(ttl)*time.Second))) {
-		ttl = imath.Min(config.TTL, imath.Max(0, int(time.Until(*force).Seconds())))
+		ttl = min(config.TTL, max(0, int(time.Until(*force).Seconds())))
 	}
 
 	if config.CacheControlPassthrough && ttl < 0 && originHeaders != nil {
@@ -86,7 +83,7 @@ func setCacheControl(rw http.ResponseWriter, force *time.Time, originHeaders htt
 
 		if val := originHeaders.Get(httpheaders.Expires); len(val) > 0 {
 			if t, err := time.Parse(http.TimeFormat, val); err == nil {
-				ttl = imath.Max(0, int(time.Until(t).Seconds()))
+				ttl = max(0, int(time.Until(t).Seconds()))
 			}
 		}
 	}
@@ -451,30 +448,6 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) {
 
 	checkErr(ctx, "timeout", router.CheckTimeout(ctx))
 
-	// Skip processing svg with unknown or the same destination imageType
-	// if it's not forced by AlwaysRasterizeSvg option
-	// Also skip processing if the format is in SkipProcessingFormats
-	shouldSkipProcessing := (originData.Format() == po.Format || po.Format == imagetype.Unknown) &&
-		(slices.Contains(po.SkipProcessingFormats, originData.Format()) ||
-			originData.Format() == imagetype.SVG && !config.AlwaysRasterizeSvg)
-
-	if shouldSkipProcessing {
-		if originData.Format() == imagetype.SVG && config.SanitizeSvg {
-			sanitized, svgErr := svg.Sanitize(originData)
-			checkErr(ctx, "svg_processing", svgErr)
-
-			defer sanitized.Close()
-
-			writeOriginContentLengthDebugHeader(ctx, rw, originData)
-			respondWithImage(reqID, r, rw, statusCode, sanitized, po, imageURL, originData, originHeaders)
-			return
-		}
-
-		writeOriginContentLengthDebugHeader(ctx, rw, originData)
-		respondWithImage(reqID, r, rw, statusCode, originData, po, imageURL, originData, originHeaders)
-		return
-	}
-
 	if !vips.SupportsLoad(originData.Format()) {
 		sendErrAndPanic(ctx, "processing", newInvalidURLErrorf(
 			http.StatusUnprocessableEntity,
@@ -482,14 +455,6 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) {
 		))
 	}
 
-	// At this point we can't allow requested format to be SVG as we can't save SVGs
-	if po.Format == imagetype.SVG {
-		sendErrAndPanic(ctx, "processing", newInvalidURLErrorf(
-			http.StatusUnprocessableEntity,
-			"Resulting image format is not supported: svg",
-		))
-	}
-
 	result, err := func() (*processing.Result, error) {
 		defer metrics.StartProcessingSegment(ctx, metrics.Meta{
 			metrics.MetaProcessingOptions: metricsMeta[metrics.MetaProcessingOptions],
@@ -497,15 +462,19 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) {
 		return processing.ProcessImage(ctx, originData, po)
 	}()
 
+	// Let's close resulting image data only if it differs from the source image data
+	if result != nil && result.OutData != nil && result.OutData != originData {
+		defer result.OutData.Close()
+	}
+
 	if err != nil {
 		// First, check if the processing error wasn't caused by an image data error
 		checkErr(ctx, "download", originData.Error())
+
 		// If it wasn't, than it was a processing error
 		sendErrAndPanic(ctx, "processing", err)
 	}
 
-	defer result.OutData.Close()
-
 	checkErr(ctx, "timeout", router.CheckTimeout(ctx))
 
 	writeDebugHeaders(rw, result)

security/image_size.go

@@ -1,11 +1,7 @@
 package security
 
-import (
-	"github.com/imgproxy/imgproxy/v3/imath"
-)
-
 func CheckDimensions(width, height, frames int, opts Options) error {
-	frames = imath.Max(frames, 1)
+	frames = max(frames, 1)
 
 	if frames > 1 && opts.MaxAnimationFrameResolution > 0 {
 		if width*height > opts.MaxAnimationFrameResolution {

svg/errors.go

@@ -0,0 +1,23 @@
+package svg
+
+import (
+	"net/http"
+
+	"github.com/imgproxy/imgproxy/v3/ierrors"
+)
+
+type (
+	SanitizeError struct{ error }
+)
+
+func newSanitizeError(err error) error {
+	return ierrors.Wrap(
+		SanitizeError{err},
+		1,
+		ierrors.WithStatusCode(http.StatusUnprocessableEntity),
+		ierrors.WithPublicMessage("Broken or unsupported SVG image"),
+		ierrors.WithShouldReport(true),
+	)
+}
+
+func (e SanitizeError) Unwrap() error { return e.error }

svg/svg.go

@@ -26,7 +26,7 @@ func Sanitize(data imagedata.ImageData) (imagedata.ImageData, error) {
 
 	buf, ok := pool.Get().(*bytes.Buffer)
 	if !ok {
-		return nil, errors.New("svg.Sanitize: failed to get buffer from pool")
+		return nil, newSanitizeError(errors.New("svg.Sanitize: failed to get buffer from pool"))
 	}
 	buf.Reset()
 
@@ -45,7 +45,7 @@ func Sanitize(data imagedata.ImageData) (imagedata.ImageData, error) {
 			switch tt {
 			case xml.ErrorToken:
 				cancel()
-				return nil, l.Err()
+				return nil, newSanitizeError(l.Err())
 			case xml.EndTagToken, xml.StartTagCloseVoidToken:
 				ignoreTag--
 			case xml.StartTagToken:
@@ -59,7 +59,7 @@ func Sanitize(data imagedata.ImageData) (imagedata.ImageData, error) {
 		case xml.ErrorToken:
 			if l.Err() != io.EOF {
 				cancel()
-				return nil, l.Err()
+				return nil, newSanitizeError(l.Err())
 			}
 
 			newData := imagedata.NewFromBytesWithFormat(

vips/vips.go

@@ -27,7 +27,6 @@ import (
 	"github.com/imgproxy/imgproxy/v3/ierrors"
 	"github.com/imgproxy/imgproxy/v3/imagedata"
 	"github.com/imgproxy/imgproxy/v3/imagetype"
-	"github.com/imgproxy/imgproxy/v3/imath"
 	"github.com/imgproxy/imgproxy/v3/metrics/cloudwatch"
 	"github.com/imgproxy/imgproxy/v3/metrics/datadog"
 	"github.com/imgproxy/imgproxy/v3/metrics/newrelic"
@@ -84,7 +83,7 @@ func Init() error {
 		// Set vips concurrency level to GOMAXPROCS if we are running in AWS Lambda
 		// since each function processes only one request at a time
 		// so we can use all available CPU cores
-		C.vips_concurrency_set(C.int(imath.Max(1, runtime.GOMAXPROCS(0))))
+		C.vips_concurrency_set(C.int(max(1, runtime.GOMAXPROCS(0))))
 	} else {
 		C.vips_concurrency_set(1)
 	}