From 00ea3441f7747600384844b7cde912195fc199eb Mon Sep 17 00:00:00 2001
From: sneak <sneak@sneak.berlin>
Date: Tue, 20 Oct 2026 01:30:51 +0000
Subject: [PATCH] Keep prefixes that do not parse out of the prefix tables
 (closes #114)

The prefix handler now drops an announcement or withdrawal whose prefix
does not parse, and logs it as an error, so the prefix never reaches the
prefix tables or the prefix count in the stats. Before, no route was
stored for such a prefix, but it was stored as a permanent row in the
prefix tables and counted.

Model: opus-5-5
---
 internal/routewatch/prefixhandler.go          | 38 ++++++++----
 .../routewatch/prefixhandler_internal_test.go | 58 +++++++++++++++++++
 2 files changed, 86 insertions(+), 10 deletions(-)

diff --git a/internal/routewatch/prefixhandler.go b/internal/routewatch/prefixhandler.go
index b5ebb0c..0da2500 100644
--- a/internal/routewatch/prefixhandler.go
+++ b/internal/routewatch/prefixhandler.go
@@ -169,10 +169,19 @@ func (h *PrefixHandler) HandleMessage(msg *ristypes.RISMessage) {
 	// each peer's last update for a prefix: a message that lists a prefix as
 	// both withdrawn and announced keeps the route (RFC 4271 section 4.3).
 	// A withdrawal names only the prefix and peer: the message's AS path, and
-	// so its origin AS, belongs to the announced prefixes.
-	for _, prefix := range msg.Withdrawals {
+	// so its origin AS, belongs to the announced prefixes. An update whose
+	// prefix does not parse is logged and dropped.
+	for _, withdrawn := range msg.Withdrawals {
+		prefix, err := canonicalPrefix(withdrawn)
+		if err != nil {
+			h.logger.Error("Dropped a withdrawal whose prefix does not parse",
+				"prefix", withdrawn, "peer", msg.Peer, "error", err)
+
+			continue
+		}
+
 		h.batch = append(h.batch, prefixUpdate{
-			prefix:      canonicalPrefix(prefix),
+			prefix:      prefix,
 			peer:        msg.Peer,
 			messageType: "withdrawal",
 			timestamp:   timestamp,
@@ -184,9 +193,18 @@ func (h *PrefixHandler) HandleMessage(msg *ristypes.RISMessage) {
 	}
 
 	for _, announcement := range msg.Announcements {
-		for _, prefix := range announcement.Prefixes {
+		for _, announced := range announcement.Prefixes {
+			prefix, err := canonicalPrefix(announced)
+			if err != nil {
+				h.logger.Error(
+					"Dropped an announcement whose prefix does not parse",
+					"prefix", announced, "peer", msg.Peer, "error", err)
+
+				continue
+			}
+
 			h.batch = append(h.batch, prefixUpdate{
-				prefix:      canonicalPrefix(prefix),
+				prefix:      prefix,
 				originASN:   originASN,
 				peer:        msg.Peer,
 				messageType: "announcement",
@@ -560,15 +578,15 @@ func (h *PrefixHandler) writeBatch(
 // the IP lookup builds when it looks a prefix up. The feed sends IPv6
 // announcements compressed ("2001:db8::/32") but IPv6 withdrawals uncompressed
 // ("2001:db8:0:0:0:0:0:0/32"); stored as received, a withdrawal would not match
-// the route its announcement stored. A prefix that does not parse is returned
-// unchanged, and the batch flush reports it.
-func canonicalPrefix(prefix string) string {
+// the route its announcement stored. It returns an error for a prefix that does
+// not parse.
+func canonicalPrefix(prefix string) (string, error) {
 	p, err := netip.ParsePrefix(prefix)
 	if err != nil {
-		return prefix
+		return "", err
 	}
 
-	return p.Masked().String()
+	return p.Masked().String(), nil
 }
 
 // parseCIDR returns the mask length and the IP version of a prefix string
diff --git a/internal/routewatch/prefixhandler_internal_test.go b/internal/routewatch/prefixhandler_internal_test.go
index 81aa4c7..cb17f73 100644
--- a/internal/routewatch/prefixhandler_internal_test.go
+++ b/internal/routewatch/prefixhandler_internal_test.go
@@ -440,6 +440,64 @@ func TestPrefixHandlerRemovesTheRouteOnAnAnnouncementWithoutAnOrigin(
 	}
 }
 
+// TestPrefixHandlerDropsAPrefixThatDoesNotParse runs an announcement of
+// "not-a-prefix" and a withdrawal of "also/bad" through the prefix handler into
+// a real database. Neither is stored in the prefix tables: the prefix count in
+// the stats stays 0, both as kept in memory and as counted from the tables when
+// the database is opened again.
+func TestPrefixHandlerDropsAPrefixThatDoesNotParse(t *testing.T) {
+	t.Parallel()
+
+	cfg := &config.Config{StateDir: t.TempDir()}
+
+	db, err := database.New(cfg, logger.New())
+	if err != nil {
+		t.Fatalf("failed to create database: %v", err)
+	}
+
+	// Built without NewPrefixHandler's flush timer, so the test flushes itself.
+	h := &PrefixHandler{db: db, logger: logger.New()}
+
+	withdrawal := &ristypes.RISMessage{
+		Type:            ristypes.MessageTypeUpdate,
+		Peer:            testPeerIP,
+		ParsedTimestamp: time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC),
+		Withdrawals:     []string{"also/bad"},
+	}
+	h.HandleMessage(peerAnnouncement(testPeerIP, "not-a-prefix"))
+	h.HandleMessage(withdrawal)
+	h.flushBatch()
+
+	checkNoPrefixes := func(when string) {
+		t.Helper()
+
+		stats, err := db.GetStatsContext(context.Background())
+		if err != nil {
+			t.Fatalf("GetStatsContext %s: %v", when, err)
+		}
+
+		if stats.Prefixes != 0 {
+			t.Errorf("prefix count %s = %d, want 0", when, stats.Prefixes)
+		}
+	}
+
+	checkNoPrefixes("after the flush")
+
+	err = db.Close()
+	if err != nil {
+		t.Fatalf("Close: %v", err)
+	}
+
+	db, err = database.New(cfg, logger.New())
+	if err != nil {
+		t.Fatalf("failed to open the database again: %v", err)
+	}
+
+	t.Cleanup(func() { _ = db.Close() })
+
+	checkNoPrefixes("after opening the database again")
+}
+
 // TestStateMessagesGoToThePrefixHandlerNotThePeerHandler checks that STATE
 // messages go to the prefix handler, which acts on a down, and not to the peer
 // handler, which would record a peer whose session never comes up as active.