Skip to content
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

Jump to bottom

Reimplement the current HTTP tracker using Axum #160

Closed
17 tasks done
Tracked by #116
, #158
...
josecelano opened this issue Jan 16, 2023 · 2 comments
Closed
17 tasks done
Tracked by #116
, #158
...

Reimplement the current HTTP tracker using Axum #160

josecelano opened this issue Jan 16, 2023 · 2 comments
Labels
Code Cleanup / Refactoring Tidying and Making Neat EPIC Contains several subissues

Comments

@josecelano
Copy link
Member

josecelano commented Jan 16, 2023 •
edited

Parent issue: #158
It depends on: #159

NOTE: We already changed the HTTP API to Axum.

Subtasks

Tracker in public mode:

Tracker in private mode:

Tracker in listed (whitelisted) mode:

Final subtasks:

Discarded subtask:

  • Catch all unhandled errors in handlers and convert them into bencoded generic HTTP tracker response errors. See this comment.
@josecelano josecelano added the Code Cleanup / Refactoring Tidying and Making Neat label Jan 16, 2023
@josecelano josecelano mentioned this issue Jan 16, 2023
Closed
2 tasks
@josecelano josecelano mentioned this issue Feb 2, 2023
Closed
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Feb 6, 2023
@josecelano
feat(http): [torrust#160] scaffolding for HTTP tracker using Axum
7375223 
We are going to migrate the HTTP tracker from Warp to Axum.
This is the basic scaffolding for Axum. Tests have been duplicated to
test the new Axum implementation. The setup allows executing both
versions: the Warp version on production and both versions (Warp and
Axum) on testing env.
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Feb 6, 2023
@josecelano
feat(http): [torrust#160] scaffolding for HTTP tracker using Axum
849633d 
We are going to migrate the HTTP tracker from Warp to Axum.
This is the basic scaffolding for Axum. Tests have been duplicated to
test the new Axum implementation. The setup allows executing both
versions: the Warp version on production and both versions (Warp and
Axum) on testing env.
@josecelano josecelano linked a pull request Feb 6, 2023 that will close this issue
Axum HTTP tracker: discarded PR #179
Merged
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Feb 7, 2023
@josecelano
refactor(http): [torrust#160] extract functions for percent decoding
6b57dfd
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Feb 7, 2023
@josecelano
refactor(http): [torrust#160] extract functions for percent decoding
13a4982
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Feb 8, 2023
@josecelano
feat(http): [torrust#160] scaffolding for HTTP tracker using Axum
0dc3050 
We are going to migrate the HTTP tracker from Warp to Axum.
This is the basic scaffolding for Axum. Tests have been duplicated to
test the new Axum implementation. The setup allows executing both
versions: the Warp version on production and both versions (Warp and
Axum) on testing env.
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Feb 8, 2023
@josecelano
refactor(http): [torrust#160] extract functions for percent decoding
b82fb75
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Feb 8, 2023
@josecelano
refactor(http): [torrust#160] extract functions for percent decoding
cffd071
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Feb 9, 2023
@josecelano
refactor(http): [torrust#160] extract functions for percent decoding
7dc4838
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Feb 9, 2023
@josecelano
feat(http): [torrust#160] Axum extractor to extract announce request …
2f62e8c 
…params from url query

WIP: only for mandatory params.
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Feb 9, 2023
@josecelano
feat(http): [torrust#160] Axum extractor to extract announce request …
9c25feb 
…params from url query

WIP: only for mandatory params.
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Feb 9, 2023
@josecelano
refactor(http): [torrust#160] reorganize dirs for Axum and Warp HTTP …
995397e 
…tracker implementations

We are going to start sharing code bettween both implementation (Warp
and Axum). We need to keep common code separated because Warp
implementation will be removed when Axum implementation is finished.
josecelano added a commit that referenced this issue Feb 9, 2023
@josecelano
Merge #180: Basic scaffolding for Axum HTTP tracker implementation
8e85f18 
995397e refactor(http): [#160] reorganize dirs for Axum and Warp HTTP tracker implementations (Jose Celano)
9c25feb feat(http): [#160] Axum extractor to extract announce request params from url query (Jose Celano)
7dc4838 refactor(http): [#160] extract functions for percent decoding (Jose Celano)
0dc3050 feat(http): [#160] scaffolding for HTTP tracker using Axum (Jose Celano)

Pull request description:

  This is the first of a series of PR associated with [this](#179) [parent](#179) [PR](#179).

  We are changing the HTTP tracker to use Axum instead of Warp. This PR allows:

  - Running full Warp implementation in production while enabling new implementation.
  - Running both implementations for testing while implementing the new Axum version.

  It also allows parsing the URL query params for the announce query. For the time being, only the mandatory params.

  The announce request URL has two parameters that contain binary data (20 bytes).

  Sample Announce URL:

  http://0.0.0.0:7070/announce?info_hash=%3B%24U%04%CF%5F%11%BB%DB%E1%20%1C%EAjk%F4Z%EE%1B%C0&peer_id=-qB00000000000000001&port=17548

  The "infohash" and "peer id" cannot be extracted using the generic Axum `Query` extractor because it cannot parse binary data. As you can read [here](https://docs.rs/axum/latest/axum/extract/struct.Query.html).

  I could have used the [RawQuery extractor](https://docs.rs/axum/latest/axum/extract/struct.RawQuery.html), but I thought this way would be much cleaner because you can pass the `AnnounceParams` into the handler like this:

  ```rust
  pub async fn announce_handler(
      State(_tracker): State<Arc<Tracker>>,
      ExtractAnnounceParams(_announce_params): ExtractAnnounceParams,
  ) -> Json<Ok> {
      todo!()
  }
  ```

  And you can also handle the errors in the extractor if you have to return a "bad request" response. Alternatively, I could have handled parsing and error handling inside the handler.

ACKs for top commit:
  da2ce7:
    ACK 995397e

Tree-SHA512: a6f667510ff77339b0c136491ff143a4a09a81085735cbca291df54144968ba7568240dea07e385a52bf2e84637b40cdb55b2cd360b48b64c4537464a8da485c
@josecelano josecelano mentioned this issue Feb 12, 2023
Merged
@josecelano
Copy link
Member Author

josecelano commented Feb 12, 2023 •
edited

I've added the subtasks. I will create a new issue for each subtask keeping this issue to track them.

@josecelano josecelano reopened this Feb 12, 2023
@josecelano josecelano mentioned this issue Feb 12, 2023
Merged
@josecelano josecelano removed a link to a pull request Feb 12, 2023
Axum HTTP tracker: discarded PR #179
Merged
This was referenced Feb 12, 2023
Closed
Merged
Closed
josecelano added a commit that referenced this issue Feb 20, 2023
@josecelano
Merge #185: Axum HTTP tracker: announce request in public mode
46e9e68 
da638d6 docs(http): fix extractor docs (Jose Celano)
30918da refactor(http): [#184] move extractor to extractor mod (Jose Celano)
99dbbe4 refactor(http): [#184] extract announce service in Axum tracker (Jose Celano)
02e2516 feat(http): [#184] Axum extractor for peer IP (Jose Celano)
74ed592 feat(http): [#184] added optional params to announce req in Axum implementation (Jose Celano)
b1612f6 test(http): improve tests (Jose Celano)
3eb7475 feat(http): [#184] normal (non-compact) announce response in axum tracker (Jose Celano)
42bd313 feat: [#184] calculate remote client ip depending on whether the tracker is running on reverse proxy or not (Jose Celano)
8318057 feat: [#184] add dependency: axum-client-ip (Jose Celano)
f327dcf fix(http): [#184] bencoded error responses for announce request (Jose Celano)
d0c8eb0 refactor(http): reorganize mods (Jose Celano)
03024e2 refactor(http): extract function to get client IP on reverse proxy (Jose Celano)

Pull request description:

  It implements the `announce` request handler in the Axum HTTP tracker for `public` mode.

  - [x] Normal (non-compact) response with only mandatory params
  - [x] Optional params for `announce` request
  - [x] Return bencoded error when remote client IP cannot be obtained from the `X-Forwarded-For` header on reverse proxy mode. See commented tests.
  - [x] Refactor: extract service for handler body.

  Out of this PR scope:

  - [ ] Capture unexpected handler errors and convert them into bencoded generic HTTP tracker response errors. Moved to [parent issue](#160).
  - [ ] Compact response.

  **UPDATE**: 16/02/2023

Top commit has no ACKs.

Tree-SHA512: 76f77ed87f64ff6a488090e9013d92cbc516135a770206408a38bcba1c57ceb01154cd87b7169ed01ee12791c12d5e991ab59c4e53ba7c164a146292d6a94677
This was referenced Feb 21, 2023
Closed
Closed
Closed
@josecelano josecelano mentioned this issue Mar 1, 2023
Closed
This was referenced Mar 1, 2023
Closed
Merged
@josecelano
Copy link
Member Author

josecelano commented Mar 1, 2023 •
edited

I'm not going to implement a "catch-all" middleware. The reason I wanted to use it was I was relaying on standard Axum responses; for example, when it can't parse a path params, it returns a 404 BAD REQUEST response with a not bencoded body. As I finally implemented custom extractors, we control all the responses and I think there are no unhandled errors. If we find one we should change it to return the bencoded official error for the HTTP trackers.

This was referenced Mar 3, 2023
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
@josecelano josecelano mentioned this issue Mar 10, 2023
Closed
2 tasks
@josecelano josecelano mentioned this issue May 16, 2023
Closed
12 tasks
@cgbosse cgbosse added the EPIC Contains several subissues label Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Code Cleanup / Refactoring Tidying and Making Neat EPIC Contains several subissues
Projects
Torrust Solution
Archived in project
Milestone
No milestone
Development

No branches or pull requests
2 participants
@josecelano @cgbosse