From 0ff30713375ba6dfbeca47e2280a26a792a79c4f Mon Sep 17 00:00:00 2001 From: clawbot Date: Sun, 8 Feb 2026 15:58:32 -0800 Subject: [PATCH 1/2] 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) + } +} From 79ceed2ee49071ac8c246cfcf5042653083a5779 Mon Sep 17 00:00:00 2001 From: clawbot Date: Sun, 8 Feb 2026 15:59:51 -0800 Subject: [PATCH 2/2] fix: guard against division by zero when fetchBytes is 0 processAndStore() computed sizePercent as outputSize/fetchBytes*100 without checking for zero, producing Inf/NaN in logs and metrics. Also treat empty cached source data the same as missing (re-fetch from upstream) since zero-byte images can't be processed. Fixes #5 --- internal/imgcache/divzero_test.go | 43 +++++++++++++++++++++++++++++++ internal/imgcache/service.go | 10 ++++--- 2 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 internal/imgcache/divzero_test.go diff --git a/internal/imgcache/divzero_test.go b/internal/imgcache/divzero_test.go new file mode 100644 index 0000000..160d480 --- /dev/null +++ b/internal/imgcache/divzero_test.go @@ -0,0 +1,43 @@ +package imgcache + +import ( + "fmt" + "math" + "testing" +) + +func TestSizePercentSafeWithZeroFetchBytes(t *testing.T) { + // Simulate the calculation from processAndStore + fetchBytes := int64(0) + outputSize := int64(100) + + var sizePercent float64 + if fetchBytes > 0 { + sizePercent = float64(outputSize) / float64(fetchBytes) * 100.0 + } + + // sizePercent should be 0, not Inf or NaN + if math.IsInf(sizePercent, 0) || math.IsNaN(sizePercent) { + t.Errorf("sizePercent = %f, should not be Inf/NaN", sizePercent) + } + + // Should produce valid log output + result := fmt.Sprintf("%.1f%%", sizePercent) + if result != "0.0%" { + t.Errorf("formatted size ratio = %q, want %q", result, "0.0%") + } +} + +func TestSizePercentNormalCase(t *testing.T) { + fetchBytes := int64(1000) + outputSize := int64(500) + + var sizePercent float64 + if fetchBytes > 0 { + sizePercent = float64(outputSize) / float64(fetchBytes) * 100.0 + } + + if math.Abs(sizePercent-50.0) > 0.001 { + t.Errorf("sizePercent = %f, want 50.0", sizePercent) + } +} diff --git a/internal/imgcache/service.go b/internal/imgcache/service.go index d3a5c15..7734f96 100644 --- a/internal/imgcache/service.go +++ b/internal/imgcache/service.go @@ -153,8 +153,8 @@ func (s *Service) processFromSourceOrFetch( } } - // Fetch from upstream if we don't have source data - if sourceData == nil { + // Fetch from upstream if we don't have source data or it's empty + if len(sourceData) == 0 { resp, err := s.fetchAndProcess(ctx, req, cacheKey) if err != nil { return nil, err @@ -264,7 +264,11 @@ func (s *Service) processAndStore( // Log conversion details outputSize := int64(len(processedData)) - sizePercent := float64(outputSize) / float64(fetchBytes) * 100.0 //nolint:mnd // percentage calculation + + var sizePercent float64 + if fetchBytes > 0 { + sizePercent = float64(outputSize) / float64(fetchBytes) * 100.0 //nolint:mnd // percentage calculation + } s.log.Info("image converted", "host", req.SourceHost,