From 920b70abc4d212a13638efb530e11b21ede16822 Mon Sep 17 00:00:00 2001 From: sigoden Date: Wed, 7 Feb 2024 16:27:22 +0800 Subject: [PATCH] refactor: improve resolve_path and handle_assets, abandon guard_path (#360) --- src/server.rs | 84 +++++++++++++++++++++++++------------------- tests/assets.rs | 22 ++++++------ tests/single_file.rs | 2 +- tests/webdav.rs | 2 +- 4 files changed, 60 insertions(+), 50 deletions(-) diff --git a/src/server.rs b/src/server.rs index 4ef237b..e1c09a0 100644 --- 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) -> Result { - 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 { - 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 = v.chars().collect(); + if chars.len() == 2 && chars[1] == ':' && chars[0].is_ascii_alphabetic() { + return None; + } + } + parts.push(v); + } else { + return None; + } } - path.strip_prefix(prefix.trim_start_matches('/')) + let new_path = parts.join("/"); + let path_prefix = self.args.path_prefix.as_str(); + if path_prefix.is_empty() { + return Some(new_path); + } + 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, size: u64) -> Result 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, size: u64) -> Result 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 8482b2a..3e6a155 100644 --- 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 915f72c..c9e0b71 100644 --- 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 ed30591..1230419 100644 --- 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(()) }