From a2b9823d9d428b2abec75e98afb8dfd412098886 Mon Sep 17 00:00:00 2001 From: Matt Palmer Date: Mon, 10 Jun 2024 09:40:09 +1000 Subject: [PATCH] Strip non-address characters from Forwarded for= (#3343) * 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 --- actix-web/CHANGES.md | 4 ++++ actix-web/src/info.rs | 28 +++++++++++++++++++++++++--- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/actix-web/CHANGES.md b/actix-web/CHANGES.md index 4e74e0902..3d9176dee 100644 --- a/actix-web/CHANGES.md +++ b/actix-web/CHANGES.md @@ -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 diff --git a/actix-web/src/info.rs b/actix-web/src/info.rs index c5d9638f4..aee936ba9 100644 --- a/actix-web/src/info.rs +++ b/actix-web/src/info.rs @@ -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()?; @@ -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" => { @@ -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]