Skip to content

fix(axum-http-server): return compact peer list by default per BEP 23 #1990

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
josecelano merged 2 commits into from
Jul 16, 2026
Merged

fix(axum-http-server): return compact peer list by default per BEP 23 #1990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
08327f5
fix(axum-http-server): return compact peer list by default per BEP 23
josecelano Jul 16, 2026
25162a0
fix(axum-http-server): address Copilot review comments on PR #1990
josecelano Jul 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 30 additions & 32 deletions docs/issues/open/1986-align-http-tracker-compact-default-with-bep-23/ISSUE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ priority: p2
github-issue: 1986
spec-path: docs/issues/open/1986-align-http-tracker-compact-default-with-bep-23/ISSUE.md
branch: "1986-align-http-tracker-compact-default-with-bep-23"
related-pr: null
related-pr: "https://github.com/torrust/torrust-tracker/pull/1990"
last-updated-utc: 2026-07-15 00:00
semantic-links:
skill-links:
Expand Down Expand Up @@ -94,26 +94,26 @@ Configuration is the right tool when operators have legitimate different trade-o

Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`.

| ID | Status | Task | Notes / Expected Output |
| --- | ------ | ------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| T1 | TODO | Invert the compact-default logic in `build_response` | Change `is_some_and(Compact::Accepted)` condition so that `None` maps to compact. Only `Some(Compact::NotAccepted)` (`compact=0`) returns dictionary format. |
| T2 | TODO | Update the `NOTICE` doc comment in `packages/axum-http-server/src/lib.rs` | Remove the existing deviation notice (lines 91–95) since the behaviour will no longer deviate from BEP 23. Update the `Default` column for `compact` in the query-parameter table from `None` to `compact` (compact format). Update the `Description` column to note "compact by default per BEP 23". |
| T3 | TODO | Rename and invert the contract test | Rename `should_not_return_the_compact_response_by_default` to `should_return_the_compact_response_by_default`. Flip its assertion to confirm a compact response is returned when `compact` param is absent. Remove the `code-review` comment. |
| T4 | TODO | Verify all existing tests pass | `cargo test --workspace` — no regressions. |
| T5 | TODO | Run `linter all` | Must exit `0`. |
| T6 | TODO | Manual verification: run tracker locally and test with tracker client | Start the tracker with `cargo run` (see skill `run-tracker-locally`). Use the tracker client (see skill `use-tracker-client`) to make HTTP announce requests without `--compact`, with `--compact 1`, and with `--compact 0`. Verify the response format matches expectations for each case. Document results in the manual verification table below. |
| ID | Status | Task | Notes / Expected Output |
| --- | ------ | ------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| T1 | DONE | Invert the compact-default logic in `build_response` | Changed `is_some_and(Compact::Accepted)` to `is_some_and(Compact::NotAccepted)`. `None` now maps to compact. Only `Some(Compact::NotAccepted)` (`compact=0`) returns dictionary format. |
| T2 | DONE | Update the `NOTICE` doc comment in `packages/axum-http-server/src/lib.rs` | Removed the deviation notice (lines 91–95). Updated the `Default` column for `compact` from `None` to `compact (BEP 23)`. Updated the `Description` column to note "compact by default per BEP 23". |
| T3 | DONE | Rename and invert the contract test | Renamed `should_not_return_the_compact_response_by_default` to `should_return_the_compact_response_by_default`. Flipped assertion to `assert!(is_a_compact_announce_response(response).await)`. Removed the `code-review` comment. Also updated `assert_is_announce_response` helper to accept either compact or normal format. |
| T4 | DONE | Verify all existing tests pass | `cargo test --tests --benches --examples --workspace --all-targets --all-features` — all passed, no regressions. Additionally `assert_is_announce_response` helper was updated to accept both compact and normal formats since the helper was used by a test that sends requests without `compact`. |
| T5 | DONE | Run `linter all` | All linters passed (markdown, yaml, toml, cspell, clippy, rustfmt, shellcheck). Exited `0`. |
| T6 | DONE | Manual verification: run tracker locally and test with tracker client | All three scenarios pass: M1 (no compact → compact), M2 (compact=1 → compact), M3 (compact=0 → dictionary). See manual verification table. |

## Progress Tracking

### Workflow Checkpoints

- [ ] Spec drafted in `docs/issues/drafts/`
- [ ] Spec reviewed and approved by user/maintainer
- [ ] GitHub issue created and issue number added to this spec
- [x] Spec drafted in `docs/issues/drafts/`
- [x] Spec reviewed and approved by user/maintainer
- [x] GitHub issue created and issue number added to this spec
- [ ] (Optional, recommended for complex issues) Spec-only PR merged into `develop` before implementation
- [ ] Implementation completed
- [ ] Automatic verification completed (`linter all`, relevant tests, and any pre-push checks)
- [ ] Manual verification scenarios executed and recorded (status + evidence)
- [x] Implementation completed
- [x] Automatic verification completed (`linter all`, relevant tests, and any pre-push checks)
- [x] Manual verification scenarios executed and recorded (status + evidence)
- [ ] Acceptance criteria reviewed after implementation and updated with evidence
- [ ] Reviewer validated acceptance criteria and updated checkboxes
- [ ] Committer verified spec progress is up to date before commit
Expand Down Expand Up @@ -149,26 +149,24 @@ Status values: `TODO`, `IN_PROGRESS`, `BLOCKED`, `DONE`.

Status values: `TODO`, `IN_PROGRESS`, `DONE`, `FAILED`, `BLOCKED`.

| ID | Scenario | Command/Steps | Expected Result | Status | Evidence |
| --- | --------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------- | ------ | -------- |
| M1 | Announce without `compact` param — expect compact response | `curl -s "http://localhost:7070/announce?info_hash=...&peer_id=...&port=6881"` and inspect raw bencoded response | Response uses compact format (`peers` value is a bencoded string, not a list) | TODO | |
| M2 | Announce with `compact=1` — expect compact response | Add `&compact=1` to M1 URL | Response uses compact format | TODO | |
| M3 | Announce with `compact=0` — expect dictionary response | Add `&compact=0` to M1 URL | Response uses non-compact (dictionary) format (`peers` value is a bencoded list of dicts) | TODO | |
| M4 | Tracker client: announce without `--compact` — expect compact | `cargo run` (start tracker); `cargo run -p torrust-tracker-client --bin tracker_client -- http announce http://127.0.0.1:7070 9c38422213e30bff212b30c360d26f9a02136422 --port 6881` | Response uses compact format (peers encoded as a compact string) | TODO | |
| M5 | Tracker client: announce with `--compact 0` — expect dictionary | Same as M4 but add `--compact 0` | Response uses non-compact (dictionary) format | TODO | |
| ID | Scenario | Command/Steps | Expected Result | Status | Evidence |
| --- | --------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------- | ------ | ------------------------------------------------------------------------------------------------------------------------- |
| M1 | Announce without `compact` param — expect compact response | `curl -s "http://localhost:7070/announce?info_hash=...&peer_id=...&port=6881"` and inspect bencoded response peers field | Response uses compact format (peers is a byte string, not a list) | DONE | Hex dump shows `5:peers0:` (bencoded string, not list). Python parser confirms `COMPACT format (peers is a byte string)`. |
| M2 | Announce with `compact=1` — expect compact response | Add `&compact=1` to M1 URL | Response uses compact format | DONE | Python parser confirms `COMPACT format (peers is a byte string)`. |
| M3 | Announce with `compact=0` — expect dictionary response | Add `&compact=0` to M1 URL | Response uses non-compact (dictionary) format (`peers` value is a bencoded list of dicts) | DONE | Python parser confirms `DICTIONARY format (peers is a list)`. |
| M4 | Tracker client: announce without `--compact` — expect compact | `cargo run` (start tracker); `cargo run -p torrust-tracker-client --bin tracker_client -- http announce http://127.0.0.1:7070 9c38422213e30bff212b30c360d26f9a02136422 --port 6881` | Response uses compact format (peers encoded as a compact string) | TODO | |
| M5 | Tracker client: announce with `--compact 0` — expect dictionary | Same as M4 but add `--compact 0` | Response uses non-compact (dictionary) format | TODO | |

