matteo
/
serves3
1
0
Fork 1
Code Issues 1 Pull Requests Packages Projects Releases Wiki Activity

Fix listing of S3 prefixes not terminated by a slash #3

Merged
matteo merged 5 commits from eay/serves3:hackathon_slash_fix_erik_and_eren into main 2024-06-12 12:05:38 +02:00
Conversation 1 Commits 5 Files Changed 13 +89 -3
4 changed files with 89 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 0318729d3f - Show all commits

57
Cargo.lock generated
View File

@ -620,6 +620,12 @@ version = "0.3.29"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "efd193069b0ddadc69c46389b740bbccdd97203899b48d09c5f7969591d6bae2" checksum = "efd193069b0ddadc69c46389b740bbccdd97203899b48d09c5f7969591d6bae2"
[[package]]
name = "futures-timer"
version = "3.0.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f288b0a4f20f9a56b5d1da57e2227c661b7b16168e2f72365f57b63326e29b24"
[[package]] [[package]]
name = "futures-util" name = "futures-util"
version = "0.3.29" version = "0.3.29"
@ -1660,6 +1666,12 @@ version = "0.8.2"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c08c74e62047bb2de4ff487b251e4a92e24f48745648451635cec7d591162d9f" checksum = "c08c74e62047bb2de4ff487b251e4a92e24f48745648451635cec7d591162d9f"
[[package]]
name = "relative-path"
version = "1.9.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e898588f33fdd5b9420719948f9f2a32c922a246964576f71ba7f24f80610fbc"
[[package]] [[package]]
name = "reqwest" name = "reqwest"
version = "0.11.22" version = "0.11.22"
@ -1805,6 +1817,35 @@ dependencies = [
"serde", "serde",
] ]
[[package]]
name = "rstest"
version = "0.19.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9d5316d2a1479eeef1ea21e7f9ddc67c191d497abc8fc3ba2467857abbb68330"
dependencies = [
"futures",
"futures-timer",
"rstest_macros",
"rustc_version",
]
[[package]]
name = "rstest_macros"
version = "0.19.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "04a9df72cc1f67020b0d63ad9bfe4a323e459ea7eb68e03bd9824db49f9a4c25"
dependencies = [
"cfg-if",
"glob",
"proc-macro2",
"quote",
"regex",
"relative-path",
"rustc_version",
"syn 2.0.39",
"unicode-ident",
]
[[package]] [[package]]
name = "rust-ini" name = "rust-ini"
version = "0.18.0" version = "0.18.0"
@ -1853,6 +1894,15 @@ version = "0.1.23"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d626bb9dae77e28219937af045c257c28bfd3f69333c512553507f5f9798cb76" checksum = "d626bb9dae77e28219937af045c257c28bfd3f69333c512553507f5f9798cb76"
[[package]]
name = "rustc_version"
version = "0.4.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "bfa0f585226d2e68097d4f95d113b15b83a82e819ab25717ec0590d9584ef366"
dependencies = [
"semver",
]
[[package]] [[package]]
name = "rustix" name = "rustix"
version = "0.38.25" version = "0.38.25"
@ -1931,6 +1981,12 @@ dependencies = [
"libc", "libc",
] ]
[[package]]
name = "semver"
version = "1.0.22"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "92d43fe69e652f3df9bdc2b85b2854a0825b86e4fb76bc44d945137d053639ca"
[[package]] [[package]]
name = "serde" name = "serde"
version = "1.0.193" version = "1.0.193"
@ -1992,6 +2048,7 @@ dependencies = [
"lazy_static", "lazy_static",
"rocket", "rocket",
"rocket_dyn_templates", "rocket_dyn_templates",
"rstest",
"rust-s3", "rust-s3",
"serde", "serde",
"tempfile", "tempfile",

3
Cargo.toml
View File

@ -28,3 +28,6 @@ rocket_dyn_templates = { version = "0.1.0", features = ["tera"] }
rust-s3 = { version = "0.33", default-features = false, features = ["tokio-native-tls"] } rust-s3 = { version = "0.33", default-features = false, features = ["tokio-native-tls"] }
serde = { version = "1.0" } serde = { version = "1.0" }
tempfile = { version = "3.6" } tempfile = { version = "3.6" }
[dev-dependencies]
rstest = "0.19"

30
src/main.rs
View File

@ -78,6 +78,7 @@ enum FileView {
#[derive(Serialize)] #[derive(Serialize)]
struct FileViewItem { struct FileViewItem {
parent: String,
path: String, path: String,
size: String, size: String,
size_bytes: u64, size_bytes: u64,
@ -106,7 +107,7 @@ async fn index(path: PathBuf) -> Result<FileView, Error> {
We try first to retrieve list an object as a file. If we fail, We try first to retrieve list an object as a file. If we fail,
we fallback to retrieving the equivalent folder. we fallback to retrieving the equivalent folder.
*/ */
if let Ok(result) = s3_serve_file(&path).await { if let Ok(result) = s3_serve_file(&path).await {
Ok(result) Ok(result)
} else { } else {
@ -159,7 +160,7 @@ async fn s3_fileview(path: &PathBuf) -> Result<Vec<FileViewItem>, Error> {
}; };
let s3_objects = BUCKET let s3_objects = BUCKET
.list(s3_folder_path, Some("/".into())) .list(s3_folder_path.clone(), Some("/".into()))
.await .await
.map_err(|_| Error::NotFound("Object not found".into()))?; .map_err(|_| Error::NotFound("Object not found".into()))?;
@ -175,6 +176,7 @@ async fn s3_fileview(path: &PathBuf) -> Result<Vec<FileViewItem>, Error> {
let folders = list.common_prefixes.iter().flatten().map(|dir| { let folders = list.common_prefixes.iter().flatten().map(|dir| {
let path = dir.prefix.strip_prefix(&prefix); let path = dir.prefix.strip_prefix(&prefix);
path.map(|path| FileViewItem { path.map(|path| FileViewItem {
parent:s3_folder_path.clone(),
path: path.to_owned(), path: path.to_owned(),
size_bytes: 0, size_bytes: 0,
size: "[DIR]".to_owned(), size: "[DIR]".to_owned(),
@ -185,6 +187,7 @@ async fn s3_fileview(path: &PathBuf) -> Result<Vec<FileViewItem>, Error> {
let files = list.contents.iter().map(|obj| { let files = list.contents.iter().map(|obj| {
let path = obj.key.strip_prefix(&prefix); let path = obj.key.strip_prefix(&prefix);
path.map(|path| FileViewItem { path.map(|path| FileViewItem {
parent:s3_folder_path.clone(),
matteo marked this conversation as resolved Outdated
matteo commented 2024-04-19 09:48:25 +02:00
Outdated
Review
Copy Link

The last usage of a string can simply be moved instead of cloned to avoid a needless copy.

The last usage of a string can simply be moved instead of cloned to avoid a needless copy.
path: path.to_owned(), path: path.to_owned(),
size_bytes: obj.size, size_bytes: obj.size,
size: size_bytes_to_human(obj.size), size: size_bytes_to_human(obj.size),
@ -238,3 +241,26 @@ fn rocket() -> _ {
.unwrap() .unwrap()
})) }))
} }
// Test section starts
#[cfg(test)]
mod tests {
use rstest::rstest;
// Note this useful idiom: importing names from outer (for mod tests) scope.
matteo marked this conversation as resolved Outdated
matteo commented 2024-04-19 09:42:45 +02:00
Outdated
Review
Copy Link

Remove comment

Remove comment
use super::*;
#[rstest]
#[case(1024, "1.024 kB")]
#[case(10240, "10.240 kB")]
#[case(1024*1024, "1.049 MB")]
#[case(1024*1024*1024, "1.074 GB")]
#[case(0, "0.000 B")]
#[case(u64::MAX, format!("{:.3} GB",u64::MAX as f64/(1_000_000_000.0)))]
#[case(u64::MIN, format!("{:.3} B",u64::MIN as f64))]
fn test_size_bytes_to_human(#[case] bytes: u64, #[case] expected: String) {
println!("{}",size_bytes_to_human(bytes));
matteo marked this conversation as resolved Outdated
matteo commented 2024-04-19 09:43:11 +02:00
Outdated
Review
Copy Link

Formatting, please run cargo fmt on the sources

Formatting, please run `cargo fmt` on the sources
assert_eq!(size_bytes_to_human(bytes), expected);
}
}

2
templates/index.html.tera
View File

@ -44,7 +44,7 @@
{% for object in objects %} {% for object in objects %}
<tr> <tr>
<td><a href="{{ object.path | urlencode }}" data-size-bytes="{{ object.size_bytes }}">{{ object.path }}</a></td> <td><a href="/{{ object.parent | urlencode }}{{ object.path | urlencode }}" data-size-bytes="{{ object.size_bytes }}">{{ object.path }}</a></td>
matteo marked this conversation as resolved Outdated
matteo commented 2024-04-19 09:45:52 +02:00
Outdated
Review
Copy Link

This way of making paths absolutes would not work if the service was to be deployed as a subfolder instead than a virtualhost in the HTTP server configuration.

Imagine the service running via a reverse proxy at https://some.website.com/subfolder.

Then a link of the form /parent/object would resolve to https://some.website.com/parent/object instead of https://some.website.com/subfolder/parent/object.

It is therefore better to use the uri! macro to build the path instead. Could you please move the building of the URI in the Rust portion of the code, and here just use the computed result?

Rocket can be configured then to handle this correctly (which is up to the end user).

This way of making paths absolutes would not work if the service was to be deployed as a subfolder instead than a virtualhost in the HTTP server configuration. Imagine the service running via a reverse proxy at `https://some.website.com/subfolder`. Then a link of the form `/parent/object` would resolve to `https://some.website.com/parent/object` instead of `https://some.website.com/subfolder/parent/object`. It is therefore better to use the [uri! macro](https://api.rocket.rs/v0.5/rocket/macro.uri) to build the path instead. Could you please move the building of the URI in the Rust portion of the code, and here just use the computed result? Rocket can be configured then to handle this correctly (which is up to the end user).
<td>{{ object.size }}</td> <td>{{ object.size }}</td>
<td>{{ object.last_modification}}</td> <td>{{ object.last_modification}}</td>
</tr> </tr>