Keep prefixes that do not parse out of the prefix tables (closes #114)
check / check (push) Canceled after 0s
check / check (push) Canceled after 0s
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
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user