Take the signature, the expiry and the URL scheme off the image request (closes #314)

The image request now holds only its source and its transform, exactly what
the variant key covers. The /v1/image/ handler checks the allowlist and
verifies the signature itself with Signer.Verify(req, sig, expires). Both
image handlers keep the URL's expiry as a local value for Cache-Control. The
image service holds no signer, allowlist or AllowHTTP, and ValidateRequest is
gone. The fetcher takes the source and builds its URL with Source.URL from its
own AllowHTTP, so no URL string is built and parsed again. chooseAutoFormat
returns the format it chooses instead of writing into the request. The signed
URL format and every route's answers are unchanged.

Model: opus-5-5

internal/handlers/image.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"net/url"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/go-chi/chi/v5/middleware"
@@ -27,13 +28,13 @@ func (s *Handlers) HandleImage() http.HandlerFunc {
 			return
 		}
 
-		req, ok := s.parseImageRequest(w, r)
+		req, sig, expires, ok := s.parseImageRequest(w, r)
 		if !ok {
 			return
 		}
 
 		// Validate signature if required
-		err := s.imgSvc.ValidateRequest(req)
+		err := s.validateSignature(req, sig, expires)
 		if err != nil {
 			s.log.Warn("signature validation failed",
 				"host", req.Source.Host,
@@ -46,10 +47,13 @@ func (s *Handlers) HandleImage() http.HandlerFunc {
 		}
 
 		// The signature covers the format auto, not the format chosen
-		if !s.chooseAutoFormat(w, r, req) {
+		format, ok := s.chooseAutoFormat(w, r, req.Transform.Format)
+		if !ok {
 			return
 		}
 
+		req.Transform.Format = format
+
 		// Get cache key for logging
 		cacheKey := imagerequest.CacheKey(req)
 
@@ -65,7 +69,7 @@ func (s *Handlers) HandleImage() http.HandlerFunc {
 
 		defer func() { _ = resp.Content.Close() }()
 
-		s.writeImageResponse(w, r, req, resp, cacheKey, startTime)
+		s.writeImageResponse(w, r, req, expires, resp, cacheKey, startTime)
 	}
 }
 
@@ -82,11 +86,11 @@ func (s *Handlers) HandleRobotsTxt() http.HandlerFunc {
 }
 
 // parseImageRequest parses the image URL's path and query parameters into
-// an image request. On invalid input it writes an error response and
-// returns false.
+// an image request, and returns it with the URL's signature and expiry. On
+// invalid input it writes an error response and returns false.
 func (s *Handlers) parseImageRequest(
 	w http.ResponseWriter, r *http.Request,
-) (*imagerequest.Request, bool) {
+) (*imagerequest.Request, string, time.Time, bool) {
 	// The path is parsed decoded once, as r.URL.Path holds it, and not from
 	// chi's wildcard: chi routes on the path as the client wrote it whenever
 	// that is not Go's own encoding of it, so its wildcard would still hold
@@ -100,7 +104,7 @@ func (s *Handlers) parseImageRequest(
 		)
 		s.respondError(w, "invalid image URL: "+err.Error(), http.StatusBadRequest)
 
-		return nil, false
+		return nil, "", time.Time{}, false
 	}
 
 	// Parse signature params from query string. r.URL.Query() would silently
@@ -113,7 +117,7 @@ func (s *Handlers) parseImageRequest(
 		s.respondError(w, fmt.Sprintf("invalid query string %q: %v",
 			r.URL.RawQuery, err), http.StatusBadRequest)
 
-		return nil, false
+		return nil, "", time.Time{}, false
 	}
 
 	for name, values := range query {
@@ -121,17 +125,15 @@ func (s *Handlers) parseImageRequest(
 			s.respondError(w, fmt.Sprintf("invalid %s: given more than once",
 				name), http.StatusBadRequest)
 
-			return nil, false
+			return nil, "", time.Time{}, false
 		}
 	}
 
-	req.Signature = query.Get("sig")
-
-	req.Expires, err = parseExpires(query)
+	expires, err := parseExpires(query)
 	if err != nil {
 		s.respondError(w, err.Error(), http.StatusBadRequest)
 
-		return nil, false
+		return nil, "", time.Time{}, false
 	}
 
 	// Parse optional quality and fit params. Only a q missing from the URL is
@@ -143,7 +145,7 @@ func (s *Handlers) parseImageRequest(
 		s.respondError(w, `invalid q: not a number, got ""`,
 			http.StatusBadRequest)
 
-		return nil, false
+		return nil, "", time.Time{}, false
 	}
 
 	req.Transform.Quality, err = parseFormInt(query, "q",
@@ -152,7 +154,7 @@ func (s *Handlers) parseImageRequest(
 		s.respondError(w, fmt.Sprintf("%v, got %q", err, qStr),
 			http.StatusBadRequest)
 
-		return nil, false
+		return nil, "", time.Time{}, false
 	}
 
 	// Only a fit missing from the URL is cover. A fit in the URL that is not a
@@ -162,7 +164,7 @@ func (s *Handlers) parseImageRequest(
 	if query.Has("fit") && req.Transform.FitMode == "" {
 		s.respondError(w, `invalid fit: not a fit mode, got ""`, http.StatusBadRequest)
 
-		return nil, false
+		return nil, "", time.Time{}, false
 	}
 
 	// Default fit mode if not set
@@ -179,10 +181,35 @@ func (s *Handlers) parseImageRequest(
 		s.respondError(w, "invalid image request: "+err.Error(),
 			http.StatusBadRequest)
 
-		return nil, false
+		return nil, "", time.Time{}, false
 	}
 
-	return req, true
+	return req, query.Get("sig"), expires, true
+}
+
+// validateSignature returns nil when the source host of req is on
+// allowlist_hosts and names no port. Otherwise it verifies sig, the URL's
+// signature of req, and expires, the URL's expiry.
+func (s *Handlers) validateSignature(
+	req *imagerequest.Request, sig string, expires time.Time,
+) error {
+	// An allowlist entry names no port, so a host followed by ":", with or
+	// without a port, needs a signature. Go's URL type reads the port as the
+	// fetcher does: in an IPv6 address written without brackets, such as
+	// 2001:db8::1:8443, it is what follows the last ":". This is checked here
+	// and not in allowlist.IsAllowed, which also matches referer_blocklist,
+	// where a Referer must be blocked whatever its port. Port and IsAllowed
+	// read only the host of a URL.
+	sourceURL := &url.URL{Host: req.Source.Host}
+	hostHasPort := sourceURL.Port() != "" ||
+		strings.HasSuffix(sourceURL.Host, ":")
+
+	if !hostHasPort && s.allowlist.IsAllowed(sourceURL) {
+		return nil
+	}
+
+	// Signature required for non-allowed hosts
+	return s.signer.Verify(req, sig, expires)
 }
 
 // parseExpires reads the exp query parameter, a Unix time in seconds. An exp
@@ -341,10 +368,11 @@ func notModified(w http.ResponseWriter, r *http.Request, etag string) bool {
 }
 
 // writeImageResponse writes headers and streams the image content,
-// handling conditional and HEAD requests.
+// handling conditional and HEAD requests. expires is the URL's expiry, which
+// ends the Cache-Control max-age.
 func (s *Handlers) writeImageResponse(
 	w http.ResponseWriter, r *http.Request,
-	req *imagerequest.Request, resp *imgcache.ImageResponse,
+	req *imagerequest.Request, expires time.Time, resp *imgcache.ImageResponse,
 	cacheKey imagerequest.VariantKey, startTime time.Time,
 ) {
 	// Set response headers
@@ -355,7 +383,7 @@ func (s *Handlers) writeImageResponse(
 	}
 
 	// Cache control headers
-	w.Header().Set("Cache-Control", cacheControl(req.Expires))
+	w.Header().Set("Cache-Control", cacheControl(expires))
 	w.Header().Set("X-Pixa-Cache", string(resp.CacheStatus))
 
 	if notModified(w, r, resp.ETag) {