Document how a source query goes into an image URL, and accept it (closes #255)

The query follows the source path after %3F. The image route now parses
the request's path decoded once instead of chi's wildcard, which holds
the path as the client wrote it whenever that is not Go's own encoding
of it, so a / ; or , written as %2F, %3B or %2C, as GenerateSignedURL
wrote them, hid the %3F and the request failed with 401. Now the ? and
every % in the source path or query must be percent-encoded and any
other character may be. GenerateSignedURL writes Go's own encoding of
the path. The README states the rule, with an example and its signature.

Model: opus-5-5

internal/handlers/image.go

@@ -9,7 +9,6 @@ import (
 	"strconv"
 	"time"
 
-	"github.com/go-chi/chi/v5"
 	"github.com/go-chi/chi/v5/middleware"
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
@@ -80,20 +79,21 @@ func (s *Handlers) HandleRobotsTxt() http.HandlerFunc {
 	}
 }
 
-// parseImageRequest parses the wildcard path and query parameters into
+// parseImageRequest parses the image URL's path and query parameters into
 // an ImageRequest. On invalid input it writes an error response and
 // returns false.
 func (s *Handlers) parseImageRequest(
 	w http.ResponseWriter, r *http.Request,
 ) (*imgcache.ImageRequest, bool) {
-	// Get the wildcard path from chi
-	pathParam := chi.URLParam(r, "*")
-
-	// Parse the URL path
-	parsed, err := imgcache.ParseImagePath(pathParam)
+	// 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
+	// the %3F before a source query when, say, a "/" in the query is written
+	// as %2F
+	parsed, err := imgcache.ParseImageURL(r.URL.Path)
 	if err != nil {
 		s.log.Warn("failed to parse image URL",
-			"path", pathParam,
+			"path", r.URL.Path,
 			"error", err,
 		)
 		s.respondError(w, "invalid image URL: "+err.Error(), http.StatusBadRequest)

internal/handlers/image_signature_internal_test.go

@@ -198,7 +198,7 @@ func TestHandleImage_GeneratedSignedURLWithSourceQueryVerifies(t *testing.T) {
 
 			signedURL, err := fix.service.GenerateSignedURL("", &imgcache.ImageRequest{
 				SourceHost:  signedHost,
-				SourcePath:  "/images/photo.jpg",
+				SourcePath:  photoPath,
 				SourceQuery: tt.query,
 				Size:        imgcache.Size{Width: 50, Height: 50},
 				Format:      imgcache.FormatJPEG,
@@ -226,7 +226,7 @@ func TestHandleImage_SourceQueryEncodedAnyWay(t *testing.T) {
 	signer := signature.New("test-signing-key-must-be-32-chars")
 	sig := signer.Sign(&signature.Request{
 		SourceHost:  signedHost,
-		SourcePath:  "/images/photo.jpg",
+		SourcePath:  photoPath,
 		SourceQuery: "url=https://origin.example.com/my%20cat.jpg",
 		Width:       50,
 		Height:      50,
@@ -237,23 +237,23 @@ func TestHandleImage_SourceQueryEncodedAnyWay(t *testing.T) {
 	})
 
 	tests := []struct {
-		name          string
-		writtenSource string
+		name         string
+		writtenQuery string
 	}{
 		{"only ? and % encoded",
-			"/images/photo.jpg%3Furl=https://origin.example.com/my%2520cat.jpg"},
+			"%3Furl=https://origin.example.com/my%2520cat.jpg"},
 		{"every reserved character encoded",
-			"/images/photo.jpg%3Furl%3Dhttps%3A%2F%2Forigin.example.com%2Fmy%2520cat.jpg"},
+			"%3Furl%3Dhttps%3A%2F%2Forigin.example.com%2Fmy%2520cat.jpg"},
 		{"lower-case hex digits",
-			"/images/photo.jpg%3furl=https://origin.example.com/my%2520cat.jpg"},
+			"%3furl=https://origin.example.com/my%2520cat.jpg"},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			target := fmt.Sprintf("/v1/image/%s%s/50x50.jpeg?sig=%s&exp=%d",
-				signedHost, tt.writtenSource, sig, expires.Unix())
+			target := fmt.Sprintf("/v1/image/%s%s%s/50x50.jpeg?sig=%s&exp=%d",
+				signedHost, photoPath, tt.writtenQuery, sig, expires.Unix())
 
 			status := getImage(t, setupTestHandler(t), target)
 			if status != http.StatusBadGateway {

internal/imgcache/urlparser.go

@@ -61,9 +61,8 @@ type ParsedURL struct {
 	Format ImageFormat
 }
 
-// ParseImagePath parses the path captured by chi's wildcard:
-// <host>/<path>/<size>.<format>, or <host>/<path>/<size> for JPEG XL
-// This is the primary entry point when using chi routing.
+// ParseImagePath parses an image URL's path after /v1/image/, decoded once:
+// <host>/<path>/<size>.<format>, or <host>/<path>/<size> for JPEG XL.
 // Examples:
 //   - cdn.example.com/photos/cat.jpg/800x600.webp
 //   - cdn.example.com/photos/cat.jpg/0x0.jpeg
@@ -79,9 +78,10 @@ func ParseImagePath(path string) (*ParsedURL, error) {
 	return parseImageComponents(path)
 }
 
-// ParseImageURL parses a full URL path like /v1/image/<host>/<path>/<size>.<format>,
-// or /v1/image/<host>/<path>/<size> for JPEG XL
-// Use ParseImagePath instead when working with chi's wildcard capture.
+// ParseImageURL parses an image URL's path, decoded once as r.URL.Path holds
+// it: /v1/image/<host>/<path>/<size>.<format>, or /v1/image/<host>/<path>/<size>
+// for JPEG XL. A source query follows the source path after a "?", which the
+// URL writes as %3F.
 func ParseImageURL(urlPath string) (*ParsedURL, error) {
 	// Remove the /v1/image/ prefix
 	const prefix = "/v1/image/"

internal/signature/signature.go

@@ -126,31 +126,20 @@ func (s *Signer) GenerateSignedURL(
 		sizeStr = fmt.Sprintf("%dx%d", req.Width, req.Height)
 	}
 
-	// Build the path.
-	// When a source query is present, it is embedded as a path segment
-	// (e.g. /host/path?query/size.fmt) so that the URL parser can extract
-	// it from the last-slash split. The "?" inside a path segment is
-	// percent-encoded by clients but chi delivers it decoded, which is
-	// exactly what the URL parser expects.
-	var path string
+	// Build the path. A source query follows the source path after a "?".
+	// The image route reads the path decoded once, so the whole path is
+	// written in Go's own percent-encoding of a URL path, which writes "?"
+	// as %3F and "%" as %25, and leaves "/", ";", ",", "=" and "&" as they
+	// are.
+	sourcePathAndQuery := req.SourcePath
 	if req.SourceQuery != "" {
-		path = fmt.Sprintf("/v1/image/%s%s%%3F%s/%s.%s",
-			req.SourceHost,
-			req.SourcePath,
-			url.PathEscape(req.SourceQuery),
-			sizeStr,
-			req.Format,
-		)
-	} else {
-		path = fmt.Sprintf("/v1/image/%s%s/%s.%s",
-			req.SourceHost,
-			req.SourcePath,
-			sizeStr,
-			req.Format,
-		)
+		sourcePathAndQuery += "?" + req.SourceQuery
 	}
 
-	return path, sig, exp
+	unescapedPath := fmt.Sprintf("/v1/image/%s%s/%s.%s",
+		req.SourceHost, sourcePathAndQuery, sizeStr, req.Format)
+
+	return (&url.URL{Path: unescapedPath}).EscapedPath(), sig, exp
 }
 
 // buildSignatureData creates the string to be signed.

internal/signature/signature_test.go

@@ -476,9 +476,10 @@ func TestGenerateSignedURL_WithoutQueryString(t *testing.T) {
 }
 
 // TestGenerateSignedURL_ReadmeQueryExample checks the README's example of a
-// source URL with a query, https://cdn.example.com/img?url=https://origin.example.com/my%20cat.jpg:
-// its signature with the README's key, and the path GenerateSignedURL writes
-// for it, with the "?" written as %3F and the "%" as %25.
+// source URL with a query, the image at
+// https://cdn.example.com/img?url=https://origin.example.com/my%20cat.jpg: its
+// signature with the README's key, and the path GenerateSignedURL writes for
+// it, with the "?" written as %3F and the "%" as %25.
 func TestGenerateSignedURL_ReadmeQueryExample(t *testing.T) {
 	t.Parallel()