fix: too many redirects caused by gateway adding trailing slash to url

rsync-gateway/src/cache.rs

@@ -256,7 +256,7 @@ impl MaybeCompressed {
                         &mut Infallible,
                     )
                     .expect("infallible");
-                // DEBUG metrics histogram of decompression speed
+                // TODO metrics histogram of decompression speed
                 debug!(elapsed = ?started_at.elapsed(), "decompression complete");
                 Arc::new(deserialized)
             }

rsync-gateway/src/handler.rs

@@ -30,15 +30,23 @@ pub async fn main_handler(
     req: HttpRequest,
 ) -> impl Responder {
     let prefix = prefix.as_str();
+    let full_path = req.path();
     let path = req.match_info().get("path").map_or_else(Vec::new, |path| {
-        percent_encoding::percent_decode(path.trim_start_matches('/').as_bytes()).collect()
+        percent_encoding::percent_decode(path.trim_end_matches('/').as_bytes()).collect()
     });
+    let req_path = if full_path.ends_with('/') {
+        let mut p = path.clone();
+        p.push(b'/');
+        p
+    } else {
+        path.clone()
+    };
 
     let namespace = &endpoint.namespace;
     let s3_prefix = &endpoint.s3_prefix;
     let Some(Revision { revision, generated_at }) = state.revision() else {
         return Either::Left(render_internal_error(
-            &path,
+            &req_path,
             prefix,
             None,
             Utc::now(),
@@ -60,7 +68,7 @@ pub async fn main_handler(
         Err(e) => {
             let query_time = query_start.elapsed();
             return Either::Left(render_internal_error(
-                &path,
+                &req_path,
                 prefix,
                 Some(revision),
                 Utc::now(),
@@ -72,7 +80,14 @@ pub async fn main_handler(
     };
     let query_time = query_start.elapsed();
 
-    Either::Right(resolved.to_responder(&path, prefix, revision, generated_at, query_time))
+    Either::Right(resolved.to_responder(
+        &req_path,
+        full_path,
+        prefix,
+        revision,
+        generated_at,
+        query_time,
+    ))
 }
 
 pub async fn rev_handler(

rsync-gateway/src/main.rs

@@ -60,7 +60,7 @@ pub async fn main() -> Result<()> {
     let mut server = HttpServer::new({
         move || {
             App::new()
-                .wrap(NormalizePath::new(TrailingSlash::Trim).use_redirects())
+                .wrap(NormalizePath::new(TrailingSlash::MergeOnly).use_redirects())
                 .wrap(TracingLogger::default())
                 .configure(cfg.clone())
                 .route("/_metrics", web::get().to(metrics::metrics_handler))

rsync-gateway/src/render.rs

@@ -23,42 +23,41 @@ impl Resolved {
     pub fn to_responder(
         &self,
         req_path: &[u8],
+        full_path: &str,
         prefix: &str,
         revision: i32,
         generated_at: DateTime<Utc>,
         query_time: Duration,
     ) -> impl Responder {
         match self {
             Self::Directory { entries } => {
+                if !req_path.ends_with(b"/") {
+                    let raw_cwd = full_path.rsplit_once('/').map_or(full_path, |(_, cwd)| cwd);
+                    return Either::Right(Redirect::to(format!("{raw_cwd}/")));
+                }
                 let orig_entries = entries.iter().filter(|entry| entry.filename != b".");
                 let parent_entry = ListingEntry {
                     filename: b"..".to_vec(),
                     len: None,
                     modify_time: None,
                     is_dir: true,
                 };
-                let entries = if req_path.is_empty() {
+                let root = req_path.is_empty() || req_path == b"/";
+                let entries = if root {
                     itertools::Either::Left(orig_entries)
                 } else {
                     itertools::Either::Right(iter::once(&parent_entry).chain(orig_entries))
                 };
-                let cwd = if req_path.is_empty() {
-                    prefix.as_bytes()
-                } else {
-                    req_path.rsplit_once_str(b"/").map_or(req_path, |(_, s)| s)
-                };
                 let root_href = req_path_to_root_href(prefix, req_path);
-                let path = String::from_utf8_lossy(req_path);
-                let title = if path.is_empty() {
-                    prefix.to_string()
+                let title = if root {
+                    format!("{prefix}/")
                 } else {
-                    format!("{prefix}/{path}")
+                    format!("{prefix}/{}", String::from_utf8_lossy(req_path))
                 };
                 let components: Vec<_> = title.split('/').collect_vec();
                 let navbar = comps_to_navbar(&components);
                 let rendered = ListingTemplate {
                     title: &title,
-                    cwd,
                     entries,
                     navbar,
                     footer: FooterRevisionTemplate {
@@ -214,7 +213,11 @@ fn comps_to_navbar<'a>(comps: &'a [&'a str]) -> impl TemplateOnce + 'a {
 }
 
 fn req_path_to_root_href(prefix: &str, req_path: &[u8]) -> String {
-    let depth = req_path.find_iter(b"/").count();
+    let depth = if req_path == b"/" {
+        0
+    } else {
+        req_path.find_iter(b"/").count()
+    };
     if depth > 0 {
         iter::repeat("../").take(depth).join("")
     } else if req_path.is_empty() {
@@ -363,7 +366,8 @@ mod tests {
 
     #[proptest(async = "tokio")]
     async fn must_render_resolved_prop(
-        req_path: Vec<u8>,
+        mut req_path: Vec<u8>,
+        raw_path: String,
         prefix: String,
         revision: i32,
         #[strategy(proptest_arbitrary_interop::arb::< DateTime < Utc >> ())] generated_at: DateTime<
@@ -372,13 +376,23 @@ mod tests {
         query_time: Duration,
         #[strategy(resolved_non_regular_strategy())] resolved: Resolved,
     ) {
-        // ensured by actix middleware
-        prop_assume!(!req_path.ends_with(b"/"));
         // ensured by validate_config
         prop_assume!(!prefix.is_empty() && !prefix.starts_with('/'));
+        // no redirect
+        if matches!(resolved, Resolved::Directory { .. }) {
+            req_path.push(b'/');
+        }
+
         let req = TestRequest::get().to_http_request();
         let resp = resolved
-            .to_responder(&req_path, &prefix, revision, generated_at, query_time)
+            .to_responder(
+                &req_path,
+                &raw_path,
+                &prefix,
+                revision,
+                generated_at,
+                query_time,
+            )
             .respond_to(&req);
         let Ok(body) = to_bytes(resp.into_body()).await else {
             panic!("must to bytes");
@@ -399,8 +413,6 @@ mod tests {
         err: String,
         #[strategy(proptest_arbitrary_interop::arb::< Uuid > ())] request_id: Uuid,
     ) {
-        // ensured by actix middleware
-        prop_assume!(!req_path.ends_with(b"/"));
         // ensured by validate_config
         prop_assume!(!prefix.is_empty() && !prefix.starts_with('/'));
         let req = TestRequest::get().to_http_request();

rsync-gateway/src/templates.rs

@@ -126,7 +126,6 @@ where
     N: TemplateOnce,
 {
     pub title: &'a str,
-    pub cwd: &'a [u8],
     pub entries: I,
     pub navbar: N,
     pub footer: T,

rsync-gateway/src/tests.rs

@@ -221,7 +221,7 @@ mod db_required {
             .unwrap();
         let app = test::init_service(
             App::new()
-                .wrap(NormalizePath::new(TrailingSlash::Trim))
+                .wrap(NormalizePath::new(TrailingSlash::MergeOnly))
                 .wrap(TracingLogger::default())
                 .configure(cfg),
         )
@@ -260,7 +260,10 @@ mod db_required {
         }
 
         // Should follow symlinks to dirs.
-        for (uri, _path) in [("/test/m/n", "/test/p"), ("/test/h/i/j/j/j/k", "/test/v/w")] {
+        for (uri, _path) in [
+            ("/test/m/n/", "/test/p/"),
+            ("/test/h/i/j/j/j/k/", "/test/v/w/"),
+        ] {
             let req = test::TestRequest::get().uri(uri).to_request();
             let resp = test::call_service(&app, req).await;
             assert_eq!(resp.status(), StatusCode::OK);
@@ -289,9 +292,16 @@ mod db_required {
             "/test/%E4%BD%A0%E5%A5%BD%20%E4%B8%96%E7%95%8C2",
             "/test/int%C3%A9r%C3%AAt2",
         ] {
-            let req = test::TestRequest::get().uri(uri).to_request();
+            let req = test::TestRequest::get()
+                .uri(&format!("{uri}/"))
+                .to_request();
             let resp = test::call_service(&app, req).await;
             assert_eq!(resp.status(), StatusCode::OK);
+
+            // Redirect to trailing slash.
+            let req = test::TestRequest::get().uri(uri).to_request();
+            let resp = test::call_service(&app, req).await;
+            assert_eq!(resp.status(), StatusCode::TEMPORARY_REDIRECT);
         }
 
         assert!(!listener_handle.is_finished(), "listener died");

rsync-gateway/templates/listing.stpl

@@ -6,7 +6,7 @@
     <meta content="width=device-width, initial-scale=1, shrink-to-fit=no" name="viewport">
     <link href="https://cdn.bootcdn.net/ajax/libs/twitter-bootstrap/4.5.3/css/bootstrap.min.css" rel="stylesheet">
 
-    <title>Index of <%= title %>/ - SJTUG Mirror Index</title>
+    <title>Index of <%= title %> - SJTUG Mirror Index</title>
 </head>
 
 <body>
@@ -24,7 +24,7 @@
         <% for ListingEntry{filename, len, modify_time, is_dir} in entries { %>
         <tr>
             <td>
-                <a href="<%- href(cwd) | disp %>/<%- href(filename) | disp %>">
+                <a href="<%- href(filename) | disp %><% if *is_dir { %>/<% } %>">
                     <%= lossy_display(filename) | disp %><% if *is_dir { %>/<% } %>
                 </a>
             </td>