From 0ff30713375ba6dfbeca47e2280a26a792a79c4f Mon Sep 17 00:00:00 2001 From: clawbot Date: Sun, 8 Feb 2026 15:58:32 -0800 Subject: [PATCH] fix: encode source query in GenerateSignedURL to avoid malformed URLs When a source URL has query parameters, GenerateSignedURL() was embedding a bare '?' in the path, causing everything after it to be parsed as the HTTP query string instead of as path segments. This made the size/format segment unreachable by the URL parser. Percent-encode the query string in the path segment so it remains part of the path and can be correctly extracted by ParseImagePath. Fixes #2 --- internal/imgcache/signature.go | 29 ++++++------ internal/imgcache/signature_query_test.go | 55 +++++++++++++++++++++++ 2 files changed, 71 insertions(+), 13 deletions(-) create mode 100644 internal/imgcache/signature_query_test.go diff --git a/internal/imgcache/signature.go b/internal/imgcache/signature.go index 892c496..1795b1f 100644 --- a/internal/imgcache/signature.go +++ b/internal/imgcache/signature.go @@ -6,6 +6,7 @@ import ( "encoding/base64" "errors" "fmt" + "net/url" "strconv" "time" ) @@ -96,22 +97,24 @@ func (s *Signer) GenerateSignedURL(req *ImageRequest, ttl time.Duration) (path s sizeStr = fmt.Sprintf("%dx%d", req.Size.Width, req.Size.Height) } - // Build the path - path = fmt.Sprintf("/v1/image/%s%s/%s.%s", - req.SourceHost, - req.SourcePath, - sizeStr, - req.Format, - ) - - // Append query if present + // 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 ParseImagePath 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. if req.SourceQuery != "" { - // The query goes before the size segment in our URL scheme - // So we need to rebuild the path - path = fmt.Sprintf("/v1/image/%s%s?%s/%s.%s", + 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, - req.SourceQuery, sizeStr, req.Format, ) diff --git a/internal/imgcache/signature_query_test.go b/internal/imgcache/signature_query_test.go new file mode 100644 index 0000000..da05d6f --- /dev/null +++ b/internal/imgcache/signature_query_test.go @@ -0,0 +1,55 @@ +package imgcache + +import ( + "strings" + "testing" + "time" +) + +func TestGenerateSignedURL_WithQueryString(t *testing.T) { + signer := NewSigner("test-secret-key-for-testing!") + + req := &ImageRequest{ + SourceHost: "cdn.example.com", + SourcePath: "/photos/cat.jpg", + SourceQuery: "token=abc&v=2", + Size: Size{Width: 800, Height: 600}, + Format: FormatWebP, + } + + path, _, _ := signer.GenerateSignedURL(req, time.Hour) + + // The path must NOT contain a bare "?" that would be interpreted as a query string delimiter. + // The size segment must appear as the last path component. + if strings.Contains(path, "?token=abc") { + t.Errorf("GenerateSignedURL() produced bare query string in path: %q", path) + } + + // The size segment must be present in the path + if !strings.Contains(path, "/800x600.webp") { + t.Errorf("GenerateSignedURL() missing size segment in path: %q", path) + } + + // Path should end with the size.format, not with query params + if !strings.HasSuffix(path, "/800x600.webp") { + t.Errorf("GenerateSignedURL() path should end with size.format: %q", path) + } +} + +func TestGenerateSignedURL_WithoutQueryString(t *testing.T) { + signer := NewSigner("test-secret-key-for-testing!") + + req := &ImageRequest{ + SourceHost: "cdn.example.com", + SourcePath: "/photos/cat.jpg", + Size: Size{Width: 800, Height: 600}, + Format: FormatWebP, + } + + path, _, _ := signer.GenerateSignedURL(req, time.Hour) + + expected := "/v1/image/cdn.example.com/photos/cat.jpg/800x600.webp" + if path != expected { + t.Errorf("GenerateSignedURL() path = %q, want %q", path, expected) + } +}