refactor: improve resolve_path and handle_assets, abandon guard_path (#360)

diff --git a/src/server.rs b/src/server.rs
index 4ef237b54bccb46e02f8492207e9a63070b0f590..e1c09a079959ebbabd61ad928b5760496ff73766 100644 (file)
--- a/src/server.rs
+++ b/src/server.rs
@@ -35,7 +35,7 @@ use std::collections::HashMap;
 use std::fs::Metadata;
 use std::io::SeekFrom;
 use std::net::SocketAddr;
-use std::path::{Path, PathBuf};
+use std::path::{Component, Path, PathBuf};
 use std::sync::atomic::{self, AtomicBool};
 use std::sync::Arc;
 use std::time::SystemTime;
@@ -71,7 +71,7 @@ pub struct Server {
 
 impl Server {
     pub fn init(args: Args, running: Arc<AtomicBool>) -> Result<Self> {
-        let assets_prefix = format!("{}__dufs_v{}_/", args.uri_prefix, env!("CARGO_PKG_VERSION"));
+        let assets_prefix = format!("__dufs_v{}__/", env!("CARGO_PKG_VERSION"));
         let single_file_req_paths = if args.path_is_file {
             vec![
                 args.uri_prefix.to_string(),
@@ -144,23 +144,23 @@ impl Server {
         let headers = req.headers();
         let method = req.method().clone();
 
-        if guard_path(req_path, &mut res) {
-            return Ok(res);
-        }
-
-        if method == Method::GET && self.handle_assets(req_path, headers, &mut res).await? {
-            return Ok(res);
-        }
-
-        let authorization = headers.get(AUTHORIZATION);
         let relative_path = match self.resolve_path(req_path) {
             Some(v) => v,
             None => {
-                status_forbid(&mut res);
+                status_bad_request(&mut res, "Invalid Path");
                 return Ok(res);
             }
         };
 
+        if method == Method::GET
+            && self
+                .handle_assets(&relative_path, headers, &mut res)
+                .await?
+        {
+            return Ok(res);
+        }
+
+        let authorization = headers.get(AUTHORIZATION);
         let guard = self.args.auth.guard(&relative_path, &method, authorization);
 
         let (user, access_paths) = match guard {
@@ -302,10 +302,10 @@ impl Server {
                     }
                 } else if is_file {
                     if query_params.contains_key("edit") {
-                        self.handle_deal_file(path, DataKind::Edit, head_only, user, &mut res)
+                        self.handle_edit_file(path, DataKind::Edit, head_only, user, &mut res)
                             .await?;
                     } else if query_params.contains_key("view") {
-                        self.handle_deal_file(path, DataKind::View, head_only, user, &mut res)
+                        self.handle_edit_file(path, DataKind::View, head_only, user, &mut res)
                             .await?;
                     } else {
                         self.handle_send_file(path, headers, head_only, &mut res)
@@ -869,7 +869,7 @@ impl Server {
         Ok(())
     }
 
-    async fn handle_deal_file(
+    async fn handle_edit_file(
         &self,
         path: &Path,
         kind: DataKind,
@@ -901,7 +901,10 @@ impl Server {
             .typed_insert(ContentType::from(mime_guess::mime::TEXT_HTML_UTF_8));
         let output = self
             .html
-            .replace("__ASSETS_PREFIX__", &self.assets_prefix)
+            .replace(
+                "__ASSETS_PREFIX__",
+                &format!("{}{}", self.args.uri_prefix, self.assets_prefix),
+            )
             .replace("__INDEX_DATA__", &serde_json::to_string(&data)?);
         res.headers_mut()
             .typed_insert(ContentLength(output.as_bytes().len() as u64));
@@ -1126,7 +1129,10 @@ impl Server {
             res.headers_mut()
                 .typed_insert(ContentType::from(mime_guess::mime::TEXT_HTML_UTF_8));
             self.html
-                .replace("__ASSETS_PREFIX__", &self.assets_prefix)
+                .replace(
+                    "__ASSETS_PREFIX__",
+                    &format!("{}{}", self.args.uri_prefix, self.assets_prefix),
+                )
                 .replace("__INDEX_DATA__", &serde_json::to_string(&data)?)
         };
         res.headers_mut()
@@ -1168,15 +1174,11 @@ impl Server {
         {
             Some(dest) => dest,
             None => {
-                *res.status_mut() = StatusCode::BAD_REQUEST;
+                status_bad_request(res, "Invalid Destination");
                 return None;
             }
         };
 
-        if guard_path(&dest_path, res) {
-            return None;
-        }
-
         let authorization = headers.get(AUTHORIZATION);
         let guard = self
             .args
@@ -1209,13 +1211,30 @@ impl Server {
     }
 
     fn resolve_path(&self, path: &str) -> Option<String> {
-        let path = path.trim_matches('/');
         let path = decode_uri(path)?;
-        let prefix = self.args.path_prefix.as_str();
-        if prefix == "/" {
-            return Some(path.to_string());
+        let path = path.trim_matches('/');
+        let mut parts = vec![];
+        for comp in Path::new(path).components() {
+            if let Component::Normal(v) = comp {
+                let v = v.to_string_lossy();
+                if cfg!(windows) {
+                    let chars: Vec<char> = v.chars().collect();
+                    if chars.len() == 2 && chars[1] == ':' && chars[0].is_ascii_alphabetic() {
+                        return None;
+                    }
+                }
+                parts.push(v);
+            } else {
+                return None;
+            }
+        }
+        let new_path = parts.join("/");
+        let path_prefix = self.args.path_prefix.as_str();
+        if path_prefix.is_empty() {
+            return Some(new_path);
         }
-        path.strip_prefix(prefix.trim_start_matches('/'))
+        new_path
+            .strip_prefix(path_prefix.trim_start_matches('/'))
             .map(|v| v.trim_matches('/').to_string())
     }
 
@@ -1690,7 +1709,7 @@ fn parse_upload_offset(headers: &HeaderMap<HeaderValue>, size: u64) -> Result<Op
         Some(v) => v,
         None => return Ok(None),
     };
-    let err = || anyhow!("Invalid X-Update-Range header");
+    let err = || anyhow!("Invalid X-Update-Range Header");
     let value = value.to_str().map_err(|_| err())?;
     if value == "append" {
         return Ok(Some(size));
@@ -1698,12 +1717,3 @@ fn parse_upload_offset(headers: &HeaderMap<HeaderValue>, size: u64) -> Result<Op
     let (start, _) = parse_range(value, size).ok_or_else(err)?;
     Ok(Some(start))
 }
-
-fn guard_path(path: &str, res: &mut Response) -> bool {
-    let path = Path::new(path);
-    if path.components().any(|v| v.as_os_str() == "..") {
-        status_bad_request(res, "");
-        return true;
-    }
-    false
-}

diff --git a/tests/assets.rs b/tests/assets.rs
index 8482b2a61baf2fe2c318122a2c605bd237974c14..3e6a155ff1e36ef75d9d1a14c46eccb06c327403 100644 (file)
--- a/tests/assets.rs
+++ b/tests/assets.rs
@@ -11,9 +11,9 @@ use std::process::{Command, Stdio};
 fn assets(server: TestServer) -> Result<(), Error> {
     let ver = env!("CARGO_PKG_VERSION");
     let resp = reqwest::blocking::get(server.url())?;
-    let index_js = format!("/__dufs_v{ver}_/index.js");
-    let index_css = format!("/__dufs_v{ver}_/index.css");
-    let favicon_ico = format!("/__dufs_v{ver}_/favicon.ico");
+    let index_js = format!("/__dufs_v{ver}__/index.js");
+    let index_css = format!("/__dufs_v{ver}__/index.css");
+    let favicon_ico = format!("/__dufs_v{ver}__/favicon.ico");
     let text = resp.text()?;
     println!("{text}");
     assert!(text.contains(&format!(r#"href="{index_css}""#)));
@@ -25,7 +25,7 @@ fn assets(server: TestServer) -> Result<(), Error> {
 #[rstest]
 fn asset_js(server: TestServer) -> Result<(), Error> {
     let url = format!(
-        "{}__dufs_v{}_/index.js",
+        "{}__dufs_v{}__/index.js",
         server.url(),
         env!("CARGO_PKG_VERSION")
     );
@@ -41,7 +41,7 @@ fn asset_js(server: TestServer) -> Result<(), Error> {
 #[rstest]
 fn asset_css(server: TestServer) -> Result<(), Error> {
     let url = format!(
-        "{}__dufs_v{}_/index.css",
+        "{}__dufs_v{}__/index.css",
         server.url(),
         env!("CARGO_PKG_VERSION")
     );
@@ -57,7 +57,7 @@ fn asset_css(server: TestServer) -> Result<(), Error> {
 #[rstest]
 fn asset_ico(server: TestServer) -> Result<(), Error> {
     let url = format!(
-        "{}__dufs_v{}_/favicon.ico",
+        "{}__dufs_v{}__/favicon.ico",
         server.url(),
         env!("CARGO_PKG_VERSION")
     );
@@ -71,9 +71,9 @@ fn asset_ico(server: TestServer) -> Result<(), Error> {
 fn assets_with_prefix(#[with(&["--path-prefix", "xyz"])] server: TestServer) -> Result<(), Error> {
     let ver = env!("CARGO_PKG_VERSION");
     let resp = reqwest::blocking::get(format!("{}xyz/", server.url()))?;
-    let index_js = format!("/xyz/__dufs_v{ver}_/index.js");
-    let index_css = format!("/xyz/__dufs_v{ver}_/index.css");
-    let favicon_ico = format!("/xyz/__dufs_v{ver}_/favicon.ico");
+    let index_js = format!("/xyz/__dufs_v{ver}__/index.js");
+    let index_css = format!("/xyz/__dufs_v{ver}__/index.css");
+    let favicon_ico = format!("/xyz/__dufs_v{ver}__/favicon.ico");
     let text = resp.text()?;
     assert!(text.contains(&format!(r#"href="{index_css}""#)));
     assert!(text.contains(&format!(r#"href="{favicon_ico}""#)));
@@ -86,7 +86,7 @@ fn asset_js_with_prefix(
     #[with(&["--path-prefix", "xyz"])] server: TestServer,
 ) -> Result<(), Error> {
     let url = format!(
-        "{}xyz/__dufs_v{}_/index.js",
+        "{}xyz/__dufs_v{}__/index.js",
         server.url(),
         env!("CARGO_PKG_VERSION")
     );
@@ -115,7 +115,7 @@ fn assets_override(tmpdir: TempDir, port: u16) -> Result<(), Error> {
     let url = format!("http://localhost:{port}");
     let resp = reqwest::blocking::get(&url)?;
     assert!(resp.text()?.starts_with(&format!(
-        "/__dufs_v{}_/index.js;DATA",
+        "/__dufs_v{}__/index.js;DATA",
         env!("CARGO_PKG_VERSION")
     )));
     let resp = reqwest::blocking::get(&url)?;

diff --git a/tests/single_file.rs b/tests/single_file.rs
index 915f72cc476d7cca3a31f0e611ff7af060e939c5..c9e0b717750ec6ab2a8c3ba5b320725ee83bb3d7 100644 (file)
--- a/tests/single_file.rs
+++ b/tests/single_file.rs
@@ -53,7 +53,7 @@ fn path_prefix_single_file(tmpdir: TempDir, port: u16, #[case] file: &str) -> Re
     let resp = reqwest::blocking::get(format!("http://localhost:{port}/xyz/index.html"))?;
     assert_eq!(resp.text()?, "This is index.html");
     let resp = reqwest::blocking::get(format!("http://localhost:{port}"))?;
-    assert_eq!(resp.status(), 403);
+    assert_eq!(resp.status(), 400);
 
     child.kill()?;
     Ok(())

diff --git a/tests/webdav.rs b/tests/webdav.rs
index ed305915b2fd281527d189b9333020201c651595..1230419325bb4dbc4ec3a410e7da2989d518eacd 100644 (file)
--- a/tests/webdav.rs
+++ b/tests/webdav.rs
@@ -49,7 +49,7 @@ fn propfind_404(server: TestServer) -> Result<(), Error> {
 
 #[rstest]
 fn propfind_double_slash(server: TestServer) -> Result<(), Error> {
-    let resp = fetch!(b"PROPFIND", format!("{}/", server.url())).send()?;
+    let resp = fetch!(b"PROPFIND", server.url()).send()?;
     assert_eq!(resp.status(), 207);
     Ok(())
 }