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

@@ -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")

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)
+	})
+}

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