fix: regression: redirect folders not ending with a slash

This commit is contained in:
Matteo Settenvini 2025-08-21 23:21:42 +02:00
parent 3bda00a7ae
commit 8137566988
Signed by: matteo
GPG key ID: 1C1B12600D81DE05
2 changed files with 63 additions and 71 deletions

View file

@ -17,8 +17,8 @@ use {
Profile, Profile,
providers::{Env, Format as _, Toml}, providers::{Env, Format as _, Toml},
}, },
http::ContentType, http::{ContentType, uri::Origin},
response::{self, Responder, stream::ByteStream}, response::{self, Redirect, Responder, stream::ByteStream},
serde::Serialize, serde::Serialize,
}, },
rocket_dyn_templates::{Template, context}, rocket_dyn_templates::{Template, context},
@ -28,6 +28,7 @@ use {
enum FileView { enum FileView {
Folder(Template), Folder(Template),
Redirect(Redirect),
File(ByteStream<BoxStream<'static, Bytes>>), File(ByteStream<BoxStream<'static, Bytes>>),
} }
@ -38,6 +39,7 @@ impl<'r> Responder<'r, 'r> for FileView {
r.set_header(ContentType::HTML); r.set_header(ContentType::HTML);
r r
}), }),
Self::Redirect(redirect) => redirect.respond_to(req),
Self::File(stream) => stream.respond_to(req), Self::File(stream) => stream.respond_to(req),
} }
} }
@ -75,12 +77,16 @@ impl From<object_store::Error> for Error {
} }
#[rocket::get("/")] #[rocket::get("/")]
async fn index_root(state: &State<Settings>) -> Result<FileView, Error> { async fn index_root(uri: &Origin<'_>, state: &State<Settings>) -> Result<FileView, Error> {
index(None, state).await index(None, uri, state).await
} }
#[rocket::get("/<path..>")] #[rocket::get("/<path..>")]
async fn index(path: Option<PathBuf>, state: &State<Settings>) -> Result<FileView, Error> { async fn index(
path: Option<PathBuf>,
uri: &Origin<'_>,
state: &State<Settings>,
) -> Result<FileView, Error> {
let object_path = if let Some(url_path) = path.as_ref() { let object_path = if let Some(url_path) = path.as_ref() {
let s = url_path.to_str().ok_or(Error::InvalidRequest( let s = url_path.to_str().ok_or(Error::InvalidRequest(
"Path cannot be converted to UTF-8".into(), "Path cannot be converted to UTF-8".into(),
@ -91,21 +97,33 @@ async fn index(path: Option<PathBuf>, state: &State<Settings>) -> Result<FileVie
None None
}; };
/* // We try first to retrieve list an object as a file.
We try first to retrieve list an object as a file. If we fail,
we fallback to retrieving the equivalent folder.
*/
if let Some(object_path) = &object_path if let Some(object_path) = &object_path
&& object_exists(object_path, &state).await? && object_exists(object_path, &state).await?
{ {
log::info!("serving S3 object at {}", &object_path); log::info!("serving S3 object at {}", &object_path);
serve_object(&object_path, &state).await return serve_object(&object_path, &state).await;
} else { }
// If we fail, we fallback to retrieving the equivalent folder.
// For hyperlinks in the generated HTML to work properly, let's
// normalize the path to end with a slash.
if !uri.path().ends_with("/") {
// If the path does not end with a slash, we redirect to
// the normalized path with a slash appended.
let redirect = uri
.map_path(|p| format!("{}/", p))
.expect("cannot append slash to origin URL, this should never happen!");
return Ok(FileView::Redirect(Redirect::permanent(
redirect.to_string(),
)));
}
// We can now assume we have a full path to a folder,
// ending with a slash.
let path = path.unwrap_or_default(); let path = path.unwrap_or_default();
log::info!("listing S3 objects at {}", path.display()); log::info!("listing S3 objects at {}", path.display());
let objects = file_view(object_path, &state).await?; let objects = file_view(object_path, &state).await?;
let rendered = Template::render( let rendered = Template::render(
"index", "index",
context! { context! {
@ -116,7 +134,6 @@ async fn index(path: Option<PathBuf>, state: &State<Settings>) -> Result<FileVie
Ok(FileView::Folder(rendered)) Ok(FileView::Folder(rendered))
} }
}
async fn object_exists(s3_path: &ObjectStorePath, settings: &Settings) -> Result<bool, Error> { async fn object_exists(s3_path: &ObjectStorePath, settings: &Settings) -> Result<bool, Error> {
log::debug!("checking existence of S3 object at {}", s3_path); log::debug!("checking existence of S3 object at {}", s3_path);

View file

@ -8,10 +8,7 @@ use {
scraper::{Html, Selector}, scraper::{Html, Selector},
}; };
#[test_log::test(tokio::test(flavor = "multi_thread"))] async fn create_sample_files(test: &common::Test) -> anyhow::Result<()> {
async fn serves_files() -> anyhow::Result<()> {
let test = common::Test::new().await?;
test.bucket test.bucket
.put( .put(
&ObjectStorePath::from("file.txt"), &ObjectStorePath::from("file.txt"),
@ -26,6 +23,14 @@ async fn serves_files() -> anyhow::Result<()> {
) )
.await?; .await?;
Ok(())
}
#[test_log::test(tokio::test(flavor = "multi_thread"))]
async fn serves_files() -> anyhow::Result<()> {
let test = common::Test::new().await?;
create_sample_files(&test).await?;
let resp = reqwest::get(test.base_url.join("file.txt")?).await?; let resp = reqwest::get(test.base_url.join("file.txt")?).await?;
assert_eq!(resp.bytes().await?, "I am a file"); assert_eq!(resp.bytes().await?, "I am a file");
@ -38,19 +43,7 @@ async fn serves_files() -> anyhow::Result<()> {
#[test_log::test(tokio::test(flavor = "multi_thread"))] #[test_log::test(tokio::test(flavor = "multi_thread"))]
async fn serves_top_level_folder() -> anyhow::Result<()> { async fn serves_top_level_folder() -> anyhow::Result<()> {
let test = common::Test::new().await?; let test = common::Test::new().await?;
create_sample_files(&test).await?;
test.bucket
.put(
&ObjectStorePath::from("file.txt"),
PutPayload::from_static("I am a file".as_bytes()),
)
.await?;
test.bucket
.put(
&ObjectStorePath::from("folder/file.txt"),
PutPayload::from_static("I am a file in a folder".as_bytes()),
)
.await?;
// Check that a file in the toplevel is listed: // Check that a file in the toplevel is listed:
let resp = reqwest::get(test.base_url.clone()).await?; let resp = reqwest::get(test.base_url.clone()).await?;
@ -72,8 +65,11 @@ async fn serves_top_level_folder() -> anyhow::Result<()> {
let selector = let selector =
Selector::parse(r#"table > tbody > tr:nth-child(1) > td:first-child > a"#).unwrap(); Selector::parse(r#"table > tbody > tr:nth-child(1) > td:first-child > a"#).unwrap();
for item in document.select(&selector) { for item in document.select(&selector) {
assert_eq!(item.attr("href"), Some("folder")); // Folders should be listed ending with a slash,
assert_eq!(item.text().next(), Some("folder")); // or HTTP gets confused. This is also due to the
// normalization we do on the path in the main program.
assert_eq!(item.attr("href"), Some("folder/"));
assert_eq!(item.text().next(), Some("folder/"));
} }
let selector = let selector =
@ -89,20 +85,7 @@ async fn serves_top_level_folder() -> anyhow::Result<()> {
#[test_log::test(tokio::test(flavor = "multi_thread"))] #[test_log::test(tokio::test(flavor = "multi_thread"))]
async fn serves_second_level_folder() -> anyhow::Result<()> { async fn serves_second_level_folder() -> anyhow::Result<()> {
let test = common::Test::new().await?; let test = common::Test::new().await?;
create_sample_files(&test).await?;
test.bucket
.put(
&ObjectStorePath::from("file.txt"),
PutPayload::from_static("I am a file".as_bytes()),
)
.await?;
test.bucket
.put(
&ObjectStorePath::from("folder/file.txt"),
PutPayload::from_static("I am a file in a folder".as_bytes()),
)
.await?;
// Check that a file in the second level is listed: // Check that a file in the second level is listed:
let resp = reqwest::get(test.base_url.join("folder/")?).await?; let resp = reqwest::get(test.base_url.join("folder/")?).await?;
@ -140,19 +123,7 @@ async fn serves_second_level_folder() -> anyhow::Result<()> {
#[test_log::test(tokio::test(flavor = "multi_thread"))] #[test_log::test(tokio::test(flavor = "multi_thread"))]
async fn serves_second_level_folder_without_ending_slash() -> anyhow::Result<()> { async fn serves_second_level_folder_without_ending_slash() -> anyhow::Result<()> {
let test = common::Test::new().await?; let test = common::Test::new().await?;
create_sample_files(&test).await?;
test.bucket
.put(
&ObjectStorePath::from("file.txt"),
PutPayload::from_static("I am a file".as_bytes()),
)
.await?;
test.bucket
.put(
&ObjectStorePath::from("folder/file.txt"),
PutPayload::from_static("I am a file in a folder".as_bytes()),
)
.await?;
// Check that a file in the second level is listed even without an ending slash: // Check that a file in the second level is listed even without an ending slash:
let resp = reqwest::get(test.base_url.join("folder")?).await?; let resp = reqwest::get(test.base_url.join("folder")?).await?;
@ -161,6 +132,10 @@ async fn serves_second_level_folder_without_ending_slash() -> anyhow::Result<()>
"Request failed with {}", "Request failed with {}",
resp.status() resp.status()
); );
// Ensure we were redirected to a URL ending with a slash
assert!(resp.url().path().ends_with("/"));
let text = resp.text().await?; let text = resp.text().await?;
println!("{}", &text); println!("{}", &text);
let document = Html::parse_document(&text); let document = Html::parse_document(&text);