Axum HTTP tracker: announce request in public mode

[josecelano]

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

• Normal (non-compact) response with only mandatory params
• Optional params for announce request
• Return bencoded error when remote client IP cannot be obtained from the X-Forwarded-For header on reverse proxy mode. See commented tests.
• 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.
• Compact response.

UPDATE: 16/02/2023

[josecelano]

@WarmBeer @da2ce7 when there is an error with an HTTP tracker request, we return a 200 status code and a bencoded body. For example, when you do not provide the mandatory announce params you get a response body like this:

d14:failure reason81:internal server error: Undefined, src/http/warp_implementation/handlers.rs:195:27e

By the way, In that case, the error is because there is a missing param. I will improve in the new version.

BEP-0003 says:

Tracker responses are bencoded dictionaries. If a tracker response has a key failure reason, then that maps to a human readable string which explains why the query failed, and no other keys are required. Otherwise, it must have two keys: interval, which maps to the number of seconds the downloader should wait between regular rerequests, and peers. peers maps to a list of dictionaries corresponding to peers, each of which contains the keys peer id, ip, and port, which map to the peer's self-selected ID, IP address or dns name as a string, and port number, respectively. Note that downloaders may rerequest on nonscheduled times if an event happens or they need more peers.

I suppose they consider the bencoded dictionary to be "human-readable", but it says nothing about the status code. Should we continue returning a 200 status code even for this kind of error (bad request due to missing params)?

I'm assuming ALL responses are bencoded dictionaries, even error responses. For the time being, I will continue returning the same error codes as the Warp version if you think we do not have to change it, but improving the error message.

[josecelano]

hi @da2ce7 @WarmBeer on this commit I've fixed error responses because there were not bencoded in the initial implementation.

This is a sample error before bencoding it:

Cannot parse query params: invalid param a=b=c in src/http/axum_implementation/query.rs:50:27

I think it does not make sense to expose the location (in src/http/axum_implementation/query.rs:50:27) in the public HTTP tracker API.

On the other hand, being an OS project, It could be helpful to get more info if the error message is not enough. What do you think?

[da2ce7]

I think it does not make sense to expose the location (in src/http/axum_implementation/query.rs:50:27) in the public HTTP tracker API.

I think that it makes sense to have it, as someone can lookup the source-code and find where the error came from... However we should consider including the git-hash, so they can lookup the right version.

[josecelano]

I think it does not make sense to expose the location (in src/http/axum_implementation/query.rs:50:27) in the public HTTP tracker API.

I think that it makes sense to have it, as someone can lookup the source-code and find where the error came from... However we should consider including the git-hash, so they can lookup the right version.

@da2ce7 OK, but do you mean in the error message? I think it would be better if you could get the exact version from your binary running a torrust-tracker --version command or something like that instead of adding the git-hash on every error message.

Anyway, that would be an extra feature not related to this request (we have to include the git-hash first). For this PR, I will continue exposing the error location in the public API responses.

[da2ce7]

@da2ce7 OK, but do you mean in the error message? I think it would be better if you could get the exact version from your binary running a torrust-tracker --version command or something like that instead of adding the git-hash on every error message.

Anyway, that would be an extra feature not related to this request (we have to include the git-hash first). For this PR, I will continue exposing the error location in the public API responses.

Yes, of-course it is out of scope of this , however the consumer of the API doesn't necessarily have access to the server to run a torrust-tracker --version command in the scope of the API provider...

[josecelano]

@da2ce7 OK, but do you mean in the error message? I think it would be better if you could get the exact version from your binary running a torrust-tracker --version command or something like that instead of adding the git-hash on every error message.
Anyway, that would be an extra feature not related to this request (we have to include the git-hash first). For this PR, I will continue exposing the error location in the public API responses.

Yes, of-course it is out of scope of this , however the consumer of the API doesn't necessarily have access to the server to run a torrust-tracker --version command in the scope of the API provider...

Ups, right! OK. Then, we can propose a new feature to include the git-hash in located errors. And it will be available automatically in the API responses.

[josecelano]

@da2ce7 OK, but do you mean in the error message? I think it would be better if you could get the exact version from your binary running a torrust-tracker --version command or something like that instead of adding the git-hash on every error message.
Anyway, that would be an extra feature not related to this request (we have to include the git-hash first). For this PR, I will continue exposing the error location in the public API responses.

Yes, of-course it is out of scope of this , however the consumer of the API doesn't necessarily have access to the server to run a torrust-tracker --version command in the scope of the API provider...

Although we could add a new non-official HTTP tracker endpoint to get the server version: /version. In fact, yesterday, I thought it would be useful to return JSON responses too (not only bencoded responses) if you receive a certain HTTP header.

[josecelano]

hi @da2ce7 @WarmBeer, I've started using this crate to get the remote client IP from HTTP headers when the tracker is running on the reverse proxy.

The reason is you can even have multiple message-header fields with the same field-name, so I decided to rely on that crate because it seems to be much more complicated than parsing a string. Besides, It could make it easier in the future to change the way we get the remote client IP. You can see the changes in this commit.

[josecelano]

Implemented the normal (non-compact) announce response:

• Only with mandatory params: info_hash, peer_id, port.
• Only for tracker in public mode.
• Only normal response (non-compact).

[josecelano]

hi @da2ce7 @WarmBeer this is ready to review. Remember this is just one subtask in this parent issue.