Ver Fonte

headerwriter in processing_handler.go (#1507)

* headerwriter in processing_handler.go

* Remove not required etag tests

* ETagEnabled, LastModifiedEnabled true by default

* Changed Passthrough signature

* Removed etag package

* Merge writeDebugHeaders*
Victor Sokolov há 1 mês atrás
pai
commit
c6a95facbb

+ 14 - 0
CHANGELOG.v4.md

@@ -0,0 +1,14 @@
+# 📑 Changelog (version/4 dev)
+
+## ✨ 2025-08-27
+
+### 🔄 Changed
+
+- `If-None-Match` is passed through to server request, `Etag` passed through from server response
+if `IMGPROXY_USE_ETAG` is true.
+- `IMGPROXY_USE_ETAG` is now true by default.
+- `IMGPROXY_USE_LAST_MODIFIED` is now true by default.
+
+### ❌ Removed
+
+- `Etag` calculations on the imgproxy side

+ 2 - 2
config/config.go

@@ -351,10 +351,10 @@ func Reset() {
 	SwiftConnectTimeoutSeconds = 10
 	SwiftTimeoutSeconds = 60
 
-	ETagEnabled = false
+	ETagEnabled = true
 	ETagBuster = ""
 
-	LastModifiedEnabled = false
+	LastModifiedEnabled = true
 
 	BaseURL = ""
 	URLReplacements = make([]URLReplacement, 0)

+ 0 - 160
etag/etag.go

@@ -1,160 +0,0 @@
-package etag
-
-import (
-	"crypto/sha256"
-	"encoding/base64"
-	"encoding/json"
-	"fmt"
-	"hash"
-	"io"
-	"net/http"
-	"net/textproto"
-	"strings"
-	"sync"
-
-	"github.com/imgproxy/imgproxy/v3/config"
-	"github.com/imgproxy/imgproxy/v3/httpheaders"
-	"github.com/imgproxy/imgproxy/v3/imagedata"
-	"github.com/imgproxy/imgproxy/v3/options"
-)
-
-type eTagCalc struct {
-	hash hash.Hash
-	enc  *json.Encoder
-}
-
-var eTagCalcPool = sync.Pool{
-	New: func() interface{} {
-		h := sha256.New()
-
-		enc := json.NewEncoder(h)
-		enc.SetEscapeHTML(false)
-		enc.SetIndent("", "")
-
-		return &eTagCalc{h, enc}
-	},
-}
-
-type Handler struct {
-	poHashActual, poHashExpected string
-
-	imgEtagActual, imgEtagExpected string
-	imgHashActual, imgHashExpected string
-}
-
-func (h *Handler) ParseExpectedETag(etag string) {
-	// We suuport only a single ETag value
-	if i := strings.IndexByte(etag, ','); i >= 0 {
-		etag = textproto.TrimString(etag[:i])
-	}
-
-	etagLen := len(etag)
-
-	// ETag is empty or invalid
-	if etagLen < 2 {
-		return
-	}
-
-	// We support strong ETags only
-	if etag[0] != '"' || etag[etagLen-1] != '"' {
-		return
-	}
-
-	// Remove quotes
-	etag = etag[1 : etagLen-1]
-
-	i := strings.Index(etag, "/")
-	if i < 0 || i > etagLen-3 {
-		// Doesn't look like imgproxy ETag
-		return
-	}
-
-	poPart, imgPartMark, imgPart := etag[:i], etag[i+1], etag[i+2:]
-
-	switch imgPartMark {
-	case 'R':
-		imgPartDec, err := base64.RawStdEncoding.DecodeString(imgPart)
-		if err == nil {
-			h.imgEtagExpected = string(imgPartDec)
-		}
-	case 'D':
-		h.imgHashExpected = imgPart
-	default:
-		// Unknown image part mark
-		return
-	}
-
-	h.poHashExpected = poPart
-}
-
-func (h *Handler) ProcessingOptionsMatch() bool {
-	return h.poHashActual == h.poHashExpected
-}
-
-func (h *Handler) SetActualProcessingOptions(po *options.ProcessingOptions) bool {
-	c := eTagCalcPool.Get().(*eTagCalc)
-	defer eTagCalcPool.Put(c)
-
-	c.hash.Reset()
-	c.hash.Write([]byte(config.ETagBuster))
-	c.enc.Encode(po)
-
-	h.poHashActual = base64.RawURLEncoding.EncodeToString(c.hash.Sum(nil))
-
-	return h.ProcessingOptionsMatch()
-}
-
-func (h *Handler) ImageEtagExpected() string {
-	return h.imgEtagExpected
-}
-
-func (h *Handler) SetActualImageData(imgdata imagedata.ImageData, headers http.Header) (bool, error) {
-	var haveActualImgETag bool
-	h.imgEtagActual = headers.Get(httpheaders.Etag)
-	haveActualImgETag = len(h.imgEtagActual) > 0
-
-	// Just in case server didn't check ETag properly and returned the same one
-	// as we expected
-	if haveActualImgETag && h.imgEtagExpected == h.imgEtagActual {
-		return true, nil
-	}
-
-	haveExpectedImgHash := len(h.imgHashExpected) != 0
-
-	if !haveActualImgETag || haveExpectedImgHash {
-		c := eTagCalcPool.Get().(*eTagCalc)
-		defer eTagCalcPool.Put(c)
-
-		c.hash.Reset()
-
-		_, err := io.Copy(c.hash, imgdata.Reader())
-		if err != nil {
-			return false, err
-		}
-
-		h.imgHashActual = base64.RawURLEncoding.EncodeToString(c.hash.Sum(nil))
-
-		return haveExpectedImgHash && h.imgHashActual == h.imgHashExpected, nil
-	}
-
-	return false, nil
-}
-
-func (h *Handler) GenerateActualETag() string {
-	return h.generate(h.poHashActual, h.imgEtagActual, h.imgHashActual)
-}
-
-func (h *Handler) GenerateExpectedETag() string {
-	return h.generate(h.poHashExpected, h.imgEtagExpected, h.imgHashExpected)
-}
-
-func (h *Handler) generate(poHash, imgEtag, imgHash string) string {
-	imgPartMark := 'D'
-	imgPart := imgHash
-	if len(imgEtag) != 0 {
-		imgPartMark = 'R'
-		imgPart = base64.RawURLEncoding.EncodeToString([]byte(imgEtag))
-	}
-
-	return fmt.Sprintf(`"%s/%c%s"`, poHash, imgPartMark, imgPart)
-}

+ 0 - 159
etag/etag_test.go

@@ -1,159 +0,0 @@
-package etag
-
-import (
-	"io"
-	"net/http"
-	"os"
-	"strings"
-	"testing"
-
-	"github.com/sirupsen/logrus"
-	"github.com/stretchr/testify/suite"
-
-	"github.com/imgproxy/imgproxy/v3/config"
-	"github.com/imgproxy/imgproxy/v3/httpheaders"
-	"github.com/imgproxy/imgproxy/v3/imagedata"
-	"github.com/imgproxy/imgproxy/v3/options"
-)
-
-const (
-	etagReq  = `"yj0WO6sFU4GCciYUBWjzvvfqrBh869doeOC2Pp5EI1Y/RImxvcmVtaXBzdW1kb2xvciI"`
-	etagData = `"yj0WO6sFU4GCciYUBWjzvvfqrBh869doeOC2Pp5EI1Y/D3t8wWhX4piqDCV4ZMEZsKvOaIO6onhKjbf9f-ZfYUV0"`
-)
-
-type EtagTestSuite struct {
-	suite.Suite
-
-	po                    *options.ProcessingOptions
-	imgWithETag           imagedata.ImageData
-	imgWithEtagHeaders    http.Header
-	imgWithoutETag        imagedata.ImageData
-	imgWithoutEtagHeaders http.Header
-
-	h Handler
-}
-
-func (s *EtagTestSuite) SetupSuite() {
-	logrus.SetOutput(io.Discard)
-	s.po = options.NewProcessingOptions()
-
-	d, err := os.ReadFile("../testdata/test1.jpg")
-	s.Require().NoError(err)
-
-	imgWithETag, err := imagedata.NewFromBytes(d)
-	s.Require().NoError(err)
-	s.imgWithEtagHeaders = make(http.Header)
-	s.imgWithEtagHeaders.Add(httpheaders.Etag, `"loremipsumdolor"`)
-
-	imgWithoutETag, err := imagedata.NewFromBytes(d)
-	s.imgWithoutEtagHeaders = make(http.Header)
-	s.Require().NoError(err)
-
-	s.imgWithETag = imgWithETag
-	s.imgWithoutETag = imgWithoutETag
-}
-
-func (s *EtagTestSuite) TeardownSuite() {
-	logrus.SetOutput(os.Stdout)
-}
-
-func (s *EtagTestSuite) SetupTest() {
-	s.h = Handler{}
-	config.Reset()
-}
-
-func (s *EtagTestSuite) TestGenerateActualReq() {
-	s.h.SetActualProcessingOptions(s.po)
-	s.h.SetActualImageData(s.imgWithETag, s.imgWithEtagHeaders)
-
-	s.Require().Equal(etagReq, s.h.GenerateActualETag())
-}
-
-func (s *EtagTestSuite) TestGenerateActualData() {
-	s.h.SetActualProcessingOptions(s.po)
-	s.h.SetActualImageData(s.imgWithoutETag, s.imgWithoutEtagHeaders)
-
-	s.Require().Equal(etagData, s.h.GenerateActualETag())
-}
-
-func (s *EtagTestSuite) TestGenerateExpectedReq() {
-	s.h.ParseExpectedETag(etagReq)
-	s.Require().Equal(etagReq, s.h.GenerateExpectedETag())
-}
-
-func (s *EtagTestSuite) TestGenerateExpectedData() {
-	s.h.ParseExpectedETag(etagData)
-	s.Require().Equal(etagData, s.h.GenerateExpectedETag())
-}
-
-func (s *EtagTestSuite) TestProcessingOptionsCheckSuccess() {
-	s.h.ParseExpectedETag(etagReq)
-
-	s.Require().True(s.h.SetActualProcessingOptions(s.po))
-	s.Require().True(s.h.ProcessingOptionsMatch())
-}
-
-func (s *EtagTestSuite) TestProcessingOptionsCheckFailure() {
-	i := strings.Index(etagReq, "/")
-	wrongEtag := `"wrongpohash` + etagReq[i:]
-
-	s.h.ParseExpectedETag(wrongEtag)
-
-	s.Require().False(s.h.SetActualProcessingOptions(s.po))
-	s.Require().False(s.h.ProcessingOptionsMatch())
-}
-
-func (s *EtagTestSuite) TestImageETagExpectedPresent() {
-	s.h.ParseExpectedETag(etagReq)
-
-	//nolint:testifylint // False-positive expected-actual
-	s.Require().Equal(s.imgWithEtagHeaders.Get(httpheaders.Etag), s.h.ImageEtagExpected())
-}
-
-func (s *EtagTestSuite) TestImageETagExpectedBlank() {
-	s.h.ParseExpectedETag(etagData)
-
-	s.Require().Empty(s.h.ImageEtagExpected())
-}
-
-func (s *EtagTestSuite) TestImageDataCheckDataToDataSuccess() {
-	s.h.ParseExpectedETag(etagData)
-	s.Require().True(s.h.SetActualImageData(s.imgWithoutETag, s.imgWithoutEtagHeaders))
-}
-
-func (s *EtagTestSuite) TestImageDataCheckDataToDataFailure() {
-	i := strings.Index(etagData, "/")
-	wrongEtag := etagData[:i] + `/Dwrongimghash"`
-
-	s.h.ParseExpectedETag(wrongEtag)
-	s.Require().False(s.h.SetActualImageData(s.imgWithoutETag, s.imgWithoutEtagHeaders))
-}
-
-func (s *EtagTestSuite) TestImageDataCheckDataToReqSuccess() {
-	s.h.ParseExpectedETag(etagData)
-	s.Require().True(s.h.SetActualImageData(s.imgWithETag, s.imgWithEtagHeaders))
-}
-
-func (s *EtagTestSuite) TestImageDataCheckDataToReqFailure() {
-	i := strings.Index(etagData, "/")
-	wrongEtag := etagData[:i] + `/Dwrongimghash"`
-
-	s.h.ParseExpectedETag(wrongEtag)
-	s.Require().False(s.h.SetActualImageData(s.imgWithETag, s.imgWithEtagHeaders))
-}
-
-func (s *EtagTestSuite) TestImageDataCheckReqToDataFailure() {
-	s.h.ParseExpectedETag(etagReq)
-	s.Require().False(s.h.SetActualImageData(s.imgWithoutETag, s.imgWithoutEtagHeaders))
-}
-
-func (s *EtagTestSuite) TestETagBusterFailure() {
-	config.ETagBuster = "busted"
-
-	s.h.ParseExpectedETag(etagReq)
-	s.Require().False(s.h.SetActualImageData(s.imgWithoutETag, s.imgWithoutEtagHeaders))
-}
-
-func TestEtag(t *testing.T) {
-	suite.Run(t, new(EtagTestSuite))
-}

+ 9 - 11
handlers/stream/handler.go

@@ -118,14 +118,16 @@ func (s *request) execute(ctx context.Context) error {
 	// Output streaming response headers
 	hw := s.handler.hw.NewRequest(res.Header, s.imageURL)
 
-	hw.Passthrough(s.handler.config.PassthroughResponseHeaders) // NOTE: priority? This is lowest as it was
+	hw.Passthrough(s.handler.config.PassthroughResponseHeaders...) // NOTE: priority? This is lowest as it was
 	hw.SetContentLength(int(res.ContentLength))
 	hw.SetCanonical()
 	hw.SetExpires(s.po.Expires)
-	hw.Write(s.rw)
 
-	// Write Content-Disposition header
-	s.writeContentDisposition(r.URL().Path, res)
+	// Set the Content-Disposition header
+	s.setContentDisposition(r.URL().Path, res, hw)
+
+	// Write headers from writer
+	hw.Write(s.rw)
 
 	// Copy the status code from the original response
 	s.rw.WriteHeader(res.StatusCode)
@@ -154,8 +156,8 @@ func (s *request) getImageRequestHeaders() http.Header {
 	return h
 }
 
-// writeContentDisposition writes the headers to the response writer
-func (s *request) writeContentDisposition(imagePath string, serverResponse *http.Response) {
+// setContentDisposition writes the headers to the response writer
+func (s *request) setContentDisposition(imagePath string, serverResponse *http.Response, hw *headerwriter.Request) {
 	// Try to set correct Content-Disposition file name and extension
 	if serverResponse.StatusCode < 200 || serverResponse.StatusCode >= 300 {
 		return
@@ -163,17 +165,13 @@ func (s *request) writeContentDisposition(imagePath string, serverResponse *http
 
 	ct := serverResponse.Header.Get(httpheaders.ContentType)
 
-	// Try to best guess the file name and extension
-	cd := httpheaders.ContentDispositionValue(
+	hw.SetContentDisposition(
 		imagePath,
 		s.po.Filename,
 		"",
 		ct,
 		s.po.ReturnAttachment,
 	)
-
-	// Write the Content-Disposition header
-	s.rw.Header().Set(httpheaders.ContentDisposition, cd)
 }
 
 // streamData copies the image data from the response body to the response writer

+ 0 - 3
headerwriter/config.go

@@ -12,7 +12,6 @@ type Config struct {
 	DefaultTTL              int  // Default Cache-Control max-age= value for cached images
 	FallbackImageTTL        int  // TTL for images served as fallbacks
 	CacheControlPassthrough bool // Passthrough the Cache-Control from the original response
-	LastModifiedEnabled     bool // Set the Last-Modified header
 	EnableClientHints       bool // Enable Vary header
 	SetVaryAccept           bool // Whether to include Accept in Vary header
 }
@@ -23,7 +22,6 @@ func NewDefaultConfig() *Config {
 		SetCanonicalHeader:      false,
 		DefaultTTL:              31536000,
 		FallbackImageTTL:        0,
-		LastModifiedEnabled:     false,
 		CacheControlPassthrough: false,
 		EnableClientHints:       false,
 		SetVaryAccept:           false,
@@ -35,7 +33,6 @@ func (c *Config) LoadFromEnv() (*Config, error) {
 	c.SetCanonicalHeader = config.SetCanonicalHeader
 	c.DefaultTTL = config.TTL
 	c.FallbackImageTTL = config.FallbackImageTTL
-	c.LastModifiedEnabled = config.LastModifiedEnabled
 	c.CacheControlPassthrough = config.CacheControlPassthrough
 	c.EnableClientHints = config.EnableClientHints
 	c.SetVaryAccept = config.AutoWebp ||

+ 60 - 59
headerwriter/writer.go

@@ -17,8 +17,8 @@ type Writer struct {
 	varyValue string
 }
 
-// writer is a private struct that builds HTTP response headers for a specific request.
-type writer struct {
+// Request is a private struct that builds HTTP response headers for a specific request.
+type Request struct {
 	writer                  *Writer
 	originalResponseHeaders http.Header // Original response headers
 	result                  http.Header // Headers to be written to the response
@@ -51,8 +51,8 @@ func New(config *Config) (*Writer, error) {
 }
 
 // NewRequest creates a new header writer instance for a specific request with the provided origin headers and URL.
-func (w *Writer) NewRequest(originalResponseHeaders http.Header, url string) *writer {
-	return &writer{
+func (w *Writer) NewRequest(originalResponseHeaders http.Header, url string) *Request {
+	return &Request{
 		writer:                  w,
 		originalResponseHeaders: originalResponseHeaders,
 		url:                     url,
@@ -63,123 +63,124 @@ func (w *Writer) NewRequest(originalResponseHeaders http.Header, url string) *wr
 
 // SetIsFallbackImage sets the Fallback-Image header to
 // indicate that the fallback image was used.
-func (w *writer) SetIsFallbackImage() {
+func (r *Request) SetIsFallbackImage() {
 	// We set maxAge to FallbackImageTTL if it's explicitly passed
-	if w.writer.config.FallbackImageTTL < 0 {
+	if r.writer.config.FallbackImageTTL < 0 {
 		return
 	}
 
 	// However, we should not overwrite existing value if set (or greater than ours)
-	if w.maxAge < 0 || w.maxAge > w.writer.config.FallbackImageTTL {
-		w.maxAge = w.writer.config.FallbackImageTTL
+	if r.maxAge < 0 || r.maxAge > r.writer.config.FallbackImageTTL {
+		r.maxAge = r.writer.config.FallbackImageTTL
 	}
 }
 
 // SetExpires sets the TTL from time
-func (w *writer) SetExpires(expires *time.Time) {
+func (r *Request) SetExpires(expires *time.Time) {
 	if expires == nil {
 		return
 	}
 
 	// Convert current maxAge to time
-	currentMaxAgeTime := time.Now().Add(time.Duration(w.maxAge) * time.Second)
+	currentMaxAgeTime := time.Now().Add(time.Duration(r.maxAge) * time.Second)
 
 	// If maxAge outlives expires or was not set, we'll use expires as maxAge.
-	if w.maxAge < 0 || expires.Before(currentMaxAgeTime) {
-		w.maxAge = min(w.writer.config.DefaultTTL, max(0, int(time.Until(*expires).Seconds())))
+	if r.maxAge < 0 || expires.Before(currentMaxAgeTime) {
+		r.maxAge = min(r.writer.config.DefaultTTL, max(0, int(time.Until(*expires).Seconds())))
 	}
 }
 
-// SetLastModified sets the Last-Modified header from request
-func (w *writer) SetLastModified() {
-	if !w.writer.config.LastModifiedEnabled {
-		return
-	}
-
-	val := w.originalResponseHeaders.Get(httpheaders.LastModified)
-	if len(val) == 0 {
-		return
+// SetVary sets the Vary header
+func (r *Request) SetVary() {
+	if len(r.writer.varyValue) > 0 {
+		r.result.Set(httpheaders.Vary, r.writer.varyValue)
 	}
-
-	w.result.Set(httpheaders.LastModified, val)
 }
 
-// SetVary sets the Vary header
-func (w *writer) SetVary() {
-	if len(w.writer.varyValue) > 0 {
-		w.result.Set(httpheaders.Vary, w.writer.varyValue)
+// SetContentDisposition sets the Content-Disposition header, passthrough to ContentDispositionValue
+func (r *Request) SetContentDisposition(originURL, filename, ext, contentType string, returnAttachment bool) {
+	value := httpheaders.ContentDispositionValue(
+		originURL,
+		filename,
+		ext,
+		contentType,
+		returnAttachment,
+	)
+
+	if value != "" {
+		r.result.Set(httpheaders.ContentDisposition, value)
 	}
 }
 
 // Passthrough copies specified headers from the original response headers to the response headers.
-func (w *writer) Passthrough(only []string) {
-	httpheaders.Copy(w.originalResponseHeaders, w.result, only)
+func (r *Request) Passthrough(only ...string) {
+	httpheaders.Copy(r.originalResponseHeaders, r.result, only)
 }
 
 // CopyFrom copies specified headers from the headers object. Please note that
 // all the past operations may overwrite those values.
-func (w *writer) CopyFrom(headers http.Header, only []string) {
-	httpheaders.Copy(headers, w.result, only)
+func (r *Request) CopyFrom(headers http.Header, only []string) {
+	httpheaders.Copy(headers, r.result, only)
 }
 
 // SetContentLength sets the Content-Length header
-func (w *writer) SetContentLength(contentLength int) {
+func (r *Request) SetContentLength(contentLength int) {
 	if contentLength < 0 {
 		return
 	}
 
-	w.result.Set(httpheaders.ContentLength, strconv.Itoa(contentLength))
+	r.result.Set(httpheaders.ContentLength, strconv.Itoa(contentLength))
 }
 
 // SetContentType sets the Content-Type header
-func (w *writer) SetContentType(mime string) {
-	w.result.Set(httpheaders.ContentType, mime)
+func (r *Request) SetContentType(mime string) {
+	r.result.Set(httpheaders.ContentType, mime)
 }
 
 // writeCanonical sets the Link header with the canonical URL.
 // It is mandatory for any response if enabled in the configuration.
-func (w *writer) SetCanonical() {
-	if !w.writer.config.SetCanonicalHeader {
+func (r *Request) SetCanonical() {
+	if !r.writer.config.SetCanonicalHeader {
 		return
 	}
 
-	if strings.HasPrefix(w.url, "https://") || strings.HasPrefix(w.url, "http://") {
-		value := fmt.Sprintf(`<%s>; rel="canonical"`, w.url)
-		w.result.Set(httpheaders.Link, value)
+	if strings.HasPrefix(r.url, "https://") || strings.HasPrefix(r.url, "http://") {
+		value := fmt.Sprintf(`<%s>; rel="canonical"`, r.url)
+		r.result.Set(httpheaders.Link, value)
 	}
 }
 
 // setCacheControl sets the Cache-Control header with the specified value.
-func (w *writer) setCacheControl(value int) bool {
+func (r *Request) setCacheControl(value int) bool {
 	if value <= 0 {
 		return false
 	}
 
-	w.result.Set(httpheaders.CacheControl, fmt.Sprintf("max-age=%d, public", value))
+	r.result.Set(httpheaders.CacheControl, fmt.Sprintf("max-age=%d, public", value))
 	return true
 }
 
 // setCacheControlNoCache sets the Cache-Control header to no-cache (default).
-func (w *writer) setCacheControlNoCache() {
-	w.result.Set(httpheaders.CacheControl, "no-cache")
+func (r *Request) setCacheControlNoCache() {
+	r.result.Set(httpheaders.CacheControl, "no-cache")
 }
 
 // setCacheControlPassthrough sets the Cache-Control header from the request
 // if passthrough is enabled in the configuration.
-func (w *writer) setCacheControlPassthrough() bool {
-	if !w.writer.config.CacheControlPassthrough || w.maxAge > 0 {
+func (r *Request) setCacheControlPassthrough() bool {
+	if !r.writer.config.CacheControlPassthrough || r.maxAge > 0 {
 		return false
 	}
 
-	if val := w.originalResponseHeaders.Get(httpheaders.CacheControl); val != "" {
-		w.result.Set(httpheaders.CacheControl, val)
+	if val := r.originalResponseHeaders.Get(httpheaders.CacheControl); val != "" {
+		r.result.Set(httpheaders.CacheControl, val)
 		return true
 	}
 
-	if val := w.originalResponseHeaders.Get(httpheaders.Expires); val != "" {
+	if val := r.originalResponseHeaders.Get(httpheaders.Expires); val != "" {
 		if t, err := time.Parse(http.TimeFormat, val); err == nil {
 			maxAge := max(0, int(time.Until(t).Seconds()))
-			return w.setCacheControl(maxAge)
+			return r.setCacheControl(maxAge)
 		}
 	}
 
@@ -187,24 +188,24 @@ func (w *writer) setCacheControlPassthrough() bool {
 }
 
 // setCSP sets the Content-Security-Policy header to prevent script execution.
-func (w *writer) setCSP() {
-	w.result.Set(httpheaders.ContentSecurityPolicy, "script-src 'none'")
+func (r *Request) setCSP() {
+	r.result.Set(httpheaders.ContentSecurityPolicy, "script-src 'none'")
 }
 
 // Write writes the headers to the response writer. It does not overwrite
 // target headers, which were set outside the header writer.
-func (w *writer) Write(rw http.ResponseWriter) {
+func (r *Request) Write(rw http.ResponseWriter) {
 	// Then, let's try to set Cache-Control using priority order
 	switch {
-	case w.setCacheControl(w.maxAge): // First, try set explicit
-	case w.setCacheControlPassthrough(): // Try to pick up from request headers
-	case w.setCacheControl(w.writer.config.DefaultTTL): // Fallback to default value
+	case r.setCacheControl(r.maxAge): // First, try set explicit
+	case r.setCacheControlPassthrough(): // Try to pick up from request headers
+	case r.setCacheControl(r.writer.config.DefaultTTL): // Fallback to default value
 	default:
-		w.setCacheControlNoCache() // By default we use no-cache
+		r.setCacheControlNoCache() // By default we use no-cache
 	}
 
-	w.setCSP()
+	r.setCSP()
 
 	// Copy all headers to the response without overwriting existing ones
-	httpheaders.CopyAll(w.result, rw.Header(), false)
+	httpheaders.CopyAll(r.result, rw.Header(), false)
 }

+ 15 - 34
headerwriter/writer_test.go

@@ -23,7 +23,7 @@ type writerTestCase struct {
 	req    http.Header
 	res    http.Header
 	config Config
-	fn     func(*writer)
+	fn     func(*Request)
 }
 
 func (s *HeaderWriterSuite) TestHeaderCases() {
@@ -45,7 +45,6 @@ func (s *HeaderWriterSuite) TestHeaderCases() {
 				SetCanonicalHeader:      false,
 				DefaultTTL:              0,
 				CacheControlPassthrough: false,
-				LastModifiedEnabled:     false,
 				EnableClientHints:       false,
 				SetVaryAccept:           false,
 			},
@@ -105,7 +104,7 @@ func (s *HeaderWriterSuite) TestHeaderCases() {
 				SetCanonicalHeader: true,
 				DefaultTTL:         3600,
 			},
-			fn: func(w *writer) {
+			fn: func(w *Request) {
 				w.SetCanonical()
 			},
 		},
@@ -134,28 +133,10 @@ func (s *HeaderWriterSuite) TestHeaderCases() {
 				SetCanonicalHeader: false,
 				DefaultTTL:         3600,
 			},
-			fn: func(w *writer) {
+			fn: func(w *Request) {
 				w.SetCanonical()
 			},
 		},
-		{
-			name: "LastModified",
-			req: http.Header{
-				httpheaders.LastModified: []string{expires.Format(http.TimeFormat)},
-			},
-			res: http.Header{
-				httpheaders.LastModified:          []string{expires.Format(http.TimeFormat)},
-				httpheaders.ContentSecurityPolicy: []string{"script-src 'none'"},
-				httpheaders.CacheControl:          []string{"max-age=3600, public"},
-			},
-			config: Config{
-				LastModifiedEnabled: true,
-				DefaultTTL:          3600,
-			},
-			fn: func(w *writer) {
-				w.SetLastModified()
-			},
-		},
 		{
 			name: "SetMaxAgeTTL",
 			req:  http.Header{},
@@ -167,7 +148,7 @@ func (s *HeaderWriterSuite) TestHeaderCases() {
 				DefaultTTL:       3600,
 				FallbackImageTTL: 1,
 			},
-			fn: func(w *writer) {
+			fn: func(w *Request) {
 				w.SetIsFallbackImage()
 			},
 		},
@@ -181,7 +162,7 @@ func (s *HeaderWriterSuite) TestHeaderCases() {
 			config: Config{
 				DefaultTTL: math.MaxInt32,
 			},
-			fn: func(w *writer) {
+			fn: func(w *Request) {
 				w.SetExpires(&expires)
 			},
 		},
@@ -196,7 +177,7 @@ func (s *HeaderWriterSuite) TestHeaderCases() {
 				DefaultTTL:       math.MaxInt32,
 				FallbackImageTTL: 600,
 			},
-			fn: func(w *writer) {
+			fn: func(w *Request) {
 				w.SetIsFallbackImage()
 				w.SetExpires(&shortExpires)
 			},
@@ -213,7 +194,7 @@ func (s *HeaderWriterSuite) TestHeaderCases() {
 				EnableClientHints: true,
 				SetVaryAccept:     true,
 			},
-			fn: func(w *writer) {
+			fn: func(w *Request) {
 				w.SetVary()
 			},
 		},
@@ -228,8 +209,8 @@ func (s *HeaderWriterSuite) TestHeaderCases() {
 				httpheaders.ContentSecurityPolicy: []string{"script-src 'none'"},
 			},
 			config: Config{},
-			fn: func(w *writer) {
-				w.Passthrough([]string{"X-Test"})
+			fn: func(w *Request) {
+				w.Passthrough("X-Test")
 			},
 		},
 		{
@@ -241,7 +222,7 @@ func (s *HeaderWriterSuite) TestHeaderCases() {
 				httpheaders.ContentSecurityPolicy: []string{"script-src 'none'"},
 			},
 			config: Config{},
-			fn: func(w *writer) {
+			fn: func(w *Request) {
 				h := http.Header{}
 				h.Set("X-From", "baz")
 				w.CopyFrom(h, []string{"X-From"})
@@ -256,7 +237,7 @@ func (s *HeaderWriterSuite) TestHeaderCases() {
 				httpheaders.ContentSecurityPolicy: []string{"script-src 'none'"},
 			},
 			config: Config{},
-			fn: func(w *writer) {
+			fn: func(w *Request) {
 				w.SetContentLength(123)
 			},
 		},
@@ -269,7 +250,7 @@ func (s *HeaderWriterSuite) TestHeaderCases() {
 				httpheaders.ContentSecurityPolicy: []string{"script-src 'none'"},
 			},
 			config: Config{},
-			fn: func(w *writer) {
+			fn: func(w *Request) {
 				w.SetContentType("image/png")
 			},
 		},
@@ -283,7 +264,7 @@ func (s *HeaderWriterSuite) TestHeaderCases() {
 			config: Config{
 				DefaultTTL: 3600,
 			},
-			fn: func(w *writer) {
+			fn: func(w *Request) {
 				w.SetExpires(nil)
 			},
 		},
@@ -298,7 +279,7 @@ func (s *HeaderWriterSuite) TestHeaderCases() {
 			config: Config{
 				SetVaryAccept: true,
 			},
-			fn: func(w *writer) {
+			fn: func(w *Request) {
 				w.SetVary()
 			},
 		},
@@ -313,7 +294,7 @@ func (s *HeaderWriterSuite) TestHeaderCases() {
 			config: Config{
 				EnableClientHints: true,
 			},
-			fn: func(w *writer) {
+			fn: func(w *Request) {
 				w.SetVary()
 			},
 		},

+ 3 - 1
httpheaders/copy.go

@@ -1,6 +1,8 @@
 package httpheaders
 
-import "net/http"
+import (
+	"net/http"
+)
 
 // Copy copies specified headers from one header to another.
 func Copy(from, to http.Header, only []string) {

+ 1 - 1
main.go

@@ -45,7 +45,7 @@ func buildRouter(r *server.Router) *server.Router {
 	}
 
 	r.GET(
-		"/*", handleProcessing,
+		"/*", callHandleProcessing,
 		r.WithSecret, r.WithCORS, r.WithPanic, r.WithReportError, r.WithMonitoring,
 	)
 

+ 98 - 189
processing_handler.go

@@ -2,13 +2,11 @@ package main
 
 import (
 	"errors"
-	"fmt"
 	"io"
 	"net/http"
 	"net/url"
 	"strconv"
 	"strings"
-	"time"
 
 	log "github.com/sirupsen/logrus"
 	"golang.org/x/sync/semaphore"
@@ -16,7 +14,6 @@ import (
 	"github.com/imgproxy/imgproxy/v3/config"
 	"github.com/imgproxy/imgproxy/v3/cookies"
 	"github.com/imgproxy/imgproxy/v3/errorreport"
-	"github.com/imgproxy/imgproxy/v3/etag"
 	"github.com/imgproxy/imgproxy/v3/handlers/stream"
 	"github.com/imgproxy/imgproxy/v3/headerwriter"
 	"github.com/imgproxy/imgproxy/v3/httpheaders"
@@ -36,8 +33,6 @@ import (
 var (
 	queueSem      *semaphore.Weighted
 	processingSem *semaphore.Weighted
-
-	headerVaryValue string
 )
 
 func initProcessingHandler() {
@@ -46,88 +41,22 @@ func initProcessingHandler() {
 	}
 
 	processingSem = semaphore.NewWeighted(int64(config.Workers))
-
-	vary := make([]string, 0)
-
-	if config.AutoWebp ||
-		config.EnforceWebp ||
-		config.AutoAvif ||
-		config.EnforceAvif ||
-		config.AutoJxl ||
-		config.EnforceJxl {
-		vary = append(vary, "Accept")
-	}
-
-	if config.EnableClientHints {
-		vary = append(vary, "Sec-CH-DPR", "DPR", "Sec-CH-Width", "Width")
-	}
-
-	headerVaryValue = strings.Join(vary, ", ")
-}
-
-func setCacheControl(rw http.ResponseWriter, force *time.Time, originHeaders http.Header) {
-	ttl := -1
-
-	if _, ok := originHeaders["Fallback-Image"]; ok && config.FallbackImageTTL > 0 {
-		ttl = config.FallbackImageTTL
-	}
-
-	if force != nil && (ttl < 0 || force.Before(time.Now().Add(time.Duration(ttl)*time.Second))) {
-		ttl = min(config.TTL, max(0, int(time.Until(*force).Seconds())))
-	}
-
-	if config.CacheControlPassthrough && ttl < 0 && originHeaders != nil {
-		if val := originHeaders.Get(httpheaders.CacheControl); len(val) > 0 {
-			rw.Header().Set(httpheaders.CacheControl, val)
-			return
-		}
-
-		if val := originHeaders.Get(httpheaders.Expires); len(val) > 0 {
-			if t, err := time.Parse(http.TimeFormat, val); err == nil {
-				ttl = max(0, int(time.Until(t).Seconds()))
-			}
-		}
-	}
-
-	if ttl < 0 {
-		ttl = config.TTL
-	}
-
-	if ttl > 0 {
-		rw.Header().Set(httpheaders.CacheControl, fmt.Sprintf("max-age=%d, public", ttl))
-	} else {
-		rw.Header().Set(httpheaders.CacheControl, "no-cache")
-	}
 }
 
-func setLastModified(rw http.ResponseWriter, originHeaders http.Header) {
-	if config.LastModifiedEnabled {
-		if val := originHeaders.Get(httpheaders.LastModified); len(val) != 0 {
-			rw.Header().Set(httpheaders.LastModified, val)
-		}
-	}
-}
-
-func setVary(rw http.ResponseWriter) {
-	if len(headerVaryValue) > 0 {
-		rw.Header().Set(httpheaders.Vary, headerVaryValue)
-	}
-}
-
-func setCanonical(rw http.ResponseWriter, originURL string) {
-	if config.SetCanonicalHeader {
-		if strings.HasPrefix(originURL, "https://") || strings.HasPrefix(originURL, "http://") {
-			linkHeader := fmt.Sprintf(`<%s>; rel="canonical"`, originURL)
-			rw.Header().Set("Link", linkHeader)
-		}
-	}
-}
-
-func writeOriginContentLengthDebugHeader(rw http.ResponseWriter, originData imagedata.ImageData) error {
+// writeDebugHeaders writes debug headers (X-Origin-*, X-Result-*) to the response
+func writeDebugHeaders(rw http.ResponseWriter, result *processing.Result, originData imagedata.ImageData) error {
 	if !config.EnableDebugHeaders {
 		return nil
 	}
 
+	if result != nil {
+		rw.Header().Set(httpheaders.XOriginWidth, strconv.Itoa(result.OriginWidth))
+		rw.Header().Set(httpheaders.XOriginHeight, strconv.Itoa(result.OriginHeight))
+		rw.Header().Set(httpheaders.XResultWidth, strconv.Itoa(result.ResultWidth))
+		rw.Header().Set(httpheaders.XResultHeight, strconv.Itoa(result.ResultHeight))
+	}
+
+	// Try to read origin image size
 	size, err := originData.Size()
 	if err != nil {
 		return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryImageDataSize))
@@ -138,18 +67,16 @@ func writeOriginContentLengthDebugHeader(rw http.ResponseWriter, originData imag
 	return nil
 }
 
-func writeDebugHeaders(rw http.ResponseWriter, result *processing.Result) {
-	if !config.EnableDebugHeaders || result == nil {
-		return
-	}
-
-	rw.Header().Set(httpheaders.XOriginWidth, strconv.Itoa(result.OriginWidth))
-	rw.Header().Set(httpheaders.XOriginHeight, strconv.Itoa(result.OriginHeight))
-	rw.Header().Set(httpheaders.XResultWidth, strconv.Itoa(result.ResultWidth))
-	rw.Header().Set(httpheaders.XResultHeight, strconv.Itoa(result.ResultHeight))
-}
-
-func respondWithImage(reqID string, r *http.Request, rw http.ResponseWriter, statusCode int, resultData imagedata.ImageData, po *options.ProcessingOptions, originURL string, originData imagedata.ImageData, originHeaders http.Header) error {
+func respondWithImage(
+	reqID string,
+	r *http.Request,
+	rw http.ResponseWriter,
+	statusCode int,
+	resultData imagedata.ImageData,
+	po *options.ProcessingOptions,
+	originURL string,
+	hw *headerwriter.Request,
+) error {
 	// We read the size of the image data here, so we can set Content-Length header.
 	// This indireclty ensures that the image data is fully read from the source, no
 	// errors happened.
@@ -158,25 +85,29 @@ func respondWithImage(reqID string, r *http.Request, rw http.ResponseWriter, sta
 		return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryImageDataSize))
 	}
 
-	contentDisposition := httpheaders.ContentDispositionValue(
+	hw.SetContentType(resultData.Format().Mime())
+	hw.SetContentLength(resultSize)
+	hw.SetContentDisposition(
 		originURL,
 		po.Filename,
 		resultData.Format().Ext(),
 		"",
 		po.ReturnAttachment,
 	)
+	hw.SetExpires(po.Expires)
+	hw.SetVary()
+	hw.SetCanonical()
 
-	rw.Header().Set(httpheaders.ContentType, resultData.Format().Mime())
-	rw.Header().Set(httpheaders.ContentDisposition, contentDisposition)
+	if config.LastModifiedEnabled {
+		hw.Passthrough(httpheaders.LastModified)
+	}
 
-	setCacheControl(rw, po.Expires, originHeaders)
-	setLastModified(rw, originHeaders)
-	setVary(rw)
-	setCanonical(rw, originURL)
+	if config.ETagEnabled {
+		hw.Passthrough(httpheaders.Etag)
+	}
 
-	rw.Header().Set(httpheaders.ContentSecurityPolicy, "script-src 'none'")
+	hw.Write(rw)
 
-	rw.Header().Set(httpheaders.ContentLength, strconv.Itoa(resultSize))
 	rw.WriteHeader(statusCode)
 
 	_, err = io.Copy(rw, resultData.Reader())
@@ -201,13 +132,20 @@ func respondWithImage(reqID string, r *http.Request, rw http.ResponseWriter, sta
 	return nil
 }
 
-func respondWithNotModified(reqID string, r *http.Request, rw http.ResponseWriter, po *options.ProcessingOptions, originURL string, originHeaders http.Header) {
-	setCacheControl(rw, po.Expires, originHeaders)
-	setVary(rw)
+func respondWithNotModified(reqID string, r *http.Request, rw http.ResponseWriter, po *options.ProcessingOptions, originURL string, hw *headerwriter.Request) {
+	hw.SetExpires(po.Expires)
+	hw.SetVary()
+
+	if config.ETagEnabled {
+		hw.Passthrough(httpheaders.Etag)
+	}
+
+	hw.Write(rw)
+
+	rw.WriteHeader(http.StatusNotModified)
 
-	rw.WriteHeader(304)
 	server.LogResponse(
-		reqID, r, 304, nil,
+		reqID, r, http.StatusNotModified, nil,
 		log.Fields{
 			"image_url":          originURL,
 			"processing_options": po,
@@ -215,7 +153,32 @@ func respondWithNotModified(reqID string, r *http.Request, rw http.ResponseWrite
 	)
 }
 
-func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) error {
+func callHandleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) error {
+	// NOTE: This is temporary, will be moved level up at once
+	hwc, err := headerwriter.NewDefaultConfig().LoadFromEnv()
+	if err != nil {
+		return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryConfig))
+	}
+
+	hw, err := headerwriter.New(hwc)
+	if err != nil {
+		return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryConfig))
+	}
+
+	sc, err := stream.NewDefaultConfig().LoadFromEnv()
+	if err != nil {
+		return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryConfig))
+	}
+
+	stream, err := stream.New(sc, hw, imagedata.Fetcher)
+	if err != nil {
+		return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryConfig))
+	}
+
+	return handleProcessing(reqID, rw, r, hw, stream)
+}
+
+func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request, hw *headerwriter.Writer, stream *stream.Handler) error {
 	stats.IncRequestsInProgress()
 	defer stats.DecRequestsInProgress()
 
@@ -277,30 +240,7 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) err
 	}
 
 	if po.Raw {
-		// NOTE: This is temporary, there would be no categoryConfig once we
-		// finish with refactoring.
-		// TODO: Move this up
-		cfg, cerr := stream.NewDefaultConfig().LoadFromEnv()
-		if cerr != nil {
-			return ierrors.Wrap(cerr, 0, ierrors.WithCategory(categoryConfig))
-		}
-
-		hwc, cerr := headerwriter.NewDefaultConfig().LoadFromEnv()
-		if cerr != nil {
-			return ierrors.Wrap(cerr, 0, ierrors.WithCategory(categoryConfig))
-		}
-
-		hw, cerr := headerwriter.New(hwc)
-		if cerr != nil {
-			return ierrors.Wrap(cerr, 0, ierrors.WithCategory(categoryConfig))
-		}
-
-		handler, cerr := stream.New(cfg, hw, imagedata.Fetcher)
-		if cerr != nil {
-			return ierrors.Wrap(cerr, 0, ierrors.WithCategory(categoryConfig))
-		}
-
-		return handler.Execute(ctx, r, imageURL, reqID, po, rw)
+		return stream.Execute(ctx, r, imageURL, reqID, po, rw)
 	}
 
 	// SVG is a special case. Though saving to svg is not supported, SVG->SVG is.
@@ -313,22 +253,14 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) err
 
 	imgRequestHeader := make(http.Header)
 
-	var etagHandler etag.Handler
-
+	// If ETag is enabled, we forward If-None-Match header
 	if config.ETagEnabled {
-		etagHandler.ParseExpectedETag(r.Header.Get("If-None-Match"))
-
-		if etagHandler.SetActualProcessingOptions(po) {
-			if imgEtag := etagHandler.ImageEtagExpected(); len(imgEtag) != 0 {
-				imgRequestHeader.Set("If-None-Match", imgEtag)
-			}
-		}
+		imgRequestHeader.Set(httpheaders.IfNoneMatch, r.Header.Get(httpheaders.IfNoneMatch))
 	}
 
+	// If LastModified is enabled, we forward If-Modified-Since header
 	if config.LastModifiedEnabled {
-		if modifiedSince := r.Header.Get("If-Modified-Since"); len(modifiedSince) != 0 {
-			imgRequestHeader.Set("If-Modified-Since", modifiedSince)
-		}
+		imgRequestHeader.Set(httpheaders.IfModifiedSince, r.Header.Get(httpheaders.IfModifiedSince))
 	}
 
 	if queueSem != nil {
@@ -393,27 +325,28 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) err
 		return imagedata.DownloadAsync(ctx, imageURL, "source image", downloadOpts)
 	}()
 
-	var nmErr imagefetcher.NotModifiedError
-
-	switch {
-	case err == nil:
+	// Close originData if no error occurred
+	if err == nil {
 		defer originData.Close()
+	}
 
-	case errors.As(err, &nmErr):
-		if config.ETagEnabled && len(etagHandler.ImageEtagExpected()) != 0 {
-			rw.Header().Set(httpheaders.Etag, etagHandler.GenerateExpectedETag())
-		}
+	// Check that image detection didn't take too long
+	if terr := server.CheckTimeout(ctx); terr != nil {
+		return ierrors.Wrap(terr, 0, ierrors.WithCategory(categoryTimeout))
+	}
 
-		respondWithNotModified(reqID, r, rw, po, imageURL, nmErr.Headers())
-		return nil
+	var nmErr imagefetcher.NotModifiedError
 
-	default:
-		// This may be a request timeout error or a request cancelled error.
-		// Check it before moving further
-		if terr := server.CheckTimeout(ctx); terr != nil {
-			return ierrors.Wrap(terr, 0, ierrors.WithCategory(categoryTimeout))
-		}
+	// Respond with NotModified if image was not modified
+	if errors.As(err, &nmErr) {
+		hwr := hw.NewRequest(nmErr.Headers(), imageURL)
+
+		respondWithNotModified(reqID, r, rw, po, imageURL, hwr)
+		return nil
+	}
 
+	// If error is not related to NotModified, respond with fallback image
+	if err != nil {
 		ierr := ierrors.Wrap(err, 0, ierrors.WithCategory(categoryDownload))
 		if config.ReportDownloadingErrors {
 			ierr = ierrors.Wrap(ierr, 0, ierrors.WithShouldReport(true))
@@ -447,28 +380,6 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) err
 		}
 	}
 
-	if terr := server.CheckTimeout(ctx); terr != nil {
-		return ierrors.Wrap(terr, 0, ierrors.WithCategory(categoryTimeout))
-	}
-
-	if config.ETagEnabled && statusCode == http.StatusOK {
-		imgDataMatch, eerr := etagHandler.SetActualImageData(originData, originHeaders)
-		if eerr != nil && config.ReportIOErrors {
-			return ierrors.Wrap(eerr, 0, ierrors.WithCategory(categoryIO))
-		}
-
-		rw.Header().Set("ETag", etagHandler.GenerateActualETag())
-
-		if imgDataMatch && etagHandler.ProcessingOptionsMatch() {
-			respondWithNotModified(reqID, r, rw, po, imageURL, originHeaders)
-			return nil
-		}
-	}
-
-	if terr := server.CheckTimeout(ctx); terr != nil {
-		return ierrors.Wrap(terr, 0, ierrors.WithCategory(categoryTimeout))
-	}
-
 	if !vips.SupportsLoad(originData.Format()) {
 		return ierrors.Wrap(newInvalidURLErrorf(
 			http.StatusUnprocessableEntity,
@@ -496,18 +407,16 @@ func handleProcessing(reqID string, rw http.ResponseWriter, r *http.Request) err
 		return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryProcessing))
 	}
 
-	if terr := server.CheckTimeout(ctx); terr != nil {
-		return ierrors.Wrap(terr, 0, ierrors.WithCategory(categoryTimeout))
-	}
-
-	writeDebugHeaders(rw, result)
+	hwr := hw.NewRequest(originHeaders, imageURL)
 
-	err = writeOriginContentLengthDebugHeader(rw, originData)
+	// Write debug headers. It seems unlogical to move they to headerwriter since they're
+	// not used anywhere else.
+	err = writeDebugHeaders(rw, result, originData)
 	if err != nil {
 		return ierrors.Wrap(err, 0, ierrors.WithCategory(categoryImageDataSize))
 	}
 
-	err = respondWithImage(reqID, r, rw, statusCode, result.OutData, po, imageURL, originData, originHeaders)
+	err = respondWithImage(reqID, r, rw, statusCode, result.OutData, po, imageURL, hwr)
 	if err != nil {
 		return err
 	}

+ 7 - 177
processing_handler_test.go

@@ -8,7 +8,6 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
-	"strings"
 	"testing"
 	"time"
 
@@ -17,11 +16,9 @@ import (
 
 	"github.com/imgproxy/imgproxy/v3/config"
 	"github.com/imgproxy/imgproxy/v3/config/configurators"
-	"github.com/imgproxy/imgproxy/v3/etag"
 	"github.com/imgproxy/imgproxy/v3/httpheaders"
 	"github.com/imgproxy/imgproxy/v3/imagedata"
 	"github.com/imgproxy/imgproxy/v3/imagetype"
-	"github.com/imgproxy/imgproxy/v3/options"
 	"github.com/imgproxy/imgproxy/v3/server"
 	"github.com/imgproxy/imgproxy/v3/svg"
 	"github.com/imgproxy/imgproxy/v3/testutil"
@@ -103,34 +100,6 @@ func (s *ProcessingHandlerTestSuite) readTestImageData(name string) imagedata.Im
 	return imgdata
 }
 
-func (s *ProcessingHandlerTestSuite) readImageData(imgdata imagedata.ImageData) []byte {
-	data, err := io.ReadAll(imgdata.Reader())
-	s.Require().NoError(err)
-	return data
-}
-
-func (s *ProcessingHandlerTestSuite) sampleETagData(imgETag string) (string, imagedata.ImageData, http.Header, string) {
-	poStr := "rs:fill:4:4"
-
-	po := options.NewProcessingOptions()
-	po.ResizingType = options.ResizeFill
-	po.Width = 4
-	po.Height = 4
-
-	imgdata := s.readTestImageData("test1.png")
-	headers := make(http.Header)
-
-	if len(imgETag) != 0 {
-		headers.Set(httpheaders.Etag, imgETag)
-	}
-
-	var h etag.Handler
-
-	h.SetActualProcessingOptions(po)
-	h.SetActualImageData(imgdata, headers)
-	return poStr, imgdata, headers, h.GenerateActualETag()
-}
-
 func (s *ProcessingHandlerTestSuite) TestRequest() {
 	rw := s.send("/unsafe/rs:fill:4:4/plain/local:///test1.png")
 	res := rw.Result()
@@ -411,166 +380,27 @@ func (s *ProcessingHandlerTestSuite) TestETagDisabled() {
 	s.Require().Empty(res.Header.Get("ETag"))
 }
 
-func (s *ProcessingHandlerTestSuite) TestETagReqNoIfNotModified() {
-	config.ETagEnabled = true
-
-	poStr, _, headers, etag := s.sampleETagData("loremipsumdolor")
-
-	ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
-		s.Empty(r.Header.Get("If-None-Match"))
-
-		rw.Header().Set("ETag", headers.Get(httpheaders.Etag))
-		rw.WriteHeader(200)
-		rw.Write(s.readTestFile("test1.png"))
-	}))
-	defer ts.Close()
-
-	rw := s.send(fmt.Sprintf("/unsafe/%s/plain/%s", poStr, ts.URL))
-	res := rw.Result()
-
-	s.Require().Equal(200, res.StatusCode)
-	s.Require().Equal(etag, res.Header.Get("ETag"))
-}
-
-func (s *ProcessingHandlerTestSuite) TestETagDataNoIfNotModified() {
-	config.ETagEnabled = true
-
-	poStr, imgdata, _, etag := s.sampleETagData("")
-
-	ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
-		s.Empty(r.Header.Get("If-None-Match"))
-
-		rw.WriteHeader(200)
-		rw.Write(s.readImageData(imgdata))
-	}))
-	defer ts.Close()
-
-	rw := s.send(fmt.Sprintf("/unsafe/%s/plain/%s", poStr, ts.URL))
-	res := rw.Result()
-
-	s.Require().Equal(200, res.StatusCode)
-	s.Require().Equal(etag, res.Header.Get("ETag"))
-}
-
-func (s *ProcessingHandlerTestSuite) TestETagReqMatch() {
-	config.ETagEnabled = true
-
-	poStr, _, headers, etag := s.sampleETagData(`"loremipsumdolor"`)
-
-	ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
-		s.Equal(headers.Get(httpheaders.Etag), r.Header.Get(httpheaders.IfNoneMatch))
-
-		rw.WriteHeader(304)
-	}))
-	defer ts.Close()
-
-	header := make(http.Header)
-	header.Set("If-None-Match", etag)
-
-	rw := s.send(fmt.Sprintf("/unsafe/%s/plain/%s", poStr, ts.URL), header)
-	res := rw.Result()
-
-	s.Require().Equal(304, res.StatusCode)
-	s.Require().Equal(etag, res.Header.Get("ETag"))
-}
-
 func (s *ProcessingHandlerTestSuite) TestETagDataMatch() {
 	config.ETagEnabled = true
 
-	poStr, imgdata, _, etag := s.sampleETagData("")
+	etag := `"loremipsumdolor"`
 
 	ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
-		s.Empty(r.Header.Get("If-None-Match"))
+		s.NotEmpty(r.Header.Get(httpheaders.IfNoneMatch))
 
-		rw.WriteHeader(200)
-		rw.Write(s.readImageData(imgdata))
+		rw.Header().Set(httpheaders.Etag, etag)
+		rw.WriteHeader(http.StatusNotModified)
 	}))
 	defer ts.Close()
 
 	header := make(http.Header)
-	header.Set("If-None-Match", etag)
+	header.Set(httpheaders.IfNoneMatch, etag)
 
-	rw := s.send(fmt.Sprintf("/unsafe/%s/plain/%s", poStr, ts.URL), header)
+	rw := s.send(fmt.Sprintf("/unsafe/plain/%s", ts.URL), header)
 	res := rw.Result()
 
 	s.Require().Equal(304, res.StatusCode)
-	s.Require().Equal(etag, res.Header.Get("ETag"))
-}
-
-func (s *ProcessingHandlerTestSuite) TestETagReqNotMatch() {
-	config.ETagEnabled = true
-
-	poStr, imgdata, headers, actualETag := s.sampleETagData(`"loremipsumdolor"`)
-	_, _, _, expectedETag := s.sampleETagData(`"loremipsum"`)
-
-	ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
-		s.Equal(`"loremipsum"`, r.Header.Get("If-None-Match"))
-
-		rw.Header().Set("ETag", headers.Get(httpheaders.Etag))
-		rw.WriteHeader(200)
-		rw.Write(s.readImageData(imgdata))
-	}))
-	defer ts.Close()
-
-	header := make(http.Header)
-	header.Set("If-None-Match", expectedETag)
-
-	rw := s.send(fmt.Sprintf("/unsafe/%s/plain/%s", poStr, ts.URL), header)
-	res := rw.Result()
-
-	s.Require().Equal(200, res.StatusCode)
-	s.Require().Equal(actualETag, res.Header.Get("ETag"))
-}
-
-func (s *ProcessingHandlerTestSuite) TestETagDataNotMatch() {
-	config.ETagEnabled = true
-
-	poStr, imgdata, _, actualETag := s.sampleETagData("")
-	// Change the data hash
-	expectedETag := actualETag[:strings.IndexByte(actualETag, '/')] + "/Dasdbefj"
-
-	ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
-		s.Empty(r.Header.Get("If-None-Match"))
-
-		rw.WriteHeader(200)
-		rw.Write(s.readImageData(imgdata))
-	}))
-	defer ts.Close()
-
-	header := make(http.Header)
-	header.Set("If-None-Match", expectedETag)
-
-	rw := s.send(fmt.Sprintf("/unsafe/%s/plain/%s", poStr, ts.URL), header)
-	res := rw.Result()
-
-	s.Require().Equal(200, res.StatusCode)
-	s.Require().Equal(actualETag, res.Header.Get("ETag"))
-}
-
-func (s *ProcessingHandlerTestSuite) TestETagProcessingOptionsNotMatch() {
-	config.ETagEnabled = true
-
-	poStr, imgdata, headers, actualETag := s.sampleETagData("")
-	// Change the processing options hash
-	expectedETag := "abcdefj" + actualETag[strings.IndexByte(actualETag, '/'):]
-
-	ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
-		s.Empty(r.Header.Get("If-None-Match"))
-
-		rw.Header().Set("ETag", headers.Get(httpheaders.Etag))
-		rw.WriteHeader(200)
-		rw.Write(s.readImageData(imgdata))
-	}))
-	defer ts.Close()
-
-	header := make(http.Header)
-	header.Set("If-None-Match", expectedETag)
-
-	rw := s.send(fmt.Sprintf("/unsafe/%s/plain/%s", poStr, ts.URL), header)
-	res := rw.Result()
-
-	s.Require().Equal(200, res.StatusCode)
-	s.Require().Equal(actualETag, res.Header.Get("ETag"))
+	s.Require().Equal(etag, res.Header.Get(httpheaders.Etag))
 }
 
 func (s *ProcessingHandlerTestSuite) TestLastModifiedEnabled() {