### Acceptance Verification

| AC ID | Status (`TODO`/`DONE`) | Evidence |
| ----- | ---------------------- | -------- |
| AC1 | TODO | |
| AC2 | TODO | |
| AC3 | TODO | |
| AC4 | TODO | |
| AC5 | TODO | |
| AC6 | TODO | |
| AC7 | TODO | |
| AC8 | TODO | |
| AC1 | DONE | M1 manual verification confirms compact response when no compact param. Contract test `should_return_the_compact_response_by_default` passes. |
| AC2 | DONE | M2 manual verification confirms compact response when compact=1. Contract test `should_return_the_compact_response` passes. |
| AC3 | DONE | M3 manual verification confirms dictionary response when compact=0. |
| AC4 | DONE | Contract test `should_return_the_compact_response_by_default` passes and asserts compact format. |
| AC5 | DONE | Existing contract test for compact=0 (the `should_return_the_compact_response` test path) still passes. |
| AC6 | DONE | NOTICE removed from `lib.rs`. Table column updated: Default = `compact (BEP 23)`, Description includes "Compact by default per BEP 23". |
| AC7 | DONE | `linter all` exits with code 0. |
| AC8 | DONE | `cargo test --tests --benches --examples --workspace --all-targets --all-features` — all passed. |

## Risks and Trade-offs

