test(bridge): make the fragment sweep test assert the property it names

TestBridgeFragmentSweepIsPerCall claimed its probe used "an EXISTING key, not a
new one: the sweep must still run". It did not: the stale datagram carried IPv4
id 61 and the probe id 62, and fragKey includes the identification, so the probe
opened a NEW key — the one arrangement in which the sweep runs even when it runs
only on new keys. Moving r.sweep(now) inside the `entry == nil` branch left the
test green.

The probe is now the SECOND fragment of a datagram whose first fragment is
already cached, with the two entries opened half a fragTimeout apart so the
stale one is past its deadline and the live one is not (deadlines are set at
creation and never refreshed). Two assertions before the probe pin the setup:
the stale entry must still be there, and the live key must already exist — if a
later edit breaks either, the test says so instead of quietly proving nothing.
The released bytes are checked too, which is the half of the timeout this test
is about (the correctness half is already caught by TestBridgeFragmentTimeout).

Same sweep of TestBridgeFragmentMalformed, which had the same shape of hole: a
FIRST fragment carries MF=1 and can never complete a datagram, so `got != nil`
is unreachable whether the packet was refused or accepted, and "truncated
header" asserted only that. Every subtest now asserts on the cache, and a case
for the classic overread — a header claiming TotalLength 276 in a 28-byte
buffer — is added; its control is the aligned subtest already at the bottom.

Mutations (linux, -race): sweep moved into the new-key branch fails
SweepIsPerCall by name; clamping TotalLength to the buffer instead of refusing
fails the new malformed subtest — and, as predicted, leaves its `got != nil`
assertion silent. Control: moving the sweep after the entry lookup while keeping
it unconditional keeps every test green, so the test discriminates "per call",
not "the line moved". No production code changed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BHw89tdWddzhjUc4bAH4tS

protocol/bridge/frag_reassembly_lx_test.go

@@ -664,30 +664,71 @@ func TestBridgeFragmentTimeout(t *testing.T) {
 	})
 }
 
+// fragTestIPv4Key is the cache key a test datagram built by ipv4Datagram lands
+// under. Spelled once, because getting it wrong is how this file's sweep test
+// came to assert nothing (see TestBridgeFragmentSweepIsPerCall).
+func fragTestIPv4Key(id uint32) fragKey {
+	return fragKey{
+		source:      fragTestSource,
+		destination: fragTestDestination,
+		id:          id,
+		protocol:    uint8(header.UDPProtocolNumber),
+		version:     4,
+	}
+}
+
 // TestBridgeFragmentSweepIsPerCall: the sweep must run on EVERY fragment, not
 // only when a new key is created. Otherwise an entry that expired while another
 // key stayed alive keeps its memory until some unrelated new datagram appears.
