From 1aacf6cbf6e86ef2baa447b6adc3e138856914eb Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 16 Oct 2026 02:11:19 +0000 Subject: [PATCH 1/2] Test that both image routes fetch an IPv6 source host without a port A router test against a test upstream that records each request: an IPv6 source host written with and without a port, through a /v1/image/ URL with a signature and through an encrypted URL. A public address must be fetched, a private one refused with 403 without reaching the upstream. The cases without a port fail: each answers 500. Model: opus-5-5 --- internal/server/source_url_internal_test.go | 110 ++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/internal/server/source_url_internal_test.go b/internal/server/source_url_internal_test.go index d006c44..0a5b4a1 100644 --- a/internal/server/source_url_internal_test.go +++ b/internal/server/source_url_internal_test.go @@ -6,12 +6,16 @@ import ( "net/url" "regexp" "slices" + "strconv" "strings" "sync" "testing" + "time" + "sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/imgcache" "sneak.berlin/go/pixa/internal/session" + "sneak.berlin/go/pixa/internal/signature" ) // percentSignPath is a source path holding a "%", which a URL writes %25. @@ -433,3 +437,109 @@ func TestGeneratorRefusesSourceURLItCannotFetchAsWritten(t *testing.T) { }) } } + +// ipv6SourcePath is the source path of the image URLs signedImageURL and +// encryptedImageURL make. +const ipv6SourcePath = "/ipv6.png" + +// TestImageRoutesFetchIPv6SourceHostWithOrWithoutPort requests ipv6SourcePath +// on an IPv6 source host, written with and without a port, through a +// /v1/image/ URL with a signature and through an encrypted URL. The public +// address 2001:db8::a, a documentation address (RFC 3849) the fetcher accepts +// as public, answers 200, and the upstream receives the request for +// ipv6SourcePath. The private address fd00::1 answers 403, and the upstream +// receives no request: the test's fetcher connects every address to the test +// upstream, so only the fetcher's check of the source URL can refuse it. +func TestImageRoutesFetchIPv6SourceHostWithOrWithoutPort(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + sourceHost string + wantStatus int + // wantRequests are the request targets the upstream must receive + wantRequests []string + }{ + {"public address without a port", "[2001:db8::a]", + http.StatusOK, []string{ipv6SourcePath}}, + {"public address with a port", "[2001:db8::a]:8443", + http.StatusOK, []string{ipv6SourcePath}}, + {"private address without a port", "[fd00::1]", + http.StatusForbidden, nil}, + {"private address with a port", "[fd00::1]:8443", + http.StatusForbidden, nil}, + } + + for _, tt := range tests { + imageURLs := map[string]string{ + "image URL with a signature": signedImageURL(t, tt.sourceHost), + "encrypted URL": encryptedImageURL(t, tt.sourceHost), + } + + for route, imageURL := range imageURLs { + t.Run(tt.name+" "+route, func(t *testing.T) { + t.Parallel() + + upstream := startRecordingUpstream(t) + s, _, _ := startImageProxy(t, upstream.Server) + + rec := httptest.NewRecorder() + s.ServeHTTP(rec, httptest.NewRequestWithContext( + t.Context(), http.MethodGet, imageURL, nil)) + t.Logf("GET %s: %d", imageURL, rec.Code) + + if rec.Code != tt.wantStatus { + t.Errorf("status = %d, want %d; body %s", + rec.Code, tt.wantStatus, rec.Body.String()) + } + + got := upstream.requestTargets() + t.Logf("upstream received %q", got) + + if !slices.Equal(got, tt.wantRequests) { + t.Errorf("upstream received %q, want %q", got, tt.wantRequests) + } + }) + } + } +} + +// signedImageURL returns the /v1/image/ URL of ipv6SourcePath on sourceHost at +// its original size as PNG, with a signature made with the test signing key, +// which a host not on the allowlist needs. The URL has no q or fit, so it is +// signed with the quality 85 and the fit cover. +func signedImageURL(t *testing.T, sourceHost string) string { + t.Helper() + + path, sig, exp := signature.New(testSigningKey).GenerateSignedURL( + &signature.Request{ + SourceHost: sourceHost, + SourcePath: ipv6SourcePath, + Format: string(imgcache.FormatPNG), + Quality: 85, + FitMode: string(imgcache.FitCover), + }, time.Hour) + + return path + "?sig=" + sig + "&exp=" + strconv.FormatInt(exp, 10) +} + +// encryptedImageURL returns the encrypted URL of ipv6SourcePath on sourceHost +// at its original size as PNG, made with the test signing key. +func encryptedImageURL(t *testing.T, sourceHost string) string { + t.Helper() + + generator, err := encurl.NewGenerator(testSigningKey) + if err != nil { + t.Fatalf("encurl.NewGenerator: %v", err) + } + + token, err := generator.Generate(&encurl.Payload{ + SourceHost: sourceHost, SourcePath: ipv6SourcePath, + Format: imgcache.FormatPNG, + }) + if err != nil { + t.Fatalf("encurl Generate: %v", err) + } + + return "/v1/e/" + token + "/img.png" +} -- 2.54.0 From 6f50eb20df196489f32a24b550686032c3dabb88 Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 16 Oct 2026 02:13:39 +0000 Subject: [PATCH 2/2] Fetch an IPv6 source host written without a port (closes #295) The fetcher's check of a source URL cut a port off the host with net.SplitHostPort, which fails when an IPv6 address in brackets has no port, so the brackets stayed on, the name lookup failed and the request answered 500. The check now parses the URL and takes the host from Hostname(), which drops the brackets with or without a port, so the address is looked up and a private one is refused with 403. Model: opus-5-5 --- TODO.md | 6 ++++++ internal/httpfetcher/httpfetcher.go | 16 ++++++++-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/TODO.md b/TODO.md index da1345b..9ae228e 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,12 @@ P2: security: per-origin rate limiting # Completed Steps +- 2026-10-16 a source host that is an IPv6 address in brackets with no port, + such as `[2001:db8::a]`, is fetched like one with a port, through `/v1/image/` + and through an encrypted URL (closes #295). The fetcher's check of the source + URL kept the brackets when no port followed them, could not look the host up, + and the request answered 500. It now checks the address without brackets, so a + private one is refused with 403, as with a port. - 2026-10-16 a `state_dir` whose name holds `%`, `?` or `#` keeps its database at `state.sqlite3` in that directory (closes #290): the `db_url` derived from it put the name into a `file:` URL as it was, so SQLite read `%41` as `A`, a diff --git a/internal/httpfetcher/httpfetcher.go b/internal/httpfetcher/httpfetcher.go index 1428040..159ee68 100644 --- a/internal/httpfetcher/httpfetcher.go +++ b/internal/httpfetcher/httpfetcher.go @@ -473,16 +473,16 @@ func validateURL(ctx context.Context, rawURL string, allowHTTP bool) error { return ErrUnsupportedScheme } - // Parse to extract host - host := extractHost(rawURL) - if host == "" { - return ErrInvalidHost + parsedURL, err := neturl.Parse(rawURL) + if err != nil { + return fmt.Errorf("%w: %w", ErrInvalidHost, err) } - // Remove port if present - h, _, err := net.SplitHostPort(host) - if err == nil { - host = h + // Hostname drops any port, and the brackets of an IPv6 address whether + // or not a port follows them + host := parsedURL.Hostname() + if host == "" { + return ErrInvalidHost } // Block obvious localhost patterns -- 2.54.0