Auto merge of #4124 - Turbo87:original-path, r=JohnTitor

middleware::log_request: Move `OriginalPath` code into `NormalizePath` middleware

The `NormalizePath` middleware is the one that mutates the `path`, so it should also be responsible for saving the original one, if necessary.

This PR slightly changes the logic in that `OriginalPath` is only added to the request extensions if the path has been changed. The `NormalizePath` middleware now defaults to `request.path()`, if `OriginalPath` is not found in the request extensions.

Author: bors

src/middleware/log_request.rs

@@ -7,6 +7,7 @@ use crate::util::request_header;
 use conduit::{header, RequestExt, StatusCode};
 use conduit_cookie::RequestSession;
 
+use crate::middleware::normalize_path::OriginalPath;
 use crate::middleware::response_timing::ResponseTime;
 use std::fmt::{self, Display, Formatter};
 
@@ -15,13 +16,8 @@ const SLOW_REQUEST_THRESHOLD_MS: u64 = 1000;
 #[derive(Default)]
 pub(super) struct LogRequests();
 
-struct OriginalPath(String);
-
 impl Middleware for LogRequests {
     fn before(&self, req: &mut dyn RequestExt) -> BeforeResult {
-        let path = OriginalPath(req.path().to_string());
-        req.mut_extensions().insert(path);
-
         if let Some(request_id) = req
             .headers()
             .get("x-request-id")
@@ -166,13 +162,16 @@ struct FullPath<'a>(&'a dyn RequestExt);
 
 impl<'a> Display for FullPath<'a> {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        // 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() {
+        let request = self.0;
+
+        let original_path = request.extensions().find::<OriginalPath>();
+        let path = original_path
+            .map(|p| p.0.as_str())
+            .unwrap_or_else(|| request.path());
+
+        write!(f, "{}", path)?;
+
+        if let Some(q_string) = request.query_string() {
             write!(f, "?{}", q_string)?;
         }
         Ok(())

src/middleware/normalize_path.rs

@@ -4,6 +4,8 @@ use super::prelude::*;
 
 use std::path::{Component, Path, PathBuf};
 
+pub struct OriginalPath(pub String);
+
 pub struct NormalizePath;
 
 impl Middleware for NormalizePath {
@@ -14,6 +16,8 @@ impl Middleware for NormalizePath {
             return Ok(());
         }
 
+        let original_path = OriginalPath(path.to_string());
+
         let path = Path::new(path)
             .components()
             .fold(
@@ -40,7 +44,10 @@ impl Middleware for NormalizePath {
             .to_string(); // non-Unicode is replaced with U+FFFD REPLACEMENT CHARACTER
 
         add_custom_metadata(req, "normalized_path", path.clone());
+
         *req.path_mut() = path;
+        req.mut_extensions().insert(original_path);
+
         Ok(())
     }
 }