From 998f527bd4d971525137458adbb6e44efa4ad52c Mon Sep 17 00:00:00 2001 From: Daniel Norman Date: Thu, 30 Apr 2026 11:00:01 +0200 Subject: [PATCH] node/db: delete unparseable ipv6 rows - Drop `type='ipv6'` rows whose bracket-stripped inner part contains no `:`, since every valid IPv6 textual form has at least one - Cover the offender (`[]:8776`), bracketed garbage (`[abc]:8776`), the unspecified (`::`), loopback, and full forms in tests - Keep `dns` and `ipv4` rows untouched Migration 8 retyped any `[..]:N` dns row to ipv6 by bracket pattern alone, without checking the inner part is parseable --- CHANGELOG.md | 3 + crates/radicle/src/node/db.rs | 123 ++++++++++++++++++++ crates/radicle/src/node/db/migrations/9.sql | 8 ++ 3 files changed, 134 insertions(+) create mode 100644 crates/radicle/src/node/db/migrations/9.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index cd9ef5b2..9facde4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,9 @@ COB type names and payload IDs remain unchanged for backwards compatibility. peers. Previously, if it found a row it could not parse, then it would fail the whole process. Now, it will continue, providing valid entries to connect to. +- Delete invalid node addresses, of the IPv6 type, from the SQLite address book. + Entries deleted include ones that have an address of the form `[]:port`, or + that have an invalid address within the `[]`. ## 1.8.0 diff --git a/crates/radicle/src/node/db.rs b/crates/radicle/src/node/db.rs index 5cd30f2a..367e50da 100644 --- a/crates/radicle/src/node/db.rs +++ b/crates/radicle/src/node/db.rs @@ -39,6 +39,7 @@ const MIGRATIONS: &[&str] = &[ include_str!("db/migrations/6.sql"), include_str!("db/migrations/7.sql"), include_str!("db/migrations/8.sql"), + include_str!("db/migrations/9.sql"), ]; #[derive(Error, Debug)] @@ -446,4 +447,126 @@ mod test { } } } + + mod migration_9 { + use super::*; + + const NODE1: &str = "node1"; + + fn db_before_migration() -> Database { + let db = Database::memory_up_to_migration(8).unwrap(); + db.execute( + "INSERT INTO nodes (id, features, alias, timestamp) + VALUES ('node1', 0, 'alias', 0)", + ) + .unwrap(); + db + } + + fn run_migration(db: &Database) { + db.execute(MIGRATIONS[8]).unwrap(); + } + + fn address_count(db: &Database, address_type: &str, value: &str) -> i64 { + db.prepare(format!( + "SELECT COUNT(*) FROM addresses + WHERE node = '{NODE1}' AND type = '{address_type}' AND value = '{value}'" + )) + .unwrap() + .into_iter() + .next() + .unwrap() + .unwrap() + .read::(0) + } + + fn insert_address(db: &Database, address_type: &str, value: &str) { + db.execute(format!( + "INSERT INTO addresses (node, type, value, source, timestamp) + VALUES ('{NODE1}', '{address_type}', '{value}', 'peer', 0)" + )) + .unwrap(); + } + + #[test] + fn empty_brackets_ipv6_row_is_deleted() { + let db = db_before_migration(); + let address: &str = "[]:8776"; + insert_address(&db, "ipv6", address); + + run_migration(&db); + + assert_eq!(address_count(&db, "ipv6", address), 0); + } + + #[test] + fn unspecified_address_is_kept() { + // `[::]:8776` parses fine as the IPv6 unspecified address. + let db = db_before_migration(); + let address = "[::]:8776"; + insert_address(&db, "ipv6", address); + + run_migration(&db); + + assert_eq!(address_count(&db, "ipv6", address), 1); + } + + #[test] + fn loopback_address_is_kept() { + let db = db_before_migration(); + let address = "[::1]:8776"; + insert_address(&db, "ipv6", address); + + run_migration(&db); + + assert_eq!(address_count(&db, "ipv6", address), 1); + } + + #[test] + fn full_ipv6_address_is_kept() { + let db = db_before_migration(); + let address = "[2001:db8::1]:8776"; + insert_address(&db, "ipv6", address); + + run_migration(&db); + + assert_eq!(address_count(&db, "ipv6", address), 1); + } + + #[test] + fn bracketed_non_ipv6_garbage_is_deleted() { + // No `:` between the brackets means it can't be IPv6. + let db = db_before_migration(); + let address = "[abc]:8776"; + insert_address(&db, "ipv6", address); + + run_migration(&db); + + assert_eq!(address_count(&db, "ipv6", address), 0); + } + + #[test] + fn ipv4_row_is_unaffected() { + let db = db_before_migration(); + let address = "192.168.1.1:8776"; + insert_address(&db, "ipv4", address); + + run_migration(&db); + + assert_eq!(address_count(&db, "ipv4", address), 1); + } + + #[test] + fn dns_row_is_unaffected_even_when_inner_part_has_no_colon() { + // Migration 9 only targets `type = 'ipv6'`. A bracketless DNS row + // like `example.com:8776` shouldn't be touched. + let db = db_before_migration(); + let address = "example.com:8776"; + insert_address(&db, "dns", address); + + run_migration(&db); + + assert_eq!(address_count(&db, "dns", address), 1); + } + } } diff --git a/crates/radicle/src/node/db/migrations/9.sql b/crates/radicle/src/node/db/migrations/9.sql new file mode 100644 index 00000000..a6904594 --- /dev/null +++ b/crates/radicle/src/node/db/migrations/9.sql @@ -0,0 +1,8 @@ +-- every valid IPv6 textual representation (including `::`) contains +-- at least one `:`. So if the bracket-stripped inner part has no `:`, the value +-- can't be a real IPv6 address. +delete from addresses +where type = 'ipv6' + and instr(value, '[') = 1 + and instr(value, ']:') > 1 + and instr(substr(value, 2, instr(value, ']:') - 2), ':') = 0; \ No newline at end of file