Parse the prefix on the prefix pages before looking it up (closes #71)
check / check (push) Waiting to run
check / check (push) Waiting to run
Both prefix pages passed the URL text to the lookup as typed. A prefix that does not parse made the lookup fail with an error that was not "no route found", so the pages answered 500; a valid prefix written in any form other than the one the tables store answered 404 although the route exists. Both handlers now parse the prefix with net/netip, mask it and look up the text form it prints, and answer 400 for a prefix that does not parse. Model: opus-5-5
This commit is contained in:
@@ -7,6 +7,7 @@ import (
|
||||
"errors"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/netip"
|
||||
"runtime"
|
||||
"sort"
|
||||
"strconv"
|
||||
@@ -748,8 +749,15 @@ func (s *Server) handlePrefixDetailJSON() http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
// Combine prefix and length into CIDR notation
|
||||
prefix := prefixParam + "/" + lenParam
|
||||
// Look the prefix up in the text form net/netip prints, the form the
|
||||
// tables store, so that any valid way of writing it finds the route.
|
||||
parsed, err := netip.ParsePrefix(prefixParam + "/" + lenParam)
|
||||
if err != nil {
|
||||
writeJSONError(w, http.StatusBadRequest, err.Error())
|
||||
|
||||
return
|
||||
}
|
||||
prefix := parsed.Masked().String()
|
||||
|
||||
routes, err := s.db.GetPrefixDetailsContext(r.Context(), prefix)
|
||||
if err != nil {
|
||||
@@ -912,8 +920,15 @@ func (s *Server) handlePrefixDetail() http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
// Combine prefix and length into CIDR notation
|
||||
prefix := prefixParam + "/" + lenParam
|
||||
// Look the prefix up in the text form net/netip prints, the form the
|
||||
// tables store, so that any valid way of writing it finds the route.
|
||||
parsed, err := netip.ParsePrefix(prefixParam + "/" + lenParam)
|
||||
if err != nil {
|
||||
http.Error(w, "Invalid prefix", http.StatusBadRequest)
|
||||
|
||||
return
|
||||
}
|
||||
prefix := parsed.Masked().String()
|
||||
|
||||
routes, err := s.db.GetPrefixDetailsContext(r.Context(), prefix)
|
||||
if err != nil {
|
||||
|
||||
@@ -148,6 +148,58 @@ 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) {
|
||||
db, err := database.New(&config.Config{StateDir: t.TempDir()}, logger.New())
|
||||
if err != nil {
|
||||
t.Fatalf("database.New: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = db.Close() })
|
||||
|
||||
ts := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC)
|
||||
if err := db.UpsertLiveRouteBatch([]*database.LiveRoute{
|
||||
{
|
||||
ID: uuid.New(), Prefix: "192.0.2.0/24", MaskLength: 24, IPVersion: 4,
|
||||
OriginASN: 64500, PeerIP: "198.51.100.1", ASPath: []int{64500}, NextHop: "198.51.100.1",
|
||||
LastUpdated: ts,
|
||||
},
|
||||
{
|
||||
ID: uuid.New(), Prefix: "2001:db8::/32", MaskLength: 32, IPVersion: 6,
|
||||
OriginASN: 64501, PeerIP: "2001:db8::1", ASPath: []int{64501}, NextHop: "2001:db8::1",
|
||||
LastUpdated: ts,
|
||||
},
|
||||
}); err != nil {
|
||||
t.Fatalf("UpsertLiveRouteBatch: %v", err)
|
||||
}
|
||||
|
||||
s := New(db, streamer.New(logger.New(), metrics.New()), logger.New(), &config.Config{})
|
||||
|
||||
tests := []struct {
|
||||
path string
|
||||
want int
|
||||
}{
|
||||
{"/api/v1/prefix/foo/24", http.StatusBadRequest},
|
||||
{"/prefix/foo/24", http.StatusBadRequest},
|
||||
{"/api/v1/prefix/2001:DB8:0:0::/32", http.StatusOK},
|
||||
{"/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},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
rec := httptest.NewRecorder()
|
||||
s.router.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, tt.path, nil))
|
||||
if rec.Code != tt.want {
|
||||
t.Errorf("%s: status %d, want %d; body %s", tt.path, rec.Code, tt.want, rec.Body)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// settledGoroutineCount lets transient goroutines finish, then reports the
|
||||
// current count.
|
||||
func settledGoroutineCount() int {
|
||||
|
||||
Reference in New Issue
Block a user