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.

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

Bug in auto_referer implementation #392

Closed
citreae535 opened this issue May 4, 2022 · 1 comment · Fixed by #393
Closed

Bug in auto_referer implementation #392

citreae535 opened this issue May 4, 2022 · 1 comment · Fixed by #393
Labels
bug Something is borken
Milestone

Comments

@citreae535
Copy link

When isahc is asked to follow redirects with auto_referer enabled, it will add a Referer header to subsequent requests after being redirected. However, the current implementation does not remove previous Referer headers. Therefore, the final request will have multiple Referer headers if it is redirected more than once. This is different from curl --location --referer ";auto" which only retains the last Referer header. I think this is a bug and should be fixed.

isahc/src/redirect.rs

Lines 83 to 87 in 2a5c175

// Set referer header.
if auto_referer {
let referer = request_builder.uri_ref().unwrap().to_string();
request_builder = request_builder.header(http::header::REFERER, referer);
}

By the way, the current implementation also does not remove the fragment and userinfo components of the URI, a MUST NOT requirement in RFC 7231. curl conforms to the RFC, and it would be nice if isach does as well.

@sagebind sagebind added the bug Something is borken label May 4, 2022
@sagebind
Copy link
Owner

sagebind commented May 4, 2022

Good catch, thanks for letting me know about this! This is indeed the wrong behavior.

@sagebind sagebind added this to the 1.7.2 milestone May 4, 2022
sagebind added a commit that referenced this issue May 5, 2022
URL fragments and userinfo parts of the authority should not be included in the Referer header when `auto_referer` is enabled.

Fixes #392
sagebind added a commit that referenced this issue May 5, 2022
URL fragments and userinfo parts of the authority should not be included in the Referer header when `auto_referer` is enabled.

Fixes #392
sagebind added a commit that referenced this issue May 6, 2022
Fix various aspects of the `auto_referer` option:

- Fix multiple `Referer` headers being included when two or more redirects are followed in a request
- URL fragments and userinfo parts of the authority should not be included in the `Referer` header
- Don't include a `Referer` header when redirecting from an HTTPS URL to an HTTP URL, as per [RFC 7231](https://httpwg.org/specs/rfc7231.html#header.referer) recommendation
- Scrub sensitive headers when redirecting to a different authority

Fixes #392
sagebind added a commit that referenced this issue May 13, 2022
Fix various aspects of the `auto_referer` option:

- Fix multiple `Referer` headers being included when two or more redirects are followed in a request
- URL fragments and userinfo parts of the authority should not be included in the `Referer` header
- Don't include a `Referer` header when redirecting from an HTTPS URL to an HTTP URL, as per [RFC 7231](https://httpwg.org/specs/rfc7231.html#header.referer) recommendation
- Scrub sensitive headers when redirecting to a different authority

Fixes #392
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is borken
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants