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 #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 #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 ba7fd04 commit 4d3d6f8
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 3 deletions.
1 change: 1 addition & 0 deletions actix-web/CHANGES.md
Expand Up @@ -9,6 +9,7 @@
### Changed

- Minimum supported Rust version (MSRV) is now 1.72.
- `ConnectionInfo.realip_remote_addr()` now returns just IP addresses from `Forwarded` header.

## 4.5.1

Expand Down
28 changes: 25 additions & 3 deletions actix-web/src/info.rs
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.split("]:")
.next()
.map(|s| s.trim_start_matches('[').trim_end_matches(']'))
// This shouldn't *actually* ever happen
.unwrap_or(val)
} else {
val.split(':').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 4d3d6f8

Please sign in to comment.