diff --git a/internal/core/config.go b/internal/core/config.go index 4475f3b..3017a52 100644 --- a/internal/core/config.go +++ b/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 diff --git a/internal/dims/request.go b/internal/dims/request.go index 8a9e340..80a460e 100644 --- a/internal/dims/request.go +++ b/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 } } diff --git a/internal/http/request.go b/internal/http/request.go index 2aa34bf..c3c4f51 100644 --- a/internal/http/request.go +++ b/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() diff --git a/internal/v4/request.go b/internal/v4/request.go index 4c71dd6..fdaa225 100644 --- a/internal/v4/request.go +++ b/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) diff --git a/internal/v4/request_test.go b/internal/v4/request_test.go index 7a11272..2eadd39 100644 --- a/internal/v4/request_test.go +++ b/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()) +} diff --git a/internal/v5/request.go b/internal/v5/request.go index 740af5c..10ff926 100644 --- a/internal/v5/request.go +++ b/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, diff --git a/internal/v5/request_test.go b/internal/v5/request_test.go index f61149d..7ce449f 100644 --- a/internal/v5/request_test.go +++ b/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()) +} diff --git a/pkg/dims/handler.go b/pkg/dims/handler.go index 3a1f51b..e59dd30 100644 --- a/pkg/dims/handler.go +++ b/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)