Remove unused signature and etag config fields

internal/core/config.go

@@ -166,7 +166,6 @@ type Config struct {
 	DevelopmentMode bool   `env:"DIMS_DEVELOPMENT_MODE" envDefault:"false"`
 	DebugMode       bool   `env:"DIMS_DEBUG_MODE" envDefault:"false"`
 	LogFormat       string `env:"DIMS_LOG_FORMAT" envDefault:"text"`
-	EtagAlgorithm   string
 
 	Timeout
 	Limits

internal/dims/request.go

@@ -26,7 +26,6 @@ type Request struct {
 	LegacyParams           []string    // Values named by _keys, in _keys order. Legacy mode only.
 	SourceImage            core.Image  // The source image.
 	config                 core.Config // The global configuration.
-	shrinkFactor           int
 	commands               []commands.Command
 }
 
@@ -76,8 +75,6 @@ func (r *Request) LoadImage(sourceImage *core.Image) (*vips.ImageRef, error) {
 	importParams := vips.NewImportParams()
 	importParams.AutoRotate.Set(true)
 
-	r.shrinkFactor = 1
-
 	// Shrink on load helps only a JPEG that is much larger than the request.
 	// Read the header for that case alone, so every other image loads once.
 	requestedSize, err := r.requestedImageSize()
@@ -95,7 +92,6 @@ func (r *Request) LoadImage(sourceImage *core.Image) (*vips.ImageRef, error) {
 		// size and has to be scaled back up.
 		if shrink := shrinkFactor(min(xs, ys)); shrink > 1 {
 			importParams.JpegShrinkFactor.Set(shrink)
-			r.shrinkFactor = shrink
 		}
 	}
 

internal/http/request.go

@@ -28,6 +28,12 @@ type Request struct {
 	httpRequest  *http.Request
 	httpResponse http.ResponseWriter
 
+	// EtagAlgorithm names the digest the endpoint uses for its ETag. It is a
+	// property of the endpoint, so each one sets it on its own request. Held
+	// on the shared configuration it was a field one handler wrote while
+	// another read it.
+	EtagAlgorithm string
+
 	// committed records that the status line has been written. Writing it
 	// twice logs a superfluous WriteHeader and appends a second image to a
 	// partly written response.
@@ -240,7 +246,7 @@ func (r *Request) CacheControl() string {
 func (r *Request) Etag() string {
 	if r.SourceImage.Etag != "" {
 		var h hash.Hash
-		if r.Config().EtagAlgorithm == "md5" {
+		if r.EtagAlgorithm == "md5" {
 			h = md5.New()
 		} else {
 			h = sha256.New()

internal/v4/request.go

@@ -28,6 +28,7 @@ func NewRequest(r *http.Request, w http.ResponseWriter, config core.Config) (*Re
 	}
 
 	request.Signature = r.PathValue("signature")
+	request.EtagAlgorithm = "md5"
 
 	return &Request{
 		Request: request,
@@ -37,15 +38,6 @@ func NewRequest(r *http.Request, w http.ResponseWriter, config core.Config) (*Re
 	}, nil
 }
 
-func (v4 *Request) HashId() string {
-	h := md5.New()
-	h.Write([]byte(v4.clientId))
-	h.Write([]byte(v4.RawCommands))
-	h.Write([]byte(v4.ImageUrl))
-
-	return fmt.Sprintf("%x", h.Sum(nil))
-}
-
 func (v4 *Request) Validate() bool {
 	expectedSignature := v4.sign(v4.RawCommands, v4.timestamp, v4.ImageUrl, v4.SignedQuery, v4.Config().SigningKey)
 

internal/v4/request_test.go

@@ -175,3 +175,20 @@ func TestValueCannotForgeASeparator(t *testing.T) {
 	require.Equal(t, "a=1%26b%3D2", forged.SignedQuery)
 	require.NotEqual(t, signatureOf(forged), signatureOf(real))
 }
+
+// The ETag digest belongs to the endpoint, not to the configuration. v4 uses
+// md5. Held on the configuration it went unset on the Lambda path, so the
+// same URL produced a different ETag there.
+//
+// HashId comes from the embedded request, which covers the commands and the
+// image URL.
+func TestEtagUsesMd5(t *testing.T) {
+	request := newTestRequest(t, query(""), "")
+	request.SourceImage.Etag = `"origin"`
+
+	digest := md5.New()
+	digest.Write([]byte(request.HashId()))
+	digest.Write([]byte(`"origin"`))
+
+	require.Equal(t, fmt.Sprintf("%q", fmt.Sprintf("%x", digest.Sum(nil))), request.Etag())
+}

internal/v5/request.go

@@ -22,6 +22,7 @@ func NewRequest(r *http.Request, w http.ResponseWriter, config core.Config) (*Re
 	}
 
 	request.Signature = r.URL.Query().Get("sig")
+	request.EtagAlgorithm = "hmac-sha256"
 
 	return &Request{
 		Request: request,

internal/v5/request_test.go

@@ -4,6 +4,7 @@ import (
 	"crypto/hmac"
 	"crypto/sha256"
 	"encoding/hex"
+	"fmt"
 	"net/http"
 	"net/url"
 	"testing"
@@ -162,3 +163,16 @@ func TestValueCannotForgeASeparator(t *testing.T) {
 	require.Equal(t, "a=1%26b%3D2", forged.SignedQuery)
 	require.NotEqual(t, signatureOf(forged), signatureOf(real))
 }
+
+// The ETag digest belongs to the endpoint, not to the configuration. v5 uses
+// sha256.
+func TestEtagUsesSha256(t *testing.T) {
+	request := newTestRequest(t, query(""), "")
+	request.SourceImage.Etag = `"origin"`
+
+	digest := sha256.New()
+	digest.Write([]byte(request.HashId()))
+	digest.Write([]byte(`"origin"`))
+
+	require.Equal(t, fmt.Sprintf("%q", fmt.Sprintf("%x", digest.Sum(nil))), request.Etag())
+}

pkg/dims/handler.go

@@ -17,18 +17,10 @@ func NewHandler(config core.Config) http.Handler {
 
 	slog.Debug("startup", "config", config)
 
-	// Each endpoint holds its own copy. A handler must not write to a shared
-	// configuration value while other requests read it.
-	v4Config := config
-	v4Config.EtagAlgorithm = "md5"
-
-	v5Config := config
-	v5Config.EtagAlgorithm = "hmac-sha256"
-
 	// v4 endpoint
 	mux.HandleFunc("/dims4/{clientId}/{signature}/{timestamp}/{commands...}",
 		func(w http.ResponseWriter, r *http.Request) {
-			request, err := v4.NewRequest(r, w, v4Config)
+			request, err := v4.NewRequest(r, w, config)
 			if err != nil {
 				w.WriteHeader(400)
 				slog.Error("error parsing request", "error", err)
@@ -46,7 +38,7 @@ func NewHandler(config core.Config) http.Handler {
 	// v5 endpoint
 	mux.HandleFunc("/v5/{commands...}",
 		func(w http.ResponseWriter, r *http.Request) {
-			request, err := v5.NewRequest(r, w, v5Config)
+			request, err := v5.NewRequest(r, w, config)
 			if err != nil {
 				w.WriteHeader(400)
 				slog.Error("error parsing request", "error", err)