Skip to content

Commit

Permalink
Strip non-address characters from Forwarded for=
Browse files Browse the repository at this point in the history
This is something of a followup to actix#2528, which asked for port information to not be included in  when it was taken from the local socket.

The  header's  element may optionally contain port information (https://datatracker.ietf.org/doc/html/rfc7239#section-6).
However, as I understand it,  is *supposed* to only contain an IP address, without port (per actix#2528).

This PR corrects that discrepancy, making it easier to parse the result of this method in application code.

There should not be any compatibility concerns, as anyone parsing the output of  would already need to handle both port and portless cases anyway.
  • Loading branch information
mpalmer committed Apr 26, 2024
1 parent b6d38f0 commit 324a43e
Showing 1 changed file with 25 additions and 3 deletions.
28 changes: 25 additions & 3 deletions actix-web/src/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,19 @@ fn unquote(val: &str) -> &str {
val.trim().trim_start_matches('"').trim_end_matches('"')
}

/// Remove port and IPv6 square brackets from a peer specification.
fn bare_address(val: &str) -> &str {
if val.starts_with("[") {
val.splitn(2, "]:")
.next()
.map(|s| s.trim_start_matches('[').trim_end_matches(']'))
// This shouldn't *actually* ever happen
.unwrap_or(val)
} else {
val.splitn(2, ":").next().unwrap_or(val)
}
}

/// Extracts and trims first value for given header name.
fn first_header_value<'a>(req: &'a RequestHead, name: &'_ HeaderName) -> Option<&'a str> {
let hdr = req.headers.get(name)?.to_str().ok()?;
Expand Down Expand Up @@ -100,7 +113,7 @@ impl ConnectionInfo {
// --- https://datatracker.ietf.org/doc/html/rfc7239#section-5.2

match name.trim().to_lowercase().as_str() {
"for" => realip_remote_addr.get_or_insert_with(|| unquote(val)),
"for" => realip_remote_addr.get_or_insert_with(|| bare_address(unquote(val))),
"proto" => scheme.get_or_insert_with(|| unquote(val)),
"host" => host.get_or_insert_with(|| unquote(val)),
"by" => {
Expand Down Expand Up @@ -368,16 +381,25 @@ mod tests {
.insert_header((header::FORWARDED, r#"for="192.0.2.60:8080""#))
.to_http_request();
let info = req.connection_info();
assert_eq!(info.realip_remote_addr(), Some("192.0.2.60:8080"));
assert_eq!(info.realip_remote_addr(), Some("192.0.2.60"));
}

#[test]
fn forwarded_for_ipv6() {
let req = TestRequest::default()
.insert_header((header::FORWARDED, r#"for="[2001:db8:cafe::17]""#))
.to_http_request();
let info = req.connection_info();
assert_eq!(info.realip_remote_addr(), Some("2001:db8:cafe::17"));
}

#[test]
fn forwarded_for_ipv6_with_port() {
let req = TestRequest::default()
.insert_header((header::FORWARDED, r#"for="[2001:db8:cafe::17]:4711""#))
.to_http_request();
let info = req.connection_info();
assert_eq!(info.realip_remote_addr(), Some("[2001:db8:cafe::17]:4711"));
assert_eq!(info.realip_remote_addr(), Some("2001:db8:cafe::17"));
}

#[test]
Expand Down

0 comments on commit 324a43e

Please sign in to comment.