Skip to content

Commit

Permalink
Strip non-address characters from Forwarded for= (#3343)
Browse files Browse the repository at this point in the history
* Strip non-address characters from Forwarded for=

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.

* Update CHANGES.md

---------

Co-authored-by: Rob Ede <robjtede@icloud.com>
  • Loading branch information
mpalmer and robjtede authored Jun 9, 2024
1 parent da56de4 commit a2b9823
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 3 deletions.
4 changes: 4 additions & 0 deletions actix-web/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Fixed

- `ConnectionInfo::realip_remote_addr()` now handles IPv6 addresses from `Forwarded` header correctly. Previously, it sometimes returned the forwarded port as well.

## 4.7.0

### Added
Expand Down
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.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 a2b9823

Please sign in to comment.