Review batch 3: 7 verified fixes across tracker, torrent, client#7
Merged
Conversation
- tracker/state: GetPeers returned 1 peer when numwant=0 (off-by-one — the limit check ran after the append). Check before appending. Test added. - tracker/registry: 'sort=seeders' applied SQL LIMIT/OFFSET by created_at then re-sorted only that page, so seeder ranking was wrong across pages. Scan the matching set (capped at 1000, logged if exceeded), then sort + paginate in memory. - tracker/registry: sanitize the DB-sourced name before interpolating into the Content-Disposition header (strip quotes/backslash/control chars). - torrent/magnet: validate btih (40 hex) and btmh:1220 (64 hex) info hashes at the parse boundary instead of storing arbitrary text (LangSec). Tests added. - client/downloader: validate PIECE block index+begin before copying, so a stray/duplicate/reordered block can't land at the wrong offset. - client/downloader: generatePeerID now uses crypto/rand instead of time-seeded math/rand (collision risk). - client/storage: WritePiece asserts every piece byte mapped to a file rather than silently returning success on a bad offset mapping. Verified false positives, not changed: swarm.go resultChan 'deadlock' (the drain loop absorbs in-flight sends before close) and the wire.go 2 MiB cap.
iksnerd
added a commit
that referenced
this pull request
Jun 21, 2026
Review batch 3: 7 verified fixes across tracker, torrent, client
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Third batch from the 2026-06-13 review — the medium/low findings I'd deprioritized, each verified by reading the code first (two claimed bugs turned out to be false positives and were left alone).
Changes
GetPeersoff-by-one —internal/tracker/state.goThe limit check ran after the append, so
numwant=0returned 1 peer instead of 0. Check before appending. Test added.sort=seederspagination —internal/tracker/registry.goSeeder counts are memory-only, so the query paginated by
created_atin SQL and then re-sorted only the current page — wrong ranking across pages. Now scans the matching set (capped at 1000, logged when exceeded so the truncation isn't silent), sorts by seeders, and paginates in memory.Content-Disposition header injection —
internal/tracker/registry.goThe DB-sourced torrent name was interpolated raw into the response header. Sanitize quotes/backslash/control chars first.
Magnet hash validation —
internal/torrent/magnet.gobtih/btmhinfo hashes were stored verbatim. Validate v1 as 40 hex chars and v2 as 64 hex at the parse boundary (LangSec). Tests added; existing tests updated to use realistic-length hashes.PIECE block validation —
internal/client/downloader.goThe block's
index/beginwere ignored, so a stray/duplicate/reordered block from a non-conforming peer landed at the wrong offset (caught only later by the SHA-1 check). Validate both before copying.generatePeerID—internal/client/downloader.goSwitched from time-seeded
math/randtocrypto/randso two clients started in the same instant don't collide.WritePieceintegrity check —internal/client/storage.goAsserts every piece byte mapped to a file instead of silently returning success on a bad offset mapping (the previously-unused
dataOffsetnow guards this).Verified false positives (not changed)
swarm.go"resultChan deadlock" (claimed critical) — the drain loop atStartabsorbs any in-flight worker sends beforewg.Wait()closes the channel; no sender can block past close.wire.gopayload cap —2<<20is exactly 2 MiB, matching its comment.Still open (need reference client / architectural change, not in this PR)
v2 Merkle padding (inconclusive — needs a Transmission 4.x golden vector), multi-file hybrid padding files, and the swarm's static peer partitioning (reliability, not correctness).
Verification
go build,go vet,gofmtclean; fullgo test ./...passes including new tests.🤖 Generated with Claude Code