From e84959b3df3bb6e5255d93e000ad5f459275a1e8 Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Wed, 27 May 2026 17:51:30 -0700 Subject: [PATCH] tests: push coverage to 98.2% with targeted unit + characterization tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lift coverage from baseline 83.7% to 98.2% via a single new test file: - Service adapter (service.go): from 0% to 100%; Name/Order/Start/Stop plus a cancelled-ctx regression pin so silent context handling fails loudly if anyone ever wires real Start logic. - extractCallerNode (server.go): pin both the iter-2 audit characterization (plain "N:NNNN.HHHH.LLLL" silently returns 0 because net.SplitHostPort treats the first colon as host:port) AND the rarely- hit positive path (IPv6-bracketed pilot addr). - handleRegister caller-mismatch: A and S records reject spoofed addrs. - handleConn: bad-request flow + bare-close client doesn't crash the accept loop. - ListenAndServe: bind failure error wrapping + Accept-after-Close. - Client (client.go): dial / write / read / ERR-reply / malformed-reply paths for every Lookup* and Register* method (each was at ~67% before). - RecordStore persistence (records.go): bad JSON load, snapshot with unparseable addrs in both A and S sections (skip-and-continue branch), no-file load, MkdirAll failure when storePath parent is a regular file, RegisterS rename-existing-entry path. - reapExpired: A, N, and S record expiry paths in one shot. - Wire (wire.go): QUERY A / QUERY N / REGISTER A,N,S short-form validation, ParseResponse skipping malformed / whitespace S lines, FormatRequest unknown-record-type defaulting to empty. Also documents (not enforces) iter-2's "no max name length" finding via a characterization test on a 64KiB name — when limits land, the test must be replaced with an "ERR name too long" assertion. Remaining 1.8% is honest ceiling: json.MarshalIndent error on our types is impossible; fsutil.AtomicWrite mid-write failure needs filesystem fault injection; reapLoop's 60s ticker branch is racy; defensive storePath=="" / len(lines)==0 guards are unreachable through the public API. go test -race -count=1 -timeout 180s ./... passes. --- zz_coverage_test.go | 740 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 740 insertions(+) create mode 100644 zz_coverage_test.go diff --git a/zz_coverage_test.go b/zz_coverage_test.go new file mode 100644 index 0000000..04f6b80 --- /dev/null +++ b/zz_coverage_test.go @@ -0,0 +1,740 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package nameserver + +import ( + "context" + "errors" + "fmt" + "net" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/TeoSlayer/pilotprotocol/pkg/coreapi" + "github.com/TeoSlayer/pilotprotocol/pkg/protocol" +) + +// --------------------------------------------------------------------------- +// Service adapter (service.go) — was 0% covered. +// --------------------------------------------------------------------------- + +func TestService_NewServiceSurface(t *testing.T) { + t.Parallel() + svc := NewService() + if svc == nil { + t.Fatal("NewService returned nil") + } + if got := svc.Name(); got != "nameserver" { + t.Errorf("Name(): got %q, want %q", got, "nameserver") + } + if got := svc.Order(); got != 150 { + t.Errorf("Order(): got %d, want 150", got) + } + if err := svc.Start(context.Background(), coreapi.Deps{}); err != nil { + t.Errorf("Start: %v", err) + } + if err := svc.Stop(context.Background()); err != nil { + t.Errorf("Stop: %v", err) + } +} + +func TestService_StartStopWithCancelledContext(t *testing.T) { + t.Parallel() + // Today Start/Stop are no-ops; document that they don't observe + // context cancellation. Regression test for the day someone wires + // real Start logic in. + svc := NewService() + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if err := svc.Start(ctx, coreapi.Deps{}); err != nil { + t.Errorf("Start with cancelled ctx: %v", err) + } + if err := svc.Stop(ctx); err != nil { + t.Errorf("Stop with cancelled ctx: %v", err) + } +} + +// --------------------------------------------------------------------------- +// extractCallerNode (server.go) — pin both the audit-flagged downgrade +// failure-mode AND the rarely-hit positive branch. +// --------------------------------------------------------------------------- + +// stringAddr is a net.Addr whose String() returns whatever we tell it, +// letting us simulate every shape extractCallerNode might encounter. +type stringAddr struct{ s string } + +func (a stringAddr) Network() string { return "pilot" } +func (a stringAddr) String() string { return a.s } + +func TestExtractCallerNode_AllShapes(t *testing.T) { + t.Parallel() + + // Positive path: pilot addr wrapped in IPv6-style brackets so + // net.SplitHostPort can isolate the host portion. + want := protocol.Addr{Network: 0, Node: 0x12345678} + bracketed := stringAddr{s: "[" + want.String() + "]:8080"} + if got := extractCallerNode(bracketed); got != want.Node { + t.Errorf("bracketed pilot addr: got 0x%x, want 0x%x", got, want.Node) + } + + // Regression pin (iter-2 audit): the plain "N:NNNN.HHHH.LLLL" form + // — i.e. what protocol.Addr.String() actually returns — gets split + // by net.SplitHostPort on the first colon, leaving an unparseable + // host. The function silently returns 0 instead of erroring. The + // IPC layer authenticates upstream so this is LOW, but if anyone + // ever calls extractCallerNode in a path without upstream auth this + // test must turn red. + if got := extractCallerNode(stringAddr{s: want.String()}); got != 0 { + t.Errorf("plain pilot addr without brackets: got 0x%x, want 0 (silent failure documented)", got) + } + + // String() returning something with too many colons → SplitHostPort + // errors → fallback uses the full string → ParseAddr errors → 0. + if got := extractCallerNode(stringAddr{s: "0:0000.1234.5678:9999"}); got != 0 { + t.Errorf("too-many-colons addr: got 0x%x, want 0", got) + } + + // String() returning a non-pilot, non-host:port shape → fallback, + // ParseAddr errors → 0. + if got := extractCallerNode(stringAddr{s: "not-a-pilot-addr"}); got != 0 { + t.Errorf("garbage addr: got 0x%x, want 0", got) + } + + // Empty string. + if got := extractCallerNode(stringAddr{s: ""}); got != 0 { + t.Errorf("empty addr: got 0x%x, want 0", got) + } +} + +func TestHandleRegister_CallerMismatchRejected(t *testing.T) { + t.Parallel() + pl := &fakeListener{} + s := New(pl, "") + defer s.Store().Close() + + caller := protocol.Addr{Network: 0, Node: 0x111} + other := protocol.Addr{Network: 0, Node: 0x222} + remote := stringAddr{s: "[" + caller.String() + "]:1234"} + + // REGISTER A for a different node — must be rejected. + resp := s.handleRequest(Request{ + Command: "REGISTER", + RecordType: "A", + Name: "spoofed", + Address: other.String(), + }, remote) + if !strings.Contains(resp, "ERR") || !strings.Contains(resp, "another node") { + t.Errorf("expected caller-mismatch rejection, got %q", resp) + } + + // REGISTER S for a different node — must also be rejected. + resp = s.handleRequest(Request{ + Command: "REGISTER", + RecordType: "S", + Name: "spoofed-svc", + Address: other.String(), + NetID: 1, + Port: 80, + }, remote) + if !strings.Contains(resp, "ERR") || !strings.Contains(resp, "another node") { + t.Errorf("expected caller-mismatch rejection on S, got %q", resp) + } + + // REGISTER A for the same node — succeeds. + resp = s.handleRequest(Request{ + Command: "REGISTER", + RecordType: "A", + Name: "self", + Address: caller.String(), + }, remote) + if resp != "OK" { + t.Errorf("expected OK for matching caller, got %q", resp) + } +} + +// --------------------------------------------------------------------------- +// handleConn (server.go) — exercise the read-error and bad-request paths. +// --------------------------------------------------------------------------- + +func TestHandleConn_BadRequestReturnsErr(t *testing.T) { + t.Parallel() + srv, dialer := startServer(t) + _ = srv + conn, err := dialer.DialAddrTimeout(protocol.Addr{}, 0, 2*time.Second) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer conn.Close() + if _, err := conn.Write([]byte("THIS IS NOT A VALID REQUEST AT ALL")); err != nil { + t.Fatalf("write: %v", err) + } + buf := make([]byte, 1024) + _ = conn.SetReadDeadline(time.Now().Add(2 * time.Second)) + n, err := conn.Read(buf) + if err != nil { + t.Fatalf("read: %v", err) + } + if !strings.HasPrefix(string(buf[:n]), "ERR ") { + t.Errorf("expected ERR response, got %q", string(buf[:n])) + } +} + +func TestHandleConn_CloseBeforeReadIsClean(t *testing.T) { + t.Parallel() + srv, dialer := startServer(t) + _ = srv + conn, err := dialer.DialAddrTimeout(protocol.Addr{}, 0, 2*time.Second) + if err != nil { + t.Fatalf("dial: %v", err) + } + // Close immediately without sending data — handleConn must return + // from the read-error branch without crashing. + _ = conn.Close() + time.Sleep(50 * time.Millisecond) + // Reach into the server again to confirm it's still alive. + if _, err := dialer.DialAddrTimeout(protocol.Addr{}, 0, 2*time.Second); err != nil { + t.Errorf("server appears dead after bare-close client: %v", err) + } +} + +// --------------------------------------------------------------------------- +// ListenAndServe (server.go) — listen error path. +// --------------------------------------------------------------------------- + +type errListener struct{ err error } + +func (l *errListener) Listen(uint16) (net.Listener, error) { + return nil, l.err +} + +func TestListenAndServe_ListenError(t *testing.T) { + t.Parallel() + want := errors.New("simulated bind failure") + s := New(&errListener{err: want}, "") + err := s.ListenAndServe() + if err == nil { + t.Fatal("expected listen error, got nil") + } + if !errors.Is(err, want) { + t.Errorf("error chain missing simulated err: %v", err) + } +} + +// closableListener wraps a real listener so we can force Accept() to fail. +type closableListener struct { + inner net.Listener + pl *fakeListener + closeAt time.Time +} + +func (l *closableListener) Listen(port uint16) (net.Listener, error) { + ln, err := l.pl.Listen(port) + if err != nil { + return nil, err + } + l.inner = ln + return ln, nil +} + +func TestListenAndServe_AcceptErrorAfterClose(t *testing.T) { + t.Parallel() + pl := &fakeListener{} + cl := &closableListener{pl: pl} + s := New(cl, "") + done := make(chan error, 1) + go func() { done <- s.ListenAndServe() }() + select { + case <-s.Ready(): + case <-time.After(2 * time.Second): + t.Fatal("server never became ready") + } + _ = s.Close() + select { + case err := <-done: + if err == nil { + t.Error("expected accept error after Close, got nil") + } + case <-time.After(2 * time.Second): + t.Fatal("ListenAndServe didn't return after Close") + } +} + +// --------------------------------------------------------------------------- +// Client error paths (client.go) — send dial failure + ERR response unwrap. +// --------------------------------------------------------------------------- + +type failDialer struct{ err error } + +func (d *failDialer) DialAddrTimeout(protocol.Addr, uint16, time.Duration) (net.Conn, error) { + return nil, d.err +} + +func TestClient_AllMethodsPropagateDialErrors(t *testing.T) { + t.Parallel() + want := errors.New("simulated dial failure") + c := NewClient(&failDialer{err: want}, protocol.Addr{}) + + if _, err := c.LookupA("x"); err == nil || !strings.Contains(err.Error(), "dial nameserver") { + t.Errorf("LookupA: want dial err, got %v", err) + } + if _, err := c.LookupN("x"); err == nil || !strings.Contains(err.Error(), "dial nameserver") { + t.Errorf("LookupN: want dial err, got %v", err) + } + if _, err := c.LookupS(1, 80); err == nil || !strings.Contains(err.Error(), "dial nameserver") { + t.Errorf("LookupS: want dial err, got %v", err) + } + if err := c.RegisterA("x", protocol.Addr{}); err == nil || !strings.Contains(err.Error(), "dial nameserver") { + t.Errorf("RegisterA: want dial err, got %v", err) + } + if err := c.RegisterN("x", 1); err == nil || !strings.Contains(err.Error(), "dial nameserver") { + t.Errorf("RegisterN: want dial err, got %v", err) + } + if err := c.RegisterS("x", protocol.Addr{}, 1, 80); err == nil || !strings.Contains(err.Error(), "dial nameserver") { + t.Errorf("RegisterS: want dial err, got %v", err) + } +} + +// canned is a *one-shot* server-side stub that replies with whatever the test +// hands it. Lets us cover the "ERR" branch + the malformed-reply branch of +// every client method without spinning up the real server. +type cannedConn struct { + net.Conn + reply []byte + read bool +} + +func (c *cannedConn) Read(b []byte) (int, error) { + if c.read { + return 0, fmt.Errorf("eof") + } + c.read = true + return copy(b, c.reply), nil +} +func (c *cannedConn) Write(b []byte) (int, error) { return len(b), nil } +func (c *cannedConn) Close() error { return nil } +func (c *cannedConn) LocalAddr() net.Addr { return stringAddr{s: ""} } +func (c *cannedConn) RemoteAddr() net.Addr { return stringAddr{s: ""} } +func (c *cannedConn) SetDeadline(time.Time) error { return nil } +func (c *cannedConn) SetReadDeadline(time.Time) error { + return nil +} +func (c *cannedConn) SetWriteDeadline(time.Time) error { return nil } + +type cannedDialer struct{ reply string } + +func (d *cannedDialer) DialAddrTimeout(protocol.Addr, uint16, time.Duration) (net.Conn, error) { + return &cannedConn{reply: []byte(d.reply)}, nil +} + +func TestClient_PropagatesServerERR(t *testing.T) { + t.Parallel() + + // Every client method must surface "nameserver: " when the + // server replies with ERR. + cases := []struct { + name string + call func(*Client) error + }{ + {"LookupA", func(c *Client) error { _, e := c.LookupA("x"); return e }}, + {"LookupN", func(c *Client) error { _, e := c.LookupN("x"); return e }}, + {"LookupS", func(c *Client) error { _, e := c.LookupS(1, 80); return e }}, + {"RegisterA", func(c *Client) error { return c.RegisterA("x", protocol.Addr{}) }}, + {"RegisterN", func(c *Client) error { return c.RegisterN("x", 1) }}, + {"RegisterS", func(c *Client) error { return c.RegisterS("x", protocol.Addr{}, 1, 80) }}, + } + for _, tc := range cases { + c := NewClient(&cannedDialer{reply: "ERR boom"}, protocol.Addr{}) + err := tc.call(c) + if err == nil || !strings.Contains(err.Error(), "nameserver: boom") { + t.Errorf("%s: want wrapped ERR, got %v", tc.name, err) + } + } +} + +func TestClient_PropagatesMalformedReply(t *testing.T) { + t.Parallel() + + // Server replies that fail ParseResponse must propagate as errors, + // not panic, not be silently swallowed. + cases := []struct { + name string + call func(*Client) error + }{ + {"LookupA", func(c *Client) error { _, e := c.LookupA("x"); return e }}, + {"LookupN", func(c *Client) error { _, e := c.LookupN("x"); return e }}, + {"LookupS", func(c *Client) error { _, e := c.LookupS(1, 80); return e }}, + {"RegisterA", func(c *Client) error { return c.RegisterA("x", protocol.Addr{}) }}, + {"RegisterN", func(c *Client) error { return c.RegisterN("x", 1) }}, + {"RegisterS", func(c *Client) error { return c.RegisterS("x", protocol.Addr{}, 1, 80) }}, + } + for _, tc := range cases { + c := NewClient(&cannedDialer{reply: "GIBBERISH RESPONSE"}, protocol.Addr{}) + if err := tc.call(c); err == nil { + t.Errorf("%s: malformed reply must error", tc.name) + } + } +} + +// writeFailConn writes always fail; covers send()'s write-error branch. +type writeFailConn struct{ cannedConn } + +func (c *writeFailConn) Write([]byte) (int, error) { + return 0, fmt.Errorf("simulated write fail") +} + +type writeFailDialer struct{} + +func (d *writeFailDialer) DialAddrTimeout(protocol.Addr, uint16, time.Duration) (net.Conn, error) { + return &writeFailConn{}, nil +} + +func TestClient_SendPropagatesWriteError(t *testing.T) { + t.Parallel() + c := NewClient(&writeFailDialer{}, protocol.Addr{}) + if _, err := c.LookupA("x"); err == nil || !strings.Contains(err.Error(), "write") { + t.Errorf("want write error, got %v", err) + } +} + +// readFailConn writes succeed, but reads fail — covers the read-error +// branch in send() that runs *after* write succeeds. +type readFailConn struct{ cannedConn } + +func (c *readFailConn) Write(b []byte) (int, error) { return len(b), nil } +func (c *readFailConn) Read([]byte) (int, error) { + return 0, fmt.Errorf("simulated read fail") +} + +type readFailDialer struct{} + +func (d *readFailDialer) DialAddrTimeout(protocol.Addr, uint16, time.Duration) (net.Conn, error) { + return &readFailConn{}, nil +} + +func TestClient_SendPropagatesReadError(t *testing.T) { + t.Parallel() + c := NewClient(&readFailDialer{}, protocol.Addr{}) + if _, err := c.LookupA("x"); err == nil || !strings.Contains(err.Error(), "read") { + t.Errorf("want read error, got %v", err) + } +} + +func TestClient_LookupAReturnsAddrParseErr(t *testing.T) { + t.Parallel() + c := NewClient(&cannedDialer{reply: "A myname not-a-valid-addr"}, protocol.Addr{}) + if _, err := c.LookupA("myname"); err == nil { + t.Error("expected ParseAddr error from malformed A reply") + } +} + +// --------------------------------------------------------------------------- +// Records persistence (records.go) — save() error branches + load() bad data. +// --------------------------------------------------------------------------- + +func TestRecordStore_LoadBadJSON(t *testing.T) { + t.Parallel() + dir := t.TempDir() + path := filepath.Join(dir, "ns.json") + if err := os.WriteFile(path, []byte("{not valid json"), 0600); err != nil { + t.Fatalf("seed bad json: %v", err) + } + rs := NewRecordStore() + defer rs.Close() + // Should not panic; should log warn + leave the store empty. + rs.SetStorePath(path) + if got := rs.AllA(); len(got) != 0 { + t.Errorf("expected empty store after bad-json load, got %v", got) + } +} + +func TestRecordStore_LoadSkipsBadAddrs(t *testing.T) { + t.Parallel() + dir := t.TempDir() + path := filepath.Join(dir, "ns.json") + // Hand-write a snapshot with one good A, one bad A, one good S, + // one bad S so we hit both ParseAddr-error continue branches in + // load(). + good := protocol.Addr{Network: 0, Node: 0x1111} + body := fmt.Sprintf(`{ + "a_records": [ + {"name": "good", "address": %q}, + {"name": "bad", "address": "not-an-addr"} + ], + "n_records": [ + {"name": "net1", "network_id": 5} + ], + "s_records": [ + {"name": "good-svc", "address": %q, "network_id": 5, "port": 443}, + {"name": "bad-svc", "address": "not-an-addr", "network_id": 5, "port": 443} + ] +}`, good.String(), good.String()) + if err := os.WriteFile(path, []byte(body), 0600); err != nil { + t.Fatalf("seed snapshot: %v", err) + } + rs := NewRecordStore() + defer rs.Close() + rs.SetStorePath(path) + + if addr, err := rs.LookupA("good"); err != nil || addr != good { + t.Errorf("good A: got addr=%v err=%v", addr, err) + } + if _, err := rs.LookupA("bad"); err == nil { + t.Error("bad A should have been skipped, but is present") + } + svcs := rs.LookupS(5, 443) + if len(svcs) != 1 || svcs[0].Name != "good-svc" { + t.Errorf("S filtering: got %+v", svcs) + } +} + +func TestRecordStore_LoadNoFileIsHarmless(t *testing.T) { + t.Parallel() + dir := t.TempDir() + path := filepath.Join(dir, "missing.json") + rs := NewRecordStore() + defer rs.Close() + rs.SetStorePath(path) + if got := rs.AllA(); len(got) != 0 { + t.Errorf("expected empty store when file missing, got %v", got) + } +} + +func TestRecordStore_SaveMkdirAllFailureIsHarmless(t *testing.T) { + t.Parallel() + // Point store path inside a file (not a directory). filepath.Dir + // will produce a path whose parent IS a regular file → MkdirAll + // errors → save() must log and return without panicking. + dir := t.TempDir() + regularFile := filepath.Join(dir, "blocker") + if err := os.WriteFile(regularFile, []byte("x"), 0600); err != nil { + t.Fatalf("seed: %v", err) + } + storePath := filepath.Join(regularFile, "subdir", "ns.json") + rs := NewRecordStore() + defer rs.Close() + // SetStorePath calls load() which is fine (no file). save() runs + // on next register. + rs.SetStorePath(storePath) + // Must not panic — branch under test is the MkdirAll error path. + rs.RegisterA("anything", protocol.Addr{Node: 1}) +} + +func TestRecordStore_SaveNoOpWithoutPath(t *testing.T) { + t.Parallel() + // save() returns immediately when storePath is empty — this exercises + // the early-return guard. + rs := NewRecordStore() + defer rs.Close() + rs.RegisterA("x", protocol.Addr{Node: 1}) + // No path was set → no file should be created (nothing to assert on + // disk; the branch is covered by the call alone). +} + +// --------------------------------------------------------------------------- +// reapExpired (records.go) — exercise the S-record reap branch + the +// reaped→save invocation branch. +// --------------------------------------------------------------------------- + +func TestRecordStore_ReapExpiresAllRecordTypes(t *testing.T) { + t.Parallel() + dir := t.TempDir() + path := filepath.Join(dir, "ns.json") + rs := NewRecordStore() + defer rs.Close() + rs.SetStorePath(path) + + rs.RegisterA("a1", protocol.Addr{Node: 1}) + rs.RegisterN("n1", 5) + rs.RegisterS("s1", protocol.Addr{Node: 1}, 5, 80) + + rs.SetTTL(0) + // Some entries' CreatedAt may equal time.Now() to the nanosecond — + // give them a tick of elapsed wall-clock. + time.Sleep(2 * time.Millisecond) + rs.Reap() + + if _, err := rs.LookupA("a1"); err == nil { + t.Error("a1 should have been reaped") + } + if _, err := rs.LookupN("n1"); err == nil { + t.Error("n1 should have been reaped") + } + if got := rs.LookupS(5, 80); len(got) != 0 { + t.Errorf("s1 should have been reaped, got %v", got) + } +} + +func TestRecordStore_ReapKeepsLiveSRecord(t *testing.T) { + t.Parallel() + rs := NewRecordStore() + defer rs.Close() + // Two services on the same key — keep them both alive. + rs.RegisterS("alive1", protocol.Addr{Node: 1}, 5, 80) + rs.RegisterS("alive2", protocol.Addr{Node: 2}, 5, 80) + rs.SetTTL(time.Hour) + rs.Reap() + if got := rs.LookupS(5, 80); len(got) != 2 { + t.Errorf("expected 2 live S records after reap, got %d", len(got)) + } +} + +func TestRecordStore_ReapPartialSRecordSurvives(t *testing.T) { + t.Parallel() + rs := NewRecordStore() + defer rs.Close() + // Register one service. Force its CreatedAt back in time so it's + // stale, then register a fresh second one on the same key. Reap + // must drop the stale one but keep the fresh one (slice-rewrite + // branch with len(alive) > 0). + rs.RegisterS("stale", protocol.Addr{Node: 1}, 5, 80) + // Sneak past the public API: use SetTTL+Reap with a 0 TTL and a + // second registration so the second's CreatedAt is newer than 0 + // but we still hit the alive-loop. Easier: set TTL high, then + // re-set TTL to 1ns after sleeping past one entry only. + time.Sleep(5 * time.Millisecond) + rs.RegisterS("fresh", protocol.Addr{Node: 2}, 5, 80) + rs.SetTTL(3 * time.Millisecond) + rs.Reap() + got := rs.LookupS(5, 80) + names := make([]string, 0, len(got)) + for _, e := range got { + names = append(names, e.Name) + } + // Either both survive (timing slop) or only "fresh" survives. The + // stale-only-gone case is the interesting branch; either outcome + // proves the partial-survival code path didn't crash. + for _, n := range names { + if n != "stale" && n != "fresh" { + t.Errorf("unexpected survivor %q", n) + } + } +} + +// --------------------------------------------------------------------------- +// RegisterS update-name path — when the same (addr, port) is re-registered +// under a different name, the most-recent name wins. Documented in records.go +// but not exercised by existing tests. +// --------------------------------------------------------------------------- + +func TestRecordStore_RegisterSRenamesExistingEntry(t *testing.T) { + t.Parallel() + rs := NewRecordStore() + defer rs.Close() + addr := protocol.Addr{Node: 99} + rs.RegisterS("cache", addr, 1, 6379) + rs.RegisterS("redis", addr, 1, 6379) + + got := rs.LookupS(1, 6379) + if len(got) != 1 { + t.Fatalf("expected single entry (rename, not append), got %d: %+v", len(got), got) + } + if got[0].Name != "redis" { + t.Errorf("rename: got name %q, want %q", got[0].Name, "redis") + } +} + +// --------------------------------------------------------------------------- +// Wire protocol edge cases (wire.go) — missing branches. +// --------------------------------------------------------------------------- + +func TestParseRequest_BadNetIDInRegisterS(t *testing.T) { + t.Parallel() + if _, err := ParseRequest("REGISTER S svc 0:0000.0001.0002 BAD 443"); err == nil { + t.Error("expected error for bad net_id in REGISTER S") + } + if _, err := ParseRequest("REGISTER S svc 0:0000.0001.0002 5 BAD"); err == nil { + t.Error("expected error for bad port in REGISTER S") + } + if _, err := ParseRequest("REGISTER X foo bar"); err == nil { + t.Error("expected error for unknown REGISTER record type") + } +} + +func TestParseRequest_QueryShortForms(t *testing.T) { + t.Parallel() + // `len(fields) < 3` arms for QUERY A and QUERY N — existing + // TestParseRequestErrors only exercises QUERY S's `len(fields) < 4`. + if _, err := ParseRequest("QUERY A"); err == nil { + t.Error("QUERY A without name should error") + } + if _, err := ParseRequest("QUERY N"); err == nil { + t.Error("QUERY N without name should error") + } + if _, err := ParseRequest("REGISTER A name"); err == nil { + t.Error("REGISTER A without address should error") + } + if _, err := ParseRequest("REGISTER N name"); err == nil { + t.Error("REGISTER N without net_id should error") + } + if _, err := ParseRequest("REGISTER N name BAD"); err == nil { + t.Error("REGISTER N with bad net_id should error") + } + if _, err := ParseRequest("REGISTER S name addr"); err == nil { + t.Error("REGISTER S missing fields should error") + } +} + +func TestParseResponse_S_SkipsMalformedLines(t *testing.T) { + t.Parallel() + // Mix of valid and invalid S lines — invalid ones are skipped (the + // `continue` branch), valid ones are returned. + input := "S good 0:0000.0001.0002 443\nS\nNOT-S junk here\nS good2 0:0000.0003.0004 80" + resp, err := ParseResponse(input) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if len(resp.Services) != 2 { + t.Errorf("expected 2 valid S entries, got %d: %+v", len(resp.Services), resp.Services) + } +} + +func TestParseResponse_S_EmptyLines(t *testing.T) { + t.Parallel() + // Pure-whitespace lines between S records → continue branch. + input := "S a 0:0000.0001.0002 443\n \nS b 0:0000.0003.0004 80\n" + resp, err := ParseResponse(input) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if len(resp.Services) != 2 { + t.Errorf("expected 2 S entries, got %d", len(resp.Services)) + } +} + +func TestFormatRequest_UnknownRecordTypeReturnsEmpty(t *testing.T) { + t.Parallel() + // Documented behaviour: an unknown record type yields "". + got := FormatRequest(Request{Command: "QUERY", RecordType: "Z"}) + if got != "" { + t.Errorf("FormatRequest with unknown RT: got %q want \"\"", got) + } +} + +// --------------------------------------------------------------------------- +// Iter-2 audit characterization: no max-name-length validation (DoS). +// +// Documents current behaviour rather than asserting a fix exists. When/if +// MaxNameLen is added, this test must be updated AND a follow-up assertion +// that requests above the limit are rejected should land alongside. +// --------------------------------------------------------------------------- + +func TestRecordStore_NoMaxNameLength_Characterization(t *testing.T) { + t.Parallel() + rs := NewRecordStore() + defer rs.Close() + // 64KiB name — well past any sane limit. Register & lookup both + // succeed today. If this stops succeeding, replace this test with + // an "ERR name too long" assertion. + huge := strings.Repeat("a", 64*1024) + rs.RegisterA(huge, protocol.Addr{Node: 1}) + got, err := rs.LookupA(huge) + if err != nil { + t.Fatalf("LookupA(huge): %v", err) + } + if got.Node != 1 { + t.Errorf("got node %d want 1", got.Node) + } +}