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

Ensure we include the port when parsing authority #2242

Merged
merged 10 commits into from
Nov 14, 2024
45 changes: 37 additions & 8 deletions axum-extra/src/extract/host.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,21 @@ use axum::extract::FromRequestParts;
use http::{
header::{HeaderMap, FORWARDED},
request::Parts,
uri::Authority,
};

const X_FORWARDED_HOST_HEADER_KEY: &str = "X-Forwarded-Host";

/// Extractor that resolves the hostname of the request.
/// Extractor that resolves the host of the request.
///
/// Hostname is resolved through the following, in order:
/// Host is resolved through the following, in order:
/// - `Forwarded` header
/// - `X-Forwarded-Host` header
/// - `Host` header
/// - request target / URI
/// - Authority of the request URI
///
/// See <https://www.rfc-editor.org/rfc/rfc9110.html#name-host-and-authority> for the definition of
/// host.
///
/// Note that user agents can set `X-Forwarded-Host` and `Host` headers to arbitrary values so make
/// sure to validate them to avoid security issues.
Expand Down Expand Up @@ -47,8 +51,8 @@ where
return Ok(Host(host.to_owned()));
}

if let Some(host) = parts.uri.host() {
return Ok(Host(host.to_owned()));
if let Some(authority) = parts.uri.authority() {
return Ok(Host(parse_authority(authority).to_owned()));
}

Err(HostRejection::FailedToResolveHost(FailedToResolveHost))
Expand All @@ -72,12 +76,19 @@ fn parse_forwarded(headers: &HeaderMap) -> Option<&str> {
})
}

fn parse_authority(auth: &Authority) -> &str {
auth.as_str()
.rsplit('@')
.next()
.expect("split always has at least 1 item")
}

#[cfg(test)]
mod tests {
use super::*;
use crate::test_helpers::TestClient;
use axum::{routing::get, Router};
use http::header::HeaderName;
use http::{header::HeaderName, Request};

fn test_client() -> TestClient {
async fn host_as_body(Host(host): Host) -> String {
Expand Down Expand Up @@ -127,8 +138,26 @@ mod tests {

#[crate::test]
async fn uri_host() {
let host = test_client().get("/").await.text().await;
assert!(host.contains("127.0.0.1"));
let client = test_client();
let port = client.server_port();
let host = client.get("/").await.text().await;
assert_eq!(host, format!("127.0.0.1:{port}"));
}

#[crate::test]
async fn ip4_uri_host() {
let mut parts = Request::new(()).into_parts().0;
parts.uri = "https://127.0.0.1:1234/image.jpg".parse().unwrap();
let host = Host::from_request_parts(&mut parts, &()).await.unwrap();
assert_eq!(host.0, "127.0.0.1:1234");
}

#[crate::test]
async fn ip6_uri_host() {
let mut parts = Request::new(()).into_parts().0;
parts.uri = "http://cool:user@[::1]:456/file.txt".parse().unwrap();
let host = Host::from_request_parts(&mut parts, &()).await.unwrap();
assert_eq!(host.0, "[::1]:456");
}

#[test]
Expand Down
2 changes: 2 additions & 0 deletions axum/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

# Unreleased

- **fixed:** Include port number when parsing authority ([#2242])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be in axum-extra's changelog, no? Also, it should mention that this affects the Host extractor, it's currently very hard to tell what this is about.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true, the merge moved the files into axum-extra, but not the changelog :) good catch!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- **fixed:** Skip SSE incompatible chars of `serde_json::RawValue` in `Event::json_data` ([#2992])
- **breaking:** Move `Host` extractor to `axum-extra` ([#2956])
- **added:** Add `method_not_allowed_fallback` to set a fallback when a path matches but there is no handler for the given HTTP method ([#2903])
Expand Down Expand Up @@ -64,6 +65,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- **change:** Update tokio-tungstenite to 0.23 ([#2841])
- **added:** `Serve::local_addr` and `WithGracefulShutdown::local_addr` functions ([#2881])

[#2242]: https://github.com/tokio-rs/axum/pull/2242
[#2653]: https://github.com/tokio-rs/axum/pull/2653
[#2790]: https://github.com/tokio-rs/axum/pull/2790
[#2841]: https://github.com/tokio-rs/axum/pull/2841
Expand Down
5 changes: 5 additions & 0 deletions axum/src/test_helpers/test_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ impl TestClient {
builder: self.client.patch(format!("http://{}{}", self.addr, url)),
}
}

#[allow(dead_code)]
pub(crate) fn server_port(&self) -> u16 {
self.addr.port()
}
}

pub(crate) struct RequestBuilder {
Expand Down