From f0353d2d8bc83d80d537b7ec3af76bd8ec1fb812 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Mon, 19 Oct 2026 20:43:33 +0000 Subject: [PATCH] Percent-decode URL parameters before parsing them (closes #103) chi matches routes against the escaped path when a request has one and hands the handlers their URL parameters still percent-encoded. The home page's lookup form encodes the address it asks for, so every IPv6 lookup answered 400, as did a comma-separated address list or a prefix that a client percent-encoded. The router now matches against the decoded path, so every handler reads its parameters decoded. Model: opus-5-5 --- internal/server/handlers_internal_test.go | 86 ++++++++++++++++++++--- internal/server/middleware.go | 15 ++++ internal/server/routes.go | 1 + 3 files changed, 93 insertions(+), 9 deletions(-) diff --git a/internal/server/handlers_internal_test.go b/internal/server/handlers_internal_test.go index 8ea16a3..21d9d19 100644 --- a/internal/server/handlers_internal_test.go +++ b/internal/server/handlers_internal_test.go @@ -183,14 +183,10 @@ func TestStatsHandlersAnswerFromMemory(t *testing.T) { } } -// TestPrefixDetailHandlersParsePrefix checks that both prefix pages find a -// stored route whatever valid text form the URL writes its prefix in, and -// answer 400 for a prefix that does not parse. The tables store the form -// net/netip prints. Before the fix the URL text was looked up as typed, so -// "2001:DB8:0:0::/32" was not found although "2001:db8::/32" was stored, and -// "foo/24" answered 500 (https://git.eeqj.de/sneak/routewatch/issues/71). -func TestPrefixDetailHandlersParsePrefix(t *testing.T) { - t.Parallel() +// newServerWithTestRoutes returns a server on a new database that holds a +// route for testPrefixV4 and one for 2001:db8::/32. +func newServerWithTestRoutes(t *testing.T) *Server { + t.Helper() db, err := database.New(&config.Config{StateDir: t.TempDir()}, logger.New()) if err != nil { @@ -217,7 +213,22 @@ func TestPrefixDetailHandlersParsePrefix(t *testing.T) { t.Fatalf("UpsertLiveRouteBatch: %v", err) } - s := New(db, streamer.New(logger.New(), metrics.New()), logger.New(), &config.Config{}) + return New(db, streamer.New(logger.New(), metrics.New()), logger.New(), + &config.Config{}) +} + +// TestPrefixDetailHandlersParsePrefix checks that both prefix pages find a +// stored route whatever valid text form the URL writes its prefix in, and +// answer 400 for a prefix that does not parse. The tables store the form +// net/netip prints. Before the fix the URL text was looked up as typed, so +// "2001:DB8:0:0::/32" was not found although "2001:db8::/32" was stored, and +// "foo/24" answered 500 (https://git.eeqj.de/sneak/routewatch/issues/71). A +// prefix with its colons percent-encoded, as JavaScript's encodeURIComponent +// writes it, answered 400 (https://git.eeqj.de/sneak/routewatch/issues/103). +func TestPrefixDetailHandlersParsePrefix(t *testing.T) { + t.Parallel() + + s := newServerWithTestRoutes(t) tests := []struct { path string @@ -229,6 +240,8 @@ func TestPrefixDetailHandlersParsePrefix(t *testing.T) { {"/prefix/2001:DB8:0:0::/32", http.StatusOK}, {"/api/v1/prefix/192.0.2.1/24", http.StatusOK}, {"/prefix/192.0.2.1/24", http.StatusOK}, + {"/api/v1/prefix/2001%3Adb8%3A%3A/32", http.StatusOK}, + {"/prefix/2001%3Adb8%3A%3A/32", http.StatusOK}, } for _, tt := range tests { @@ -243,6 +256,61 @@ func TestPrefixDetailHandlersParsePrefix(t *testing.T) { } } +// TestIPLookupsReadPercentEncodedAddresses checks that the IP lookups find the +// addresses a URL asks for when it percent-encodes them, as the home page's +// lookup form does with encodeURIComponent. Before the fix the handler read +// "2001%3Adb8%3A%3A1" undecoded as a host name, which did not resolve, and +// answered 400, so the home page could look up no IPv6 address +// (https://git.eeqj.de/sneak/routewatch/issues/103). +func TestIPLookupsReadPercentEncodedAddresses(t *testing.T) { + t.Parallel() + + s := newServerWithTestRoutes(t) + + tests := []struct { + path string + wantIPs string // the addresses of the results, comma-separated + }{ + {"/ip/2001%3Adb8%3A%3A1", "2001:db8::1"}, + {"/api/v1/ip/192.0.2.1%2C192.0.2.2", "192.0.2.1,192.0.2.2"}, + } + + for _, tt := range tests { + rec := httptest.NewRecorder() + s.router.ServeHTTP(rec, httptest.NewRequestWithContext( + t.Context(), http.MethodGet, tt.path, nil)) + + if rec.Code != http.StatusOK { + t.Errorf("%s: status %d, want %d; body %s", + tt.path, rec.Code, http.StatusOK, rec.Body) + + continue + } + + var body struct { + Data IPLookupResponse `json:"data"` + } + + err := json.Unmarshal(rec.Body.Bytes(), &body) + if err != nil { + t.Errorf("%s: decoding the answer %q: %v", tt.path, rec.Body, err) + + continue + } + + gotIPs := make([]string, 0, len(body.Data.Results)) + + for _, result := range body.Data.Results { + gotIPs = append(gotIPs, result.IP) + } + + got := strings.Join(gotIPs, ",") + if got != tt.wantIPs { + t.Errorf("%s: results for %q, want %q", tt.path, got, tt.wantIPs) + } + } +} + // errASDetailsLookupFailed is what failingASDetailsDB answers every AS details // lookup with. var errASDetailsLookupFailed = errors.New("AS details lookup failed") diff --git a/internal/server/middleware.go b/internal/server/middleware.go index d2a185d..f088733 100644 --- a/internal/server/middleware.go +++ b/internal/server/middleware.go @@ -10,6 +10,8 @@ import ( "net/http" "sync" "time" + + "github.com/go-chi/chi/v5" ) // responseWriter wraps http.ResponseWriter to capture the response @@ -281,3 +283,16 @@ func RequestLoggerMiddleware(logger *slog.Logger) func(http.Handler) http.Handle }) } } + +// DecodedPathMiddleware makes the router match routes against the decoded +// path, so that the handlers read their URL parameters decoded. Without it, +// chi matches against the escaped path whenever the request has one and hands +// the handlers their parameters still escaped: /ip/2001%3Adb8%3A%3A1 would +// look up "2001%3Adb8%3A%3A1". +func DecodedPathMiddleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + chi.RouteContext(r.Context()).RoutePath = r.URL.Path + + next.ServeHTTP(w, r) + }) +} diff --git a/internal/server/routes.go b/internal/server/routes.go index aca1005..f1fcbc8 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -12,6 +12,7 @@ func (s *Server) setupRoutes() { r := chi.NewRouter() // Middleware + r.Use(DecodedPathMiddleware) r.Use(middleware.RequestID) r.Use(middleware.RealIP) r.Use(RequestLoggerMiddleware(s.logger.Logger)) // Structured request logging