From 9d9fb52c97555dd548186ca06a2bffe929cf0ee6 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 02:59:07 +0000 Subject: [PATCH 1/5] Test that the image URL's host is one spelling (part of #246) The image URL parser must lower-case the host and refuse one holding "@", which a URL reads as a user name and password before the host; ErrInvalidHost is added for that refusal. A host with a port, or a ":" with no port, must match no allowlist entry, so its request needs a signature. All three tests fail until the fix. Model: opus-5-5 --- internal/imgcache/service_internal_test.go | 51 ++++++++++++++++++++ internal/imgcache/urlparser.go | 1 + internal/imgcache/urlparser_internal_test.go | 43 +++++++++++++++++ 3 files changed, 95 insertions(+) diff --git a/internal/imgcache/service_internal_test.go b/internal/imgcache/service_internal_test.go index 85ad9dd..39c7bf9 100644 --- a/internal/imgcache/service_internal_test.go +++ b/internal/imgcache/service_internal_test.go @@ -250,6 +250,57 @@ func TestService_ValidateRequest_SignatureExactHostMatch(t *testing.T) { } } +// TestService_ValidateRequest_HostWithPortNeedsSignature verifies that a host +// followed by a port, or by a colon with no port, matches no allowlist entry, +// so that a request for it without a signature is refused, while the same +// host with no port is allowlisted. +func TestService_ValidateRequest_HostWithPortNeedsSignature(t *testing.T) { + t.Parallel() + + svc, _ := SetupTestService(t, + WithSigningKey(testSigningKey), + WithAllowlist(testHostCDN, "203.0.113.5", "2001:db8::1"), + ) + + tests := []struct { + host string + allowlisted bool + }{ + {testHostCDN, true}, + {"cdn.example.com:8443", false}, + {"cdn.example.com:443", false}, + {"cdn.example.com:", false}, + {"203.0.113.5", true}, + {"203.0.113.5:8080", false}, + {"[2001:db8::1]", true}, + {"[2001:db8::1]:8443", false}, + } + + for _, tt := range tests { + t.Run(tt.host, func(t *testing.T) { + t.Parallel() + + req := &ImageRequest{ + SourceHost: tt.host, + SourcePath: testPathCat, + Size: Size{Width: 50, Height: 50}, + Format: FormatJPEG, + Quality: 85, + FitMode: FitCover, + } + + err := svc.ValidateRequest(req) + t.Logf("ValidateRequest() for host %q with no signature: %v", + tt.host, err) + + if allowlisted := err == nil; allowlisted != tt.allowlisted { + t.Errorf("host %q served without a signature = %v, want %v", + tt.host, allowlisted, tt.allowlisted) + } + }) + } +} + func TestService_Get_InvalidFile(t *testing.T) { t.Parallel() diff --git a/internal/imgcache/urlparser.go b/internal/imgcache/urlparser.go index 0d5e6c5..d836930 100644 --- a/internal/imgcache/urlparser.go +++ b/internal/imgcache/urlparser.go @@ -15,6 +15,7 @@ import ( var ( ErrInvalidPath = errors.New("invalid image path") ErrMissingHost = errors.New("missing source host") + ErrInvalidHost = errors.New("invalid source host") ErrMissingSize = errors.New("missing size specification") ErrInvalidSize = errors.New("invalid size format") ErrInvalidFormat = errors.New("invalid or unsupported format") diff --git a/internal/imgcache/urlparser_internal_test.go b/internal/imgcache/urlparser_internal_test.go index 9f4e68d..518a04e 100644 --- a/internal/imgcache/urlparser_internal_test.go +++ b/internal/imgcache/urlparser_internal_test.go @@ -234,6 +234,49 @@ func TestParseImagePath(t *testing.T) { } } +// TestParseImagePath_LowerCasesHost verifies that the host is lower-cased, so +// that every spelling of its letter case is the same host to the allowlist, +// the upstream connection limits and the cache, and that the path keeps its +// case. +func TestParseImagePath_LowerCasesHost(t *testing.T) { + t.Parallel() + + got, err := ParseImagePath("CDN.Example.COM/Photos/Cat.jpg/800x600.webp") + if err != nil { + t.Fatalf("ParseImagePath() error = %v", err) + } + + t.Logf("parsed host %q, path %q", got.Host, got.Path) + + if got.Host != testHostCDN { + t.Errorf("Host = %q, want %q", got.Host, testHostCDN) + } + + if got.Path != "/Photos/Cat.jpg" { + t.Errorf("Path = %q, want %q", got.Path, "/Photos/Cat.jpg") + } +} + +// TestParseImagePath_RefusesUserinfo verifies that a host holding "@" is +// refused: a URL reads what comes before the "@" as a user name and password +// and the rest as the host. +func TestParseImagePath_RefusesUserinfo(t *testing.T) { + t.Parallel() + + for _, input := range []string{ + "user@cdn.example.com/photos/cat.jpg/800x600.webp", + "user:password@cdn.example.com/photos/cat.jpg/800x600.webp", + } { + got, err := ParseImagePath(input) + t.Logf("ParseImagePath(%q) = %+v, error = %v", input, got, err) + + if !errors.Is(err, ErrInvalidHost) { + t.Errorf("ParseImagePath(%q) error = %v, want %v", + input, err, ErrInvalidHost) + } + } +} + func TestParsedURL_ToImageRequest(t *testing.T) { t.Parallel() -- 2.54.0 From ad47aab290b0d21a087a146124cb0810e835891e Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 03:26:47 +0000 Subject: [PATCH 2/5] Lower-case the image URL's host and require a signature for a port (closes #246) With cdn.example.com allowlisted, CDN.Example.com and cdn.example.com:8443 passed the allowlist too, and each spelling had its own cache entries and its own connections under upstream_connections_per_host. The image URL parser now lower-cases the host and refuses one holding "@" with ErrInvalidHost (400). ValidateRequest counts a host followed by ":", with or without a port, as not allowlisted, so it needs a signature. That check is not in allowlist.IsAllowed, which also matches referer_blocklist. A URL is now signed over the lower-cased host; README.md says so. Model: opus-5-5 --- README.md | 9 ++++++++- TODO.md | 11 +++++++++++ internal/imgcache/service.go | 10 +++++++++- internal/imgcache/urlparser.go | 10 ++++++++++ 4 files changed, 38 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 5fb6a49..687e737 100644 --- a/README.md +++ b/README.md @@ -418,7 +418,8 @@ HMAC-SHA256(secret, "host:path:query:width:height:format:expiration:quality:fit" Where: -- `host` — source origin hostname (e.g. `cdn.example.com`) +- `host` — source origin hostname in lower case, whatever its case in the URL, + followed by `:` and the port when the URL has one (e.g. `cdn.example.com`) - `path` — source path (e.g. `/photos/cat.jpg`), with each `%` written as `%25` and each `:` as `%3A`, so `/photos/a:b.jpg` is signed as `/photos/a%3Ab.jpg` - `query` — source query string, empty string if none, with `%` and `:` written @@ -473,6 +474,12 @@ entry that is neither a host name (letters, digits, hyphens, underscores and dots, with at most one leading dot) nor an IP address, such as one with a port or a `*.` wildcard, aborts startup. +pixa lower-cases the host of a `/v1/image/` URL before anything else uses it, so +`CDN.Example.com` is matched, fetched, cached and counted against +`upstream_connections_per_host` as `cdn.example.com`. Such a URL whose host has +a port, as in `cdn.example.com:8443`, or a `:` with no port, needs a signature +even when the host is allowlisted. A host holding `@` is refused with 400. + ### Configuration Every setting can be given as an environment variable, in a YAML config file diff --git a/TODO.md b/TODO.md index 7793ef4..3285f97 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,17 @@ P2: security: per-origin rate limiting # Completed Steps +- 2026-10-15 the host of a `/v1/image/` URL is lower-cased, and one with a port + needs a signature (closes #246): with `cdn.example.com` allowlisted, + `CDN.Example.com` and `cdn.example.com:8443` passed the allowlist too, and + each spelling was fetched and cached on its own and had connections of its own + under `upstream_connections_per_host`. The image URL parser now lower-cases + the host and refuses one holding `@`, which a URL reads as a user name and + password, with 400. A host with a port, or with a `:` and no port, needs a + signature even when it is allowlisted. That is checked in the image service, + not in `allowlist.IsAllowed`, which also matches `referer_blocklist`, where a + `Referer` is blocked whatever its port. A URL is signed over the lower-cased + host. - 2026-10-15 a `:` in the source path or query no longer moves where the path ends and the query begins (closes #247): the signed data and the cache key joined host, path and query with `:` and no escaping, so a URL signed for diff --git a/internal/imgcache/service.go b/internal/imgcache/service.go index 52684a9..c7706c5 100644 --- a/internal/imgcache/service.go +++ b/internal/imgcache/service.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "log/slog" + "net" "net/url" "runtime/debug" "time" @@ -218,7 +219,14 @@ func (s *Service) ValidateRequest(req *ImageRequest) error { return fmt.Errorf("invalid source URL: %w", err) } - if s.allowlist.IsAllowed(parsedURL) { + // An allowlist entry names no port, so a host followed by ":", with or + // without a port, needs a signature. This is checked here and not in + // allowlist.IsAllowed, which also matches referer_blocklist, where a + // Referer must be blocked whatever its port. + _, _, splitErr := net.SplitHostPort(parsedURL.Host) + hostHasPort := splitErr == nil + + if !hostHasPort && s.allowlist.IsAllowed(parsedURL) { return nil } diff --git a/internal/imgcache/urlparser.go b/internal/imgcache/urlparser.go index d836930..ea5b6fd 100644 --- a/internal/imgcache/urlparser.go +++ b/internal/imgcache/urlparser.go @@ -144,6 +144,16 @@ func parseImageComponents(remainder string) (*ParsedURL, error) { return nil, ErrMissingHost } + // A URL reads what comes before "@" as a user name and password, and the + // rest as the host + if strings.Contains(host, "@") { + return nil, fmt.Errorf("%w: %q holds @", ErrInvalidHost, host) + } + + // Host names do not depend on letter case, so the allowlist, the upstream + // connection limits and the cache see one spelling of each host + host = strings.ToLower(host) + // Extract query string if present in path if qIndex := strings.Index(path, "?"); qIndex != -1 { query = path[qIndex+1:] -- 2.54.0 From b71f2958db319e8bdd99606607201e2fcabdd6c0 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 04:54:40 +0000 Subject: [PATCH 3/5] Test that an unbracketed IPv6 host with a port needs a signature (part of #246) With 2001:db8::1 allowlisted, Go's URL parser, which the fetcher uses, reads 2001:db8::1:8443 as 2001:db8::1 with port 8443, and 2001:db8::1: as 2001:db8::1 with an empty port. Both must need a signature. The test fails until the port check reads the port as the parsed URL does. Model: opus-5-5 --- internal/imgcache/service_internal_test.go | 40 ++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/internal/imgcache/service_internal_test.go b/internal/imgcache/service_internal_test.go index 39c7bf9..e4bbb30 100644 --- a/internal/imgcache/service_internal_test.go +++ b/internal/imgcache/service_internal_test.go @@ -301,6 +301,46 @@ func TestService_ValidateRequest_HostWithPortNeedsSignature(t *testing.T) { } } +// TestService_ValidateRequest_UnbracketedIPv6PortNeedsSignature verifies that +// an allowlisted IPv6 address written without brackets and followed by a +// port, or by a colon with no port, needs a signature. Go's URL parser, which +// the fetcher uses, reads 2001:db8::1:8443 as the host 2001:db8::1 with the +// port 8443. +func TestService_ValidateRequest_UnbracketedIPv6PortNeedsSignature( + t *testing.T, +) { + t.Parallel() + + svc, _ := SetupTestService(t, + WithSigningKey(testSigningKey), + WithAllowlist("2001:db8::1"), + ) + + for _, host := range []string{"2001:db8::1:8443", "2001:db8::1:"} { + t.Run(host, func(t *testing.T) { + t.Parallel() + + req := &ImageRequest{ + SourceHost: host, + SourcePath: testPathCat, + Size: Size{Width: 50, Height: 50}, + Format: FormatJPEG, + Quality: 85, + FitMode: FitCover, + } + + err := svc.ValidateRequest(req) + t.Logf("ValidateRequest() for host %q with no signature: %v", + host, err) + + if err == nil { + t.Errorf("host %q served without a signature, want refused", + host) + } + }) + } +} + func TestService_Get_InvalidFile(t *testing.T) { t.Parallel() -- 2.54.0 From 589318db9706cf5e5d5f3105f7d8d2a673660290 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 05:28:07 +0000 Subject: [PATCH 4/5] Read the source host's port from the parsed URL (part of #246) net.SplitHostPort fails on an IPv6 address written without brackets, so with 2001:db8::1 allowlisted, 2001:db8::1:8443 and 2001:db8::1: counted as having no port and were served without a signature, though the fetcher connects to port 8443. ValidateRequest now takes a host to have a port when the parsed URL's Port() is set or the host ends in ":", as Go's URL parser and so the fetcher read it. Model: opus-5-5 --- internal/imgcache/service.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/internal/imgcache/service.go b/internal/imgcache/service.go index c7706c5..01f0db3 100644 --- a/internal/imgcache/service.go +++ b/internal/imgcache/service.go @@ -7,9 +7,9 @@ import ( "fmt" "io" "log/slog" - "net" "net/url" "runtime/debug" + "strings" "time" "github.com/dustin/go-humanize" @@ -220,11 +220,13 @@ func (s *Service) ValidateRequest(req *ImageRequest) error { } // An allowlist entry names no port, so a host followed by ":", with or - // without a port, needs a signature. This is checked here and not in - // allowlist.IsAllowed, which also matches referer_blocklist, where a - // Referer must be blocked whatever its port. - _, _, splitErr := net.SplitHostPort(parsedURL.Host) - hostHasPort := splitErr == nil + // without a port, needs a signature. The port is read from the parsed + // URL, as the fetcher reads it: 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. + hostHasPort := parsedURL.Port() != "" || + strings.HasSuffix(parsedURL.Host, ":") if !hostHasPort && s.allowlist.IsAllowed(parsedURL) { return nil -- 2.54.0 From 6453b9a1250e611dc022731d70b015110a4cdac6 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 10:03:47 +0000 Subject: [PATCH 5/5] Say the source host is signed as the URL writes it (part of #246) The Signature Specification read as though the hostname were signed with ":" and the port appended. The code signs the host segment as the URL writes it up to the first "/", lower-cased, so IPv6 brackets and a ":" with or without a port are kept. The README line, the TODO.md entry and the parser comment now say so. Model: opus-5-5 --- README.md | 5 +++-- TODO.md | 4 ++-- internal/imgcache/urlparser.go | 3 ++- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 687e737..609ad9e 100644 --- a/README.md +++ b/README.md @@ -418,8 +418,9 @@ HMAC-SHA256(secret, "host:path:query:width:height:format:expiration:quality:fit" Where: -- `host` — source origin hostname in lower case, whatever its case in the URL, - followed by `:` and the port when the URL has one (e.g. `cdn.example.com`) +- `host` — source host as the URL writes it up to the first `/`, in lower case, + keeping an IPv6 address's brackets and any `:` and port after it (e.g. + `cdn.example.com`, `cdn.example.com:8443`, `[2001:db8::1]:8443`) - `path` — source path (e.g. `/photos/cat.jpg`), with each `%` written as `%25` and each `:` as `%3A`, so `/photos/a:b.jpg` is signed as `/photos/a%3Ab.jpg` - `query` — source query string, empty string if none, with `%` and `:` written diff --git a/TODO.md b/TODO.md index 3285f97..d1ccf51 100644 --- a/TODO.md +++ b/TODO.md @@ -39,8 +39,8 @@ P2: security: per-origin rate limiting password, with 400. A host with a port, or with a `:` and no port, needs a signature even when it is allowlisted. That is checked in the image service, not in `allowlist.IsAllowed`, which also matches `referer_blocklist`, where a - `Referer` is blocked whatever its port. A URL is signed over the lower-cased - host. + `Referer` is blocked whatever its port. A URL is signed over its host as it + writes it, in lower case, with any IPv6 brackets, `:` and port kept. - 2026-10-15 a `:` in the source path or query no longer moves where the path ends and the query begins (closes #247): the signed data and the cache key joined host, path and query with `:` and no escaping, so a URL signed for diff --git a/internal/imgcache/urlparser.go b/internal/imgcache/urlparser.go index ea5b6fd..4247be6 100644 --- a/internal/imgcache/urlparser.go +++ b/internal/imgcache/urlparser.go @@ -151,7 +151,8 @@ func parseImageComponents(remainder string) (*ParsedURL, error) { } // Host names do not depend on letter case, so the allowlist, the upstream - // connection limits and the cache see one spelling of each host + // connection limits, the cache and the signature see one spelling of each + // host host = strings.ToLower(host) // Extract query string if present in path -- 2.54.0