From ee974f67bf3537fd4b0d333c1f63bbe71547be62 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 02:59:07 +0000 Subject: [PATCH] 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()