+//
+// THE PROBE MUST NOT OPEN A KEY, or the test proves nothing — a fragment that
+// creates an entry sweeps even in the broken arrangement, so the assertion
+// passes either way. fragKey carries the IPv4 identification (see fragKey), so
+// a probe with a different id is a NEW key however firmly the comment beside it
+// says "existing"; this test asserted exactly that for one wave, and the
+// arrangement it names went unmeasured. The probe below is therefore the SECOND
+// fragment of a datagram whose first fragment is already cached, and the
+// assertion immediately before it pins that the entry really is there.
+//
+// The two entries need different deadlines to be told apart at the moment of
+// the probe, and a deadline is set once at creation and never refreshed (see
+// fragTimeout). So the stale entry is opened at t0 and the live one half a
+// timeout later: at t0+fragTimeout the first is past its deadline and the
+// second is not.
 func TestBridgeFragmentSweepIsPerCall(t *testing.T) {
 	clock := &fragTestClock{value: 1}
 	backend := &backendBase{}
 	backend.fragments.nowFunc = clock.now
 
-	stale := ipv4Datagram(61, udpSegment(fragTestBody(600)))
-	staleFragments := ipv4Fragments(stale, 256)
+	staleFragments := ipv4Fragments(ipv4Datagram(61, udpSegment(fragTestBody(600))), 256)
 	backend.returnDatagram(staleFragments[0], 0)
-	staleBytes := backend.fragments.bytes
-	if staleBytes == 0 {
-		t.Fatal("the stale fragment was not cached")
+	if backend.fragments.bytes == 0 {
+		t.Fatal("the stale fragment was not cached, so the sweep below would have nothing to fail to collect")
 	}
 
-	live := ipv4Datagram(62, udpSegment(fragTestBody(600)))
-	liveFragments := ipv4Fragments(live, 256)
-	clock.advance(fragTimeout)
-	// An EXISTING key, not a new one: the sweep must still run.
+	clock.advance(fragTimeout / 2)
+	liveFragments := ipv4Fragments(ipv4Datagram(62, udpSegment(fragTestBody(600))), 256)
+	if len(liveFragments) < 2 {
+		t.Fatalf("test setup produced %d live fragments, want at least 2 — the probe has to be a fragment that does NOT open a key", len(liveFragments))
+	}
 	backend.returnDatagram(liveFragments[0], 0)
-	if _, held := backend.fragments.entries[fragKey{source: fragTestSource, destination: fragTestDestination, id: 61, protocol: uint8(header.UDPProtocolNumber), version: 4}]; held {
+	if _, held := backend.fragments.entries[fragTestIPv4Key(61)]; !held {
+		t.Fatal("the stale entry was collected half a timeout early — then the assertion below could not tell a per-call sweep from a per-key one")
+	}
+
+	// Past the stale entry's deadline, still inside the live one's.
+	clock.advance(fragTimeout / 2)
+	if _, held := backend.fragments.entries[fragTestIPv4Key(62)]; !held {
+		t.Fatal("the live entry is gone, so the probe below would CREATE a key — and a key-creating fragment sweeps even when the sweep runs only on new keys")
+	}
+	staleSize := backend.fragments.bytes
+	backend.returnDatagram(liveFragments[1], 0)
+	if _, held := backend.fragments.entries[fragTestIPv4Key(61)]; held {
 		t.Fatal("the expired entry survived a fragment that did not create a new key — the sweep only runs on new keys, so a busy flow pins dead memory")
 	}
+	if backend.fragments.bytes >= staleSize {
+		t.Fatalf("the cache still accounts for %d bytes (was %d before the sweep) — the expired entry was unlinked without returning its bytes to the budget", backend.fragments.bytes, staleSize)
+	}
 }
 
 // TestBridgeFragmentEntryEviction: the (fragMaxEntries+1)-th datagram evicts
@@ -845,6 +886,13 @@ func TestBridgeFragmentTooManyHoles(t *testing.T) {
 
 // TestBridgeFragmentMalformed: closed validation — a fragment no conforming
 // fragmenter emits is refused rather than reassembled.
+//
+// EVERY SUBTEST HERE ASSERTS ON THE CACHE, not only on the returned datagram.
+// "Nothing came back" is not evidence about a FIRST fragment: one carries MF=1
+// and can never complete a datagram, so `got != nil` is unreachable whether the
+// packet was refused or accepted. What separates the two is whether an entry
+// was allocated — and the control subtest at the bottom is what shows that
+// number is capable of being 1.
 func TestBridgeFragmentMalformed(t *testing.T) {
 	datagram := ipv4Datagram(111, udpSegment(fragTestBody(600)))
 
@@ -865,6 +913,23 @@ func TestBridgeFragmentMalformed(t *testing.T) {
 		if got := backend.returnDatagram(bad, 0); got != nil {
 			t.Fatal("a truncated fragment was accepted")
 		}
+		if len(backend.fragments.entries) != 0 {
+			t.Fatal("a packet shorter than an IPv4 header allocated an entry")
+		}
+	})
+
+	// The overread case: the header is whole and says TotalLength 276, but only
+	// 28 bytes arrived. Parsing this by trusting the header slices past the end
+	// of the buffer, so it must be refused on the length, not on the header.
+	t.Run("shorter than its declared TotalLength", func(t *testing.T) {
+		backend := &backendBase{}
+		bad := ipv4Fragment(datagram, 0, 256, true)[:header.IPv4MinimumSize+8]
+		if got := backend.returnDatagram(bad, 0); got != nil {
+			t.Fatal("a fragment shorter than its own TotalLength was accepted")
+		}
+		if len(backend.fragments.entries) != 0 {
+			t.Fatal("a fragment shorter than its own TotalLength allocated an entry — whatever it cached was read past the end of the caller's buffer")
+		}
 	})
 
 	t.Run("control: the same shape, aligned, is accepted", func(t *testing.T) {