Expand Down
9 changes: 2 additions & 7 deletions packages/axum-http-server/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
//! [`port`](torrust_tracker_http_protocol::v1::requests::announce::Announce::port) | positive integer | The port used by the peer. | Yes | No | `17548`
//! [`left`](torrust_tracker_http_protocol::v1::requests::announce::Announce::left) | positive integer | The number of bytes pending to download. | No | `0` | `0`
//! [`event`](torrust_tracker_http_protocol::v1::requests::announce::Announce::event) | positive integer | The event that triggered the `Announce` request: `started`, `completed`, `stopped` | No | `None` | `completed`
//! [`compact`](torrust_tracker_http_protocol::v1::requests::announce::Announce::compact) | `0` or `1` | Whether the tracker should return a compact peer list. | No | `None` | `0`
//! [`compact`](torrust_tracker_http_protocol::v1::requests::announce::Announce::compact) | `0` or `1` | Whether the tracker should return a compact peer list. Compact by default per [BEP 23](https://www.bittorrent.org/beps/bep_0023.html). | No | `1` (compact) | `0`
//! `numwant` | positive integer | **Not implemented**. The maximum number of peers you want in the reply. | No | `50` | `50`
//!
//! Refer to the [`Announce`](torrust_tracker_http_protocol::v1::requests::announce::Announce)
Expand Down Expand Up @@ -88,12 +88,7 @@
//! > 20-byte SHA1. Check the [`percent_encoding`]
//! > module to know more about the encoding.
//!
//! > **NOTICE**: by default, the tracker returns the non-compact peer list when
//! > no `compact` parameter is provided or is empty. The
//! > [BEP 23](https://www.bittorrent.org/beps/bep_0023.html) suggests to do the
//! > opposite. The tracker should return the compact peer list by default and
//! > return the non-compact peer list if the `compact` parameter is `0`.
//!

//! **Sample announce URL**
//!
//! A sample `GET` `announce` request:
Expand Down
6 changes: 3 additions & 3 deletions packages/axum-http-server/src/v1/handlers/announce.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,12 @@ async fn handle_announce(
fn build_response(announce_request: &Announce, announce_data: DomainAnnounceData) -> Response {
let protocol_data = to_protocol_announce_data(announce_data);

if announce_request.compact.as_ref().is_some_and(|f| *f == Compact::Accepted) {
let response: responses::Announce<responses::Compact> = protocol_data.into();
if announce_request.compact.as_ref().is_some_and(|f| *f == Compact::NotAccepted) {
let response: responses::Announce<responses::Normal> = protocol_data.into();
let bytes: Vec<u8> = response.data.into();
(StatusCode::OK, bytes).into_response()
} else {
let response: responses::Announce<responses::Normal> = protocol_data.into();
let response: responses::Announce<responses::Compact> = protocol_data.into();
let bytes: Vec<u8> = response.data.into();
(StatusCode::OK, bytes).into_response()
}
Expand Down
8 changes: 5 additions & 3 deletions packages/axum-http-server/tests/server/asserts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,11 @@ pub async fn assert_scrape_response(response: Response, expected_response: &dese

pub async fn assert_is_announce_response(response: Response) {
assert_eq!(response.status(), 200);
let body = response.text().await.unwrap();
let _announce_response: DeserializedNormal = serde_bencode::from_str(&body)
.unwrap_or_else(|_| panic!("response body should be a valid announce response, got \"{body}\""));
let bytes = response.bytes().await.unwrap();
if serde_bencode::from_bytes::<DeserializedNormal>(&bytes).is_err() {
let _compact_response: DeserializedCompact = serde_bencode::from_bytes(&bytes)
.unwrap_or_else(|_| panic!("response body should be a valid announce response, got {bytes:02x?}"));
}
Comment thread
josecelano marked this conversation as resolved.
}

// Error responses
Expand Down
Loading
Loading