Merge branch 'master' into error-response-mapping

* 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>

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

actix-web/src/error/error.rs

@@ -6,7 +6,7 @@ use crate::{HttpResponse, ResponseError};
 
 /// General purpose Actix Web error.
 ///
-/// An Actix Web error is used to carry errors from `std::error` through actix in a convenient way.
+/// An Actix Web error is used to carry errors from `std::error` through Actix in a convenient way.
 /// It can be created through converting errors with `into()`.
 ///
 /// Whenever it is created from an external object a response error is created for it that can be
@@ -14,6 +14,7 @@ use crate::{HttpResponse, ResponseError};
 /// you can always get a `ResponseError` reference from it.
 pub struct Error {
     cause: Box<dyn ResponseError>,
+    response_mappers: Vec<Box<dyn Fn(HttpResponse) -> HttpResponse>>,
 }
 
 impl Error {
@@ -29,7 +30,20 @@ impl Error {
 
     /// Shortcut for creating an `HttpResponse`.
     pub fn error_response(&self) -> HttpResponse {
-        self.cause.error_response()
+        let mut res = self.cause.error_response();
+
+        for mapper in &self.response_mappers {
+            res = (mapper)(res);
+        }
+
+        res
+    }
+
+    pub fn add_mapper<F, B>(&mut self, mapper: F)
+    where
+        F: Fn(HttpResponse) -> HttpResponse + 'static,
+    {
+        self.response_mappers.push(Box::new(mapper))
     }
 }
 
@@ -56,6 +70,7 @@ impl<T: ResponseError + 'static> From<T> for Error {
     fn from(err: T) -> Error {
         Error {
             cause: Box::new(err),
+            response_mappers: Vec::new(),
         }
     }
 }

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]