Conversation
…future. * Applied a README fix about library installation on Linux (Thanks https://github.com/diederikdehaas). * Some small version bumps, nothing special.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe release updates version metadata, proxy and tracker-limit configuration, validation, tracker state, persistence, HTTP/WebSocket responses, and UDP authentication and proxy handling. ChangesRelease metadata and documentation
Configuration and validation
Tracker and persistence
Protocol responses
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tracker/impls/torrent_tracker_keys.rs (1)
52-60: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftPreserve persisted absolute key expiries during loading.
The database loaders pass stored expiries into
add_key, but this method addsSystemTime::now()again. A persisted0becomes “expires now,” and any normal absolute expiry is extended by its epoch value. Add a separate absolute-expiry insertion path and use it from all database loaders; otherwise permanent and expired announce-key authorization is incorrect.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tracker/impls/torrent_tracker_keys.rs` around lines 52 - 60, Preserve stored absolute expiry timestamps when loading announce keys. Keep add_key’s timeout-relative behavior for callers that provide durations, add a separate absolute-expiry insertion method with the same entry and statistics handling, and update every database loader to use it for persisted expiries, including zero and already-expired values.
🧹 Nitpick comments (1)
src/tracker/impls/torrent_tracker_peers.rs (1)
50-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
max_peers_per_torrentis enforced per map, not per torrent.
peer_cap()feedsenforce_peer_capfor each of the six maps independently (seeds,seeds_ipv6,peers,peers_ipv6,rtc_seeds,rtc_peers), so the actual worst-case per-torrent residency is6 * max_peers_per_torrent. The internal doc says "per-map", but the config key is user-facing and reads as a torrent-wide bound — worth documenting in the config description so operators size it correctly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tracker/impls/torrent_tracker_peers.rs` around lines 50 - 55, Update the user-facing configuration description for max_peers_per_torrent to state that the limit applies independently to each peer map, not to the torrent as a whole. Document that the six maps used by enforce_peer_cap can result in up to six times the configured value per torrent, while preserving the existing 0-means-unlimited semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/database/impls/query_builder.rs`:
- Around line 13-17: Update quote_identifier to quote PostgreSQL identifiers
instead of returning safe unquoted text: escape any embedded double quotes
according to PostgreSQL identifier rules and wrap the result in double quotes.
Preserve the existing backtick quoting for sqlite3 and mysql.
In `@src/http/http.rs`:
- Around line 863-869: Update configuration loading and validation for
trusted_proxies so enabling it with an empty trusted_proxy_addrs emits a clear
operator warning. Preserve the existing proxy handling behavior, including the
explicit allowlist check in the relevant remote-IP validation flow.
In `@src/security/security.rs`:
- Around line 127-129: Update validate_remote_ip to reject IPv6 unique-local
addresses in fc00::/7 and link-local addresses in fe80::/10 when validating
real_ip header claims, including when trusted_proxies is enabled with an empty
source allowlist. Preserve the existing loopback, unspecified, and IPv4
private-address validation behavior.
In `@src/tracker/impls/torrent_sharding.rs`:
- Around line 118-122: Update the cutoff handling in the peer-expiration logic
to compute BT and RTC cutoffs independently instead of returning when either
checked subtraction fails. Guard each corresponding peer comparison with its own
optional cutoff, and ensure the whole-torrent expired_full condition evaluates
false when the BT cutoff is unavailable.
In `@src/udp/impls/udp_server.rs`:
- Around line 58-60: Replace the ahash-based CONNECTION_ID_SECRET with a
cryptographic keyed PRF/MAC key, and update derive_connection_id to compute the
address-bound token using HMAC-SHA256 or keyed BLAKE3 before truncating to
ConnectionId. Preserve per-process regeneration and the existing connection-ID
input and output behavior.
---
Outside diff comments:
In `@src/tracker/impls/torrent_tracker_keys.rs`:
- Around line 52-60: Preserve stored absolute expiry timestamps when loading
announce keys. Keep add_key’s timeout-relative behavior for callers that provide
durations, add a separate absolute-expiry insertion method with the same entry
and statistics handling, and update every database loader to use it for
persisted expiries, including zero and already-expired values.
---
Nitpick comments:
In `@src/tracker/impls/torrent_tracker_peers.rs`:
- Around line 50-55: Update the user-facing configuration description for
max_peers_per_torrent to state that the limit applies independently to each peer
map, not to the torrent as a whole. Document that the six maps used by
enforce_peer_cap can result in up to six times the configured value per torrent,
while preserving the existing 0-means-unlimited semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 58eb9ae2-5474-4a43-afdb-e4da7d60d225
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (46)
Cargo.tomlREADME.mddocker/Dockerfiledocker/build.batsrc/api/api.rssrc/common/common.rssrc/config/config.rssrc/config/impls/configuration.rssrc/config/structs/api_trackers_config.rssrc/config/structs/http_trackers_config.rssrc/config/structs/tracker_config.rssrc/config/structs/udp_trackers_config.rssrc/database/impls/database_connector.rssrc/database/impls/database_connector_mysql.rssrc/database/impls/database_connector_pgsql.rssrc/database/impls/database_connector_sqlite.rssrc/database/impls/query_builder.rssrc/database/traits/database_backend.rssrc/http/http.rssrc/main.rssrc/security/security.rssrc/security/tests.rssrc/tracker/impls/announce_entry.rssrc/tracker/impls/torrent_counts.rssrc/tracker/impls/torrent_sharding.rssrc/tracker/impls/torrent_tracker.rssrc/tracker/impls/torrent_tracker_handlers.rssrc/tracker/impls/torrent_tracker_keys.rssrc/tracker/impls/torrent_tracker_peers.rssrc/tracker/impls/torrent_tracker_rtctorrent.rssrc/tracker/impls/torrent_tracker_torrents.rssrc/tracker/impls/torrent_tracker_torrents_updates.rssrc/tracker/impls/torrent_tracker_users.rssrc/tracker/impls/torrent_update_data.rssrc/tracker/structs/torrent_counts.rssrc/tracker/types/torrents_updates.rssrc/udp/enums/server_error.rssrc/udp/impls/parse_pool.rssrc/udp/impls/udp_server.rssrc/udp/structs/udp_server.rssrc/udp/udp.rssrc/websocket/websocket.rstests/common/mod.rstests/config_tests.rstests/tracker_tests.rstests/udp_tests.rs
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/database/impls/query_builder.rs (1)
30-35: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse PostgreSQL escape literals or bound parameters.
text_literal()escapes quotes by doubling, but PostgreSQL ordinary string literals can interpret a backslash as escaping a doubled quote and\'escape sequence whenstandard_conforming_stringsis off. For the PostgreSQL branch, emitE'...'and escape backslashes/quots consistently, or preferably bind values sosqlxkeeps the literal out of client-side string concatenation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/database/impls/query_builder.rs` around lines 30 - 35, Update QueryBuilder::text_literal for the PostgreSQL branch to avoid unsafe ordinary string-literal escaping: preferably route values through bound parameters, or emit an E-prefixed literal while escaping backslashes and single quotes consistently. Preserve the existing MySQL and SQLite behavior unless required by the chosen implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/database/impls/query_builder.rs`:
- Around line 30-35: Update QueryBuilder::text_literal for the PostgreSQL branch
to avoid unsafe ordinary string-literal escaping: preferably route values
through bound parameters, or emit an E-prefixed literal while escaping
backslashes and single quotes consistently. Preserve the existing MySQL and
SQLite behavior unless required by the chosen implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dc5e8634-96cf-4066-a748-cdabdf6337d4
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
Cargo.tomlsrc/config/impls/configuration.rssrc/config/structs/tracker_config.rssrc/database/database.rssrc/database/impls/database_connector_mysql.rssrc/database/impls/database_connector_pgsql.rssrc/database/impls/database_connector_sqlite.rssrc/database/impls/query_builder.rssrc/security/security.rssrc/security/tests.rssrc/tracker/impls/torrent_sharding.rssrc/tracker/impls/torrent_tracker_keys.rssrc/udp/impls/udp_server.rstests/tracker_tests.rs
🚧 Files skipped from review as they are similar to previous changes (8)
- src/config/structs/tracker_config.rs
- src/tracker/impls/torrent_sharding.rs
- src/database/impls/database_connector_pgsql.rs
- Cargo.toml
- src/security/tests.rs
- src/database/impls/database_connector_sqlite.rs
- src/config/impls/configuration.rs
- src/udp/impls/udp_server.rs
Summary by CodeRabbit
/{key}/{userkey}/announceand strengthened UDP BEP15 connection-id validation.