Auto merge of #2616 - jtgeibel:fix-path-rewrite-logging, r=JohnTitor

Fix logging of original request path

In the original implementation the logging middleware would log the
final rewritten path. The NormalizePath middleware had custom logic
to log the original path, but the other 2 uses of path_mut() were not
handled consistently. The logging middleware now ensures that the
original request path is always logged.

r? @JohnTitor

Author: bors

src/middleware/log_request.rs

@@ -13,10 +13,13 @@ const SLOW_REQUEST_THRESHOLD_MS: u64 = 1000;
 pub(super) struct LogRequests();
 
 struct RequestStart(Instant);
+struct OriginalPath(String);
 
 impl Middleware for LogRequests {
     fn before(&self, req: &mut dyn RequestExt) -> BeforeResult {
         req.mut_extensions().insert(RequestStart(Instant::now()));
+        let path = OriginalPath(req.path().to_string());
+        req.mut_extensions().insert(path);
         Ok(())
     }
 
@@ -59,6 +62,7 @@ pub fn add_custom_metadata<V: Display>(req: &mut dyn RequestExt, key: &'static s
 
 #[cfg(test)]
 pub(crate) fn get_log_message(req: &dyn RequestExt, key: &'static str) -> String {
+    // Unwrap shouldn't panic as no other code has access to the private struct to remove it
     for (k, v) in &req.extensions().find::<CustomMetadata>().unwrap().entries {
         if key == *k {
             return v.clone();
@@ -113,7 +117,12 @@ struct FullPath<'a>(&'a dyn RequestExt);
 
 impl<'a> Display for FullPath<'a> {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", self.0.path())?;
+        // Unwrap shouldn't panic as no other code has access to the private struct to remove it
+        write!(
+            f,
+            "{}",
+            self.0.extensions().find::<OriginalPath>().unwrap().0
+        )?;
         if let Some(q_string) = self.0.query_string() {
             write!(f, "?{}", q_string)?;
         }

src/middleware/normalize_path.rs

@@ -14,33 +14,32 @@ impl Middleware for NormalizePath {
             return Ok(());
         }
 
-        let original = path.to_string();
-        let length = path.len();
-        let path = Path::new(path);
-
-        let path = path
+        let path = Path::new(path)
             .components()
-            .fold(PathBuf::with_capacity(length), |mut result, p| match p {
-                Component::Normal(x) => {
-                    if x != "" {
-                        result.push(x)
-                    };
-                    result
-                }
-                Component::ParentDir => {
-                    result.pop();
-                    result
-                }
-                Component::RootDir => {
-                    result.push(Component::RootDir);
-                    result
-                }
-                _ => result,
-            })
+            .fold(
+                PathBuf::with_capacity(path.len()),
+                |mut result, p| match p {
+                    Component::Normal(x) => {
+                        if x != "" {
+                            result.push(x)
+                        };
+                        result
+                    }
+                    Component::ParentDir => {
+                        result.pop();
+                        result
+                    }
+                    Component::RootDir => {
+                        result.push(Component::RootDir);
+                        result
+                    }
+                    _ => result,
+                },
+            )
             .to_string_lossy()
             .to_string(); // non-Unicode is replaced with U+FFFD REPLACEMENT CHARACTER
 
-        super::log_request::add_custom_metadata(req, "original_path", original);
+        super::log_request::add_custom_metadata(req, "normalized_path", path.clone());
         *req.path_mut() = path;
         Ok(())
     }