diff --git a/src/resolve.rs b/src/resolve.rs index aa6c6db..7634dd8 100644 --- a/src/resolve.rs +++ b/src/resolve.rs @@ -78,7 +78,10 @@ pub enum ResolveResult { /// The requested file could not be accessed. PermissionDenied, /// A directory was requested as a file. - IsDirectory, + IsDirectory { + /// Path to redirect to. + redirect_to: String, + }, /// The requested file was found. Found(ResolvedFile), } @@ -169,7 +172,18 @@ impl Resolver { // We may have opened a directory for a file request, in which case we redirect. if !is_dir_request && file.is_dir { - return Ok(ResolveResult::IsDirectory); + // Build the redirect path. On Windows, we can't just append the entire path, because + // it contains Windows path separators. Instead, append each component separately. + let mut target = String::with_capacity(path.as_os_str().len() + 2); + target.push('/'); + for component in path.components() { + target.push_str(&component.as_os_str().to_string_lossy()); + target.push('/'); + } + + return Ok(ResolveResult::IsDirectory { + redirect_to: target, + }); } // If not a directory, serve this file. diff --git a/src/response_builder.rs b/src/response_builder.rs index 7b9c934..7139beb 100644 --- a/src/response_builder.rs +++ b/src/response_builder.rs @@ -91,9 +91,10 @@ impl<'a> ResponseBuilder<'a> { ResolveResult::PermissionDenied => HttpResponseBuilder::new() .status(StatusCode::FORBIDDEN) .body(Body::Empty), - ResolveResult::IsDirectory => { - let mut target = self.path.to_owned(); - target.push('/'); + ResolveResult::IsDirectory { + redirect_to: mut target, + } => { + // Preserve any query string from the original request. if let Some(query) = self.query { target.push('?'); target.push_str(query); diff --git a/tests/static.rs b/tests/static.rs index 2cf4898..fb555c8 100644 --- a/tests/static.rs +++ b/tests/static.rs @@ -118,13 +118,32 @@ async fn returns_404_if_file_not_found() { #[tokio::test] async fn redirects_if_trailing_slash_is_missing() { - let harness = Harness::new(vec![("dir/index.html", "this is index")]); + let harness = Harness::new(vec![("foo/bar/index.html", "this is index")]); - let res = harness.get("/dir").await.unwrap(); + let res = harness.get("/foo/bar").await.unwrap(); assert_eq!(res.status(), StatusCode::MOVED_PERMANENTLY); let url = res.headers().get(header::LOCATION).unwrap(); - assert_eq!(url, "/dir/"); + assert_eq!(url, "/foo/bar/"); +} + +#[tokio::test] +async fn redirects_to_sanitized_path() { + let harness = Harness::new(vec![("foo.org/bar/index.html", "this is index")]); + + // Previous versions would base the redirect on the request path, but that is user input, and + // the user could construct a schema-relative redirect this way. + let res = harness.get("//foo.org/bar").await.unwrap(); + assert_eq!(res.status(), StatusCode::MOVED_PERMANENTLY); + + let url = res.headers().get(header::LOCATION).unwrap(); + // TODO: The request path is apparently parsed differently on Windows, but at least the + // resulting redirect is still safe, and that's the important part. + if cfg!(target_os = "windows") { + assert_eq!(url, "/"); + } else { + assert_eq!(url, "/foo.org/bar/"); + } } #[tokio::test]