From f319f281642f459247047e733cd869aafa752d09 Mon Sep 17 00:00:00 2001 From: "John T. Wodder II" Date: Wed, 14 Feb 2024 12:10:59 -0500 Subject: [PATCH 1/4] Add breadcrumbs to web view --- CHANGELOG.md | 1 + src/dav/html.rs | 26 +++++ src/dav/mod.rs | 24 +++- src/dav/path.rs | 146 ++++++++++++++----------- src/dav/static/styles.css | 5 + src/dav/templates/collection.html.tera | 5 + src/paths/component.rs | 12 +- src/paths/purepath.rs | 9 ++ 8 files changed, 158 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b0069f..fd94257 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ In Development - Disable log coloration when stderr is not a terminal - Suppress noisy & irrelevant log messages from various dependencies - Log errors that cause 404 and 500 responses +- Added breadcrumbs to HTML views of collections v0.2.0 (2024-02-07) ------------------- diff --git a/src/dav/html.rs b/src/dav/html.rs index 3322423..dab85fa 100644 --- a/src/dav/html.rs +++ b/src/dav/html.rs @@ -1,6 +1,7 @@ use super::util::Href; use super::{DavCollection, DavItem, DavResource, ResourceKind}; use crate::consts::HTML_TIMESTAMP_FORMAT; +use crate::paths::Component; use serde::{ser::Serializer, Serialize}; use tera::{Context, Error, Tera}; use thiserror::Error; @@ -43,12 +44,19 @@ impl Templater { #[derive(Clone, Debug, Eq, PartialEq, Serialize)] pub(super) struct CollectionContext { pub(super) title: String, + pub(super) breadcrumbs: Vec, pub(super) rows: Vec, pub(super) package_url: &'static str, pub(super) package_version: &'static str, pub(super) package_commit: Option<&'static str>, } +#[derive(Clone, Debug, Eq, PartialEq, Serialize)] +pub(super) struct Link { + name: String, + href: Href, +} + #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd, Serialize)] pub(super) struct ColRow { name: String, @@ -144,3 +152,21 @@ fn maybe_timestamp( None => serializer.serialize_none(), } } + +pub(super) fn make_breadcrumbs(pathparts: Vec) -> Vec { + let mut links = Vec::with_capacity(pathparts.len().saturating_add(1)); + let mut cumpath = String::from("/"); + links.push(Link { + name: String::from("dandidav"), + href: Href::from_path(&cumpath), + }); + for p in pathparts { + cumpath.push_str(&p); + cumpath.push('/'); + links.push(Link { + name: p.into(), + href: Href::from_path(&cumpath), + }); + } + links +} diff --git a/src/dav/mod.rs b/src/dav/mod.rs index 5d910d4..d3e5369 100644 --- a/src/dav/mod.rs +++ b/src/dav/mod.rs @@ -11,6 +11,7 @@ use self::util::*; use self::xml::*; use crate::consts::{DAV_XML_CONTENT_TYPE, HTML_CONTENT_TYPE}; use crate::dandi::*; +use crate::paths::Component; use crate::zarrman::*; use axum::{ body::Body, @@ -64,14 +65,17 @@ impl DandiDav { let m = req.method(); match m { &Method::GET => { - let Some(path) = DavPath::parse_uri_path(uri_path) else { + let Some(parts) = split_uri_path(uri_path) else { return Ok(not_found()); }; - self.get(&path, uri_path).await + let Some(path) = DavPath::from_components(parts.clone()) else { + return Ok(not_found()); + }; + self.get(&path, parts).await } &Method::OPTIONS => Ok(StatusCode::NO_CONTENT.into_response()), m if m.as_str().eq_ignore_ascii_case("PROPFIND") => { - let Some(path) = DavPath::parse_uri_path(uri_path) else { + let Some(path) = split_uri_path(uri_path).and_then(DavPath::from_components) else { return Ok(not_found()); }; match req.extract::<(FiniteDepth, PropFind), _>().await { @@ -83,7 +87,11 @@ impl DandiDav { } } - async fn get(&self, path: &DavPath, uri_path: &str) -> Result, DavError> { + async fn get( + &self, + path: &DavPath, + pathparts: Vec, + ) -> Result, DavError> { match self.resolve_with_children(path).await? { DavResourceWithChildren::Collection { col, children } => { let mut rows = children.into_iter().map(ColRow::from).collect::>(); @@ -91,8 +99,14 @@ impl DandiDav { if path != &DavPath::Root { rows.insert(0, ColRow::parentdir(col.parent_web_link())); } + let mut title = format!("{} \u{2014} /", self.title); + for p in &pathparts { + title.push_str(p); + title.push('/'); + } let context = CollectionContext { - title: format!("{} — {}", self.title, uri_path), + title, + breadcrumbs: make_breadcrumbs(pathparts), rows, package_url: env!("CARGO_PKG_REPOSITORY"), package_version: env!("CARGO_PKG_VERSION"), diff --git a/src/dav/path.rs b/src/dav/path.rs index 3263c57..b2c8b3c 100644 --- a/src/dav/path.rs +++ b/src/dav/path.rs @@ -1,6 +1,6 @@ use crate::consts::FAST_NOT_EXIST; use crate::dandi::{DandisetId, PublishedVersionId}; -use crate::paths::PurePath; +use crate::paths::{Component, ParseComponentError, PurePath}; #[derive(Clone, Debug, Eq, PartialEq)] pub(super) enum DavPath { @@ -32,26 +32,23 @@ pub(super) enum DavPath { } impl DavPath { - pub(super) fn parse_uri_path(s: &str) -> Option { - let Ok(path) = percent_encoding::percent_decode_str(s).decode_utf8() else { - return None; - }; - let mut parts = SplitComponents::new(&path); - let Some(p1) = parts.next() else { + pub(super) fn from_components(parts: Vec) -> Option { + let mut iter = parts.into_iter(); + let Some(p1) = iter.next() else { return Some(DavPath::Root); }; if p1.eq_ignore_ascii_case("dandisets") { - let Some(did) = parts.next() else { + let Some(did) = iter.next() else { return Some(DavPath::DandisetIndex); }; let Ok(dandiset_id) = did.parse::() else { return None; }; - let Some(p3) = parts.next() else { + let Some(p3) = iter.next() else { return Some(DavPath::Dandiset { dandiset_id }); }; let version = if p3.eq_ignore_ascii_case("releases") { - let Some(v) = parts.next() else { + let Some(v) = iter.next() else { return Some(DavPath::DandisetReleases { dandiset_id }); }; let Ok(pv) = v.parse::() else { @@ -65,32 +62,25 @@ impl DavPath { } else { return None; }; - let path = join_parts(parts)?; - if path.is_empty() { - Some(DavPath::Version { + match join_parts(iter)? { + None => Some(DavPath::Version { dandiset_id, version, - }) - } else if path == "dandiset.yaml" { - Some(DavPath::DandisetYaml { + }), + Some(p) if p == "dandiset.yaml" => Some(DavPath::DandisetYaml { dandiset_id, version, - }) - } else { - let path = PurePath::try_from(path).expect("should be valid path"); - Some(DavPath::DandiResource { + }), + Some(path) => Some(DavPath::DandiResource { dandiset_id, version, path, - }) + }), } } else if p1.eq_ignore_ascii_case("zarrs") { - let path = join_parts(parts)?; - if path.is_empty() { - Some(DavPath::ZarrIndex) - } else { - let path = PurePath::try_from(path).expect("should be valid path"); - Some(DavPath::ZarrPath { path }) + match join_parts(iter)? { + None => Some(DavPath::ZarrIndex), + Some(path) => Some(DavPath::ZarrPath { path }), } } else { None @@ -105,6 +95,28 @@ pub(super) enum VersionSpec { Latest, } +pub(super) fn split_uri_path(s: &str) -> Option> { + // TODO: Convert decoding-failures into DavError: + let path = percent_encoding::percent_decode_str(s).decode_utf8().ok()?; + let mut parts = Vec::new(); + for p in SplitComponents::new(&path) { + match p.parse::() { + Ok(c) => parts.push(c), + Err(ParseComponentError::Empty) => unreachable!("part should not be empty"), + Err(ParseComponentError::Slash) => { + unreachable!("part should not contain / after splitting on /") + } + // TODO: Report NULs as DavErrors: + Err(ParseComponentError::Nul) => return None, + Err(ParseComponentError::CurDir) => (), + Err(ParseComponentError::ParentDir) => { + let _ = parts.pop(); + } + } + } + Some(parts) +} + #[derive(Clone, Debug, Eq, PartialEq)] struct SplitComponents<'a>(&'a str); @@ -133,24 +145,19 @@ impl<'a> Iterator for SplitComponents<'a> { impl std::iter::FusedIterator for SplitComponents<'_> {} -fn join_parts(parts: SplitComponents<'_>) -> Option { - let mut path = String::new(); - for p in parts { - if p == "." { - continue; - } else if p == ".." || FAST_NOT_EXIST.binary_search(&p).is_ok() { - // axum collapses `..` components in requests; the only way a `..` - // could have snuck in is if the component were percent-escaped, in - // which case we're going to reject the user's meddling. - return None; - } else { - if !path.is_empty() { - path.push('/'); - } - path.push_str(p); - } - } - Some(path) +fn is_fast_not_exist(s: &str) -> bool { + FAST_NOT_EXIST.binary_search(&s).is_ok() +} + +// - Returns `None` if path does not exist due to containing a `FAST_NOT_EXIST` +// component +// - Returns `Some(None)` if path is empty/root/has no components +fn join_parts>(iter: I) -> Option> { + let parts = iter.collect::>(); + parts + .iter() + .all(|c| !is_fast_not_exist(c)) + .then(|| PurePath::from_components(parts)) } #[cfg(test)] @@ -165,8 +172,6 @@ mod tests { #[case("/dandisets/draft")] #[case("/dandisets/000123/0.201234.1")] #[case("/dandisets/000123/releases/draft")] - #[case("/dandisets/000123/draft/foo/../bar")] - #[case("/dandisets/000123/draft/foo/%2e%2e/bar")] #[case("/dandisets/000123/draft/.git/index")] #[case("/dandisets/000123/draft/foo/.svn")] #[case("/dandisets/000123/draft/foo/%2esvn")] @@ -174,7 +179,8 @@ mod tests { #[case("/dandisets/000123/draft/foo/.nols/bar")] #[case("/dandisets/000123/draft/.bzr")] fn test_bad_uri_paths(#[case] path: &str) { - assert_eq!(DavPath::parse_uri_path(path), None); + let parts = split_uri_path(path).unwrap(); + assert_eq!(DavPath::from_components(parts), None); } #[rstest] @@ -182,7 +188,8 @@ mod tests { #[case("/")] #[case("//")] fn test_root(#[case] path: &str) { - assert_eq!(DavPath::parse_uri_path(path), Some(DavPath::Root)); + let parts = split_uri_path(path).unwrap(); + assert_eq!(DavPath::from_components(parts), Some(DavPath::Root)); } #[rstest] @@ -193,7 +200,11 @@ mod tests { #[case("/Dandisets")] #[case("/DandiSets")] fn test_dandiset_index(#[case] path: &str) { - assert_eq!(DavPath::parse_uri_path(path), Some(DavPath::DandisetIndex)); + let parts = split_uri_path(path).unwrap(); + assert_eq!( + DavPath::from_components(parts), + Some(DavPath::DandisetIndex) + ); } #[rstest] @@ -203,7 +214,8 @@ mod tests { #[case("/Dandisets/000123")] #[case("/DandiSets/000123")] fn test_dandiset(#[case] path: &str) { - assert_matches!(DavPath::parse_uri_path(path), Some(DavPath::Dandiset {dandiset_id}) => { + let parts = split_uri_path(path).unwrap(); + assert_matches!(DavPath::from_components(parts), Some(DavPath::Dandiset {dandiset_id}) => { assert_eq!(dandiset_id, "000123"); }); } @@ -214,7 +226,8 @@ mod tests { #[case("/Dandisets/000123/Releases")] #[case("/DandiSets/000123/ReLeAsEs/")] fn test_dandiset_releases(#[case] path: &str) { - assert_matches!(DavPath::parse_uri_path(path), Some(DavPath::DandisetReleases {dandiset_id}) => { + let parts = split_uri_path(path).unwrap(); + assert_matches!(DavPath::from_components(parts), Some(DavPath::DandisetReleases {dandiset_id}) => { assert_eq!(dandiset_id, "000123"); }); } @@ -225,7 +238,8 @@ mod tests { #[case("/Dandisets/000123/Draft")] #[case("/DandiSets/000123/dRaFt/")] fn test_dandiset_draft(#[case] path: &str) { - assert_matches!(DavPath::parse_uri_path(path), Some(DavPath::Version {dandiset_id, version}) => { + let parts = split_uri_path(path).unwrap(); + assert_matches!(DavPath::from_components(parts), Some(DavPath::Version {dandiset_id, version}) => { assert_eq!(dandiset_id, "000123"); assert_eq!(version, VersionSpec::Draft); }); @@ -237,7 +251,8 @@ mod tests { #[case("/Dandisets/000123/Latest")] #[case("/DandiSets/000123/LaTeST/")] fn test_dandiset_latest(#[case] path: &str) { - assert_matches!(DavPath::parse_uri_path(path), Some(DavPath::Version {dandiset_id, version}) => { + let parts = split_uri_path(path).unwrap(); + assert_matches!(DavPath::from_components(parts), Some(DavPath::Version {dandiset_id, version}) => { assert_eq!(dandiset_id, "000123"); assert_eq!(version, VersionSpec::Latest); }); @@ -249,7 +264,8 @@ mod tests { #[case("/Dandisets/000123/Releases//0.240123.42")] #[case("/DandiSets/000123/ReLeAsEs/0.240123.42//")] fn test_dandiset_published_version(#[case] path: &str) { - assert_matches!(DavPath::parse_uri_path(path), Some(DavPath::Version {dandiset_id, version}) => { + let parts = split_uri_path(path).unwrap(); + assert_matches!(DavPath::from_components(parts), Some(DavPath::Version {dandiset_id, version}) => { assert_eq!(dandiset_id, "000123"); assert_matches!(version, VersionSpec::Published(v) => { assert_eq!(v, "0.240123.42"); @@ -263,7 +279,8 @@ mod tests { #[case("/Dandisets/000123/Draft/dandiset.yaml")] #[case("/DandiSets/000123/dRaFt/dandiset.yaml")] fn test_dandiset_draft_dandiset_yaml(#[case] path: &str) { - assert_matches!(DavPath::parse_uri_path(path), Some(DavPath::DandisetYaml {dandiset_id, version}) => { + let parts = split_uri_path(path).unwrap(); + assert_matches!(DavPath::from_components(parts), Some(DavPath::DandisetYaml {dandiset_id, version}) => { assert_eq!(dandiset_id, "000123"); assert_eq!(version, VersionSpec::Draft); }); @@ -278,8 +295,11 @@ mod tests { #[case("/dandisets/000123/draft/foo%20bar", "foo bar")] #[case("/dandisets/000123/draft/foo/./bar", "foo/bar")] #[case("/dandisets/000123/draft//foo//bar/", "foo/bar")] + #[case("/dandisets/000123/draft/foo/../bar", "bar")] + #[case("/dandisets/000123/draft/foo/%2e%2e/bar", "bar")] fn test_dandiset_draft_resource(#[case] s: &str, #[case] respath: &str) { - assert_matches!(DavPath::parse_uri_path(s), Some(DavPath::DandiResource {dandiset_id, version, path}) => { + let parts = split_uri_path(s).unwrap(); + assert_matches!(DavPath::from_components(parts), Some(DavPath::DandiResource {dandiset_id, version, path}) => { assert_eq!(dandiset_id, "000123"); assert_eq!(version, VersionSpec::Draft); assert_eq!(path, respath); @@ -296,7 +316,8 @@ mod tests { #[case("/dandisets/000123/latest/foo/./bar", "foo/bar")] #[case("/dandisets/000123/latest//foo//bar/", "foo/bar")] fn test_dandiset_latest_resource(#[case] s: &str, #[case] respath: &str) { - assert_matches!(DavPath::parse_uri_path(s), Some(DavPath::DandiResource {dandiset_id, version, path}) => { + let parts = split_uri_path(s).unwrap(); + assert_matches!(DavPath::from_components(parts), Some(DavPath::DandiResource {dandiset_id, version, path}) => { assert_eq!(dandiset_id, "000123"); assert_eq!(version, VersionSpec::Latest); assert_eq!(path, respath); @@ -316,7 +337,8 @@ mod tests { #[case("/dandisets/000123/RELEASES/0.240123.42/foo/./bar", "foo/bar")] #[case("/dandisets/000123/releases/0.240123.42//foo//bar/", "foo/bar")] fn test_dandiset_publish_version_resource(#[case] s: &str, #[case] respath: &str) { - assert_matches!(DavPath::parse_uri_path(s), Some(DavPath::DandiResource {dandiset_id, version, path}) => { + let parts = split_uri_path(s).unwrap(); + assert_matches!(DavPath::from_components(parts), Some(DavPath::DandiResource {dandiset_id, version, path}) => { assert_eq!(dandiset_id, "000123"); assert_matches!(version, VersionSpec::Published(v) => { assert_eq!(v, "0.240123.42"); @@ -333,7 +355,8 @@ mod tests { #[case("/Zarrs")] #[case("/ZARRS")] fn test_zarr_index(#[case] path: &str) { - assert_eq!(DavPath::parse_uri_path(path), Some(DavPath::ZarrIndex)); + let parts = split_uri_path(path).unwrap(); + assert_eq!(DavPath::from_components(parts), Some(DavPath::ZarrIndex)); } #[rstest] @@ -342,7 +365,8 @@ mod tests { #[case("/zarrs/123/abc", "123/abc")] #[case("/ZARRS/123/ABC", "123/ABC")] fn test_zarr_path(#[case] s: &str, #[case] respath: &str) { - assert_matches!(DavPath::parse_uri_path(s), Some(DavPath::ZarrPath {path}) => { + let parts = split_uri_path(s).unwrap(); + assert_matches!(DavPath::from_components(parts), Some(DavPath::ZarrPath {path}) => { assert_eq!(path, respath); }); } diff --git a/src/dav/static/styles.css b/src/dav/static/styles.css index dfdd2d3..5d64eb0 100644 --- a/src/dav/static/styles.css +++ b/src/dav/static/styles.css @@ -1,3 +1,8 @@ +div.breadcrumbs { + margin-bottom: 16px; + font: 11px SFMono-Regular, Consolas, Liberation Mono, Menlo, monospace; +} + table { border-collapse: collapse; border-spacing: 0; diff --git a/src/dav/templates/collection.html.tera b/src/dav/templates/collection.html.tera index 984ce1e..283e36f 100644 --- a/src/dav/templates/collection.html.tera +++ b/src/dav/templates/collection.html.tera @@ -5,6 +5,11 @@ +
+ {% for bc in breadcrumbs %} + {{bc.name}} {% if not loop.last %} / {% endif %} + {% endfor %} +
diff --git a/src/paths/component.rs b/src/paths/component.rs index 076f88c..7901b7d 100644 --- a/src/paths/component.rs +++ b/src/paths/component.rs @@ -12,8 +12,10 @@ fn validate(s: &str) -> Result<(), ParseComponentError> { Err(ParseComponentError::Slash) } else if s.contains('\0') { Err(ParseComponentError::Nul) - } else if s == "." || s == ".." { - Err(ParseComponentError::SpecialDir) + } else if s == "." { + Err(ParseComponentError::CurDir) + } else if s == ".." { + Err(ParseComponentError::ParentDir) } else { Ok(()) } @@ -41,8 +43,10 @@ pub(crate) enum ParseComponentError { Slash, #[error("path components cannot contain NUL")] Nul, - #[error(r#"path components cannot equal "." or "..""#)] - SpecialDir, + #[error(r#"path components cannot equal ".""#)] + CurDir, + #[error(r#"path components cannot equal "..""#)] + ParentDir, } #[cfg(test)] diff --git a/src/paths/purepath.rs b/src/paths/purepath.rs index 6993e6c..1f3bad3 100644 --- a/src/paths/purepath.rs +++ b/src/paths/purepath.rs @@ -83,6 +83,15 @@ impl PurePath { self.0.push('/'); self.0.push_str(c.as_ref()); } + + pub(crate) fn from_components>(iter: I) -> Option { + let mut iter = iter.into_iter(); + let mut path = PurePath::from(iter.next()?); + for c in iter { + path.push(&c); + } + Some(path) + } } impl From for PurePath { From e6f91f027da4cd31f465d53fcc405449a995331b Mon Sep 17 00:00:00 2001 From: "John T. Wodder II" Date: Wed, 14 Feb 2024 12:20:40 -0500 Subject: [PATCH 2/4] Test SplitComponents --- src/dav/path.rs | 458 ++++++++++++++++++++++++++++-------------------- 1 file changed, 268 insertions(+), 190 deletions(-) diff --git a/src/dav/path.rs b/src/dav/path.rs index b2c8b3c..e4b627b 100644 --- a/src/dav/path.rs +++ b/src/dav/path.rs @@ -163,211 +163,289 @@ fn join_parts>(iter: I) -> Option #[cfg(test)] mod tests { use super::*; - use assert_matches::assert_matches; - use rstest::rstest; - - #[rstest] - #[case("/foo")] - #[case("/dandisets/123")] - #[case("/dandisets/draft")] - #[case("/dandisets/000123/0.201234.1")] - #[case("/dandisets/000123/releases/draft")] - #[case("/dandisets/000123/draft/.git/index")] - #[case("/dandisets/000123/draft/foo/.svn")] - #[case("/dandisets/000123/draft/foo/%2esvn")] - #[case("/dandisets/000123/draft/foo%2f.svn")] - #[case("/dandisets/000123/draft/foo/.nols/bar")] - #[case("/dandisets/000123/draft/.bzr")] - fn test_bad_uri_paths(#[case] path: &str) { - let parts = split_uri_path(path).unwrap(); - assert_eq!(DavPath::from_components(parts), None); - } - #[rstest] - #[case("")] - #[case("/")] - #[case("//")] - fn test_root(#[case] path: &str) { - let parts = split_uri_path(path).unwrap(); - assert_eq!(DavPath::from_components(parts), Some(DavPath::Root)); - } + mod split_components { + use super::*; - #[rstest] - #[case("/dandisets")] - #[case("/dandisets/")] - #[case("/dandisets//")] - #[case("//dandisets/")] - #[case("/Dandisets")] - #[case("/DandiSets")] - fn test_dandiset_index(#[case] path: &str) { - let parts = split_uri_path(path).unwrap(); - assert_eq!( - DavPath::from_components(parts), - Some(DavPath::DandisetIndex) - ); - } + #[test] + fn empty() { + let s = ""; + let mut parts = SplitComponents::new(s); + assert!(parts.next().is_none()); + } - #[rstest] - #[case("/dandisets/000123")] - #[case("/dandisets/000123/")] - #[case("/dandisets//000123")] - #[case("/Dandisets/000123")] - #[case("/DandiSets/000123")] - fn test_dandiset(#[case] path: &str) { - let parts = split_uri_path(path).unwrap(); - assert_matches!(DavPath::from_components(parts), Some(DavPath::Dandiset {dandiset_id}) => { - assert_eq!(dandiset_id, "000123"); - }); - } + #[test] + fn slash() { + let s = "/"; + let mut parts = SplitComponents::new(s); + assert!(parts.next().is_none()); + } - #[rstest] - #[case("/dandisets/000123/releases")] - #[case("/dandisets/000123/releases/")] - #[case("/Dandisets/000123/Releases")] - #[case("/DandiSets/000123/ReLeAsEs/")] - fn test_dandiset_releases(#[case] path: &str) { - let parts = split_uri_path(path).unwrap(); - assert_matches!(DavPath::from_components(parts), Some(DavPath::DandisetReleases {dandiset_id}) => { - assert_eq!(dandiset_id, "000123"); - }); - } + #[test] + fn double_slash() { + let s = "//"; + let mut parts = SplitComponents::new(s); + assert!(parts.next().is_none()); + } - #[rstest] - #[case("/dandisets/000123/draft")] - #[case("/dandisets/000123/draft/")] - #[case("/Dandisets/000123/Draft")] - #[case("/DandiSets/000123/dRaFt/")] - fn test_dandiset_draft(#[case] path: &str) { - let parts = split_uri_path(path).unwrap(); - assert_matches!(DavPath::from_components(parts), Some(DavPath::Version {dandiset_id, version}) => { - assert_eq!(dandiset_id, "000123"); - assert_eq!(version, VersionSpec::Draft); - }); - } + #[test] + fn foo() { + let s = "/foo"; + let parts = SplitComponents::new(s).collect::>(); + assert_eq!(parts, ["foo"]); + } + + #[test] + fn foo_slash() { + let s = "/foo/"; + let parts = SplitComponents::new(s).collect::>(); + assert_eq!(parts, ["foo"]); + } - #[rstest] - #[case("/dandisets/000123/latest")] - #[case("/dandisets/000123/latest/")] - #[case("/Dandisets/000123/Latest")] - #[case("/DandiSets/000123/LaTeST/")] - fn test_dandiset_latest(#[case] path: &str) { - let parts = split_uri_path(path).unwrap(); - assert_matches!(DavPath::from_components(parts), Some(DavPath::Version {dandiset_id, version}) => { - assert_eq!(dandiset_id, "000123"); - assert_eq!(version, VersionSpec::Latest); - }); + #[test] + fn no_slash_foo() { + let s = "foo"; + let parts = SplitComponents::new(s).collect::>(); + assert_eq!(parts, ["foo"]); + } + + #[test] + fn foo_bar() { + let s = "/foo/bar"; + let parts = SplitComponents::new(s).collect::>(); + assert_eq!(parts, ["foo", "bar"]); + } + + #[test] + fn foo_bar_slash() { + let s = "/foo/bar/"; + let parts = SplitComponents::new(s).collect::>(); + assert_eq!(parts, ["foo", "bar"]); + } + + #[test] + fn foo_bar_all_double_slash() { + let s = "//foo//bar//"; + let parts = SplitComponents::new(s).collect::>(); + assert_eq!(parts, ["foo", "bar"]); + } + + #[test] + fn foo_double_slash_bar() { + let s = "/foo//bar"; + let parts = SplitComponents::new(s).collect::>(); + assert_eq!(parts, ["foo", "bar"]); + } } - #[rstest] - #[case("/dandisets/000123/releases/0.240123.42")] - #[case("/dandisets/000123/releases/0.240123.42/")] - #[case("/Dandisets/000123/Releases//0.240123.42")] - #[case("/DandiSets/000123/ReLeAsEs/0.240123.42//")] - fn test_dandiset_published_version(#[case] path: &str) { - let parts = split_uri_path(path).unwrap(); - assert_matches!(DavPath::from_components(parts), Some(DavPath::Version {dandiset_id, version}) => { - assert_eq!(dandiset_id, "000123"); - assert_matches!(version, VersionSpec::Published(v) => { - assert_eq!(v, "0.240123.42"); + mod dav_path_from_components { + use super::*; + use assert_matches::assert_matches; + use rstest::rstest; + + #[rstest] + #[case("/foo")] + #[case("/dandisets/123")] + #[case("/dandisets/draft")] + #[case("/dandisets/000123/0.201234.1")] + #[case("/dandisets/000123/releases/draft")] + #[case("/dandisets/000123/draft/.git/index")] + #[case("/dandisets/000123/draft/foo/.svn")] + #[case("/dandisets/000123/draft/foo/%2esvn")] + #[case("/dandisets/000123/draft/foo%2f.svn")] + #[case("/dandisets/000123/draft/foo/.nols/bar")] + #[case("/dandisets/000123/draft/.bzr")] + fn test_bad_uri_paths(#[case] path: &str) { + let parts = split_uri_path(path).unwrap(); + assert_eq!(DavPath::from_components(parts), None); + } + + #[rstest] + #[case("")] + #[case("/")] + #[case("//")] + fn test_root(#[case] path: &str) { + let parts = split_uri_path(path).unwrap(); + assert_eq!(DavPath::from_components(parts), Some(DavPath::Root)); + } + + #[rstest] + #[case("/dandisets")] + #[case("/dandisets/")] + #[case("/dandisets//")] + #[case("//dandisets/")] + #[case("/Dandisets")] + #[case("/DandiSets")] + fn test_dandiset_index(#[case] path: &str) { + let parts = split_uri_path(path).unwrap(); + assert_eq!( + DavPath::from_components(parts), + Some(DavPath::DandisetIndex) + ); + } + + #[rstest] + #[case("/dandisets/000123")] + #[case("/dandisets/000123/")] + #[case("/dandisets//000123")] + #[case("/Dandisets/000123")] + #[case("/DandiSets/000123")] + fn test_dandiset(#[case] path: &str) { + let parts = split_uri_path(path).unwrap(); + assert_matches!(DavPath::from_components(parts), Some(DavPath::Dandiset {dandiset_id}) => { + assert_eq!(dandiset_id, "000123"); }); - }); - } + } - #[rstest] - #[case("/dandisets/000123/draft/dandiset.yaml")] - #[case("/dandisets/000123/draft/dandiset.yaml/")] - #[case("/Dandisets/000123/Draft/dandiset.yaml")] - #[case("/DandiSets/000123/dRaFt/dandiset.yaml")] - fn test_dandiset_draft_dandiset_yaml(#[case] path: &str) { - let parts = split_uri_path(path).unwrap(); - assert_matches!(DavPath::from_components(parts), Some(DavPath::DandisetYaml {dandiset_id, version}) => { - assert_eq!(dandiset_id, "000123"); - assert_eq!(version, VersionSpec::Draft); - }); - } + #[rstest] + #[case("/dandisets/000123/releases")] + #[case("/dandisets/000123/releases/")] + #[case("/Dandisets/000123/Releases")] + #[case("/DandiSets/000123/ReLeAsEs/")] + fn test_dandiset_releases(#[case] path: &str) { + let parts = split_uri_path(path).unwrap(); + assert_matches!(DavPath::from_components(parts), Some(DavPath::DandisetReleases {dandiset_id}) => { + assert_eq!(dandiset_id, "000123"); + }); + } - #[rstest] - #[case("/dandisets/000123/draft/Dandiset.yaml", "Dandiset.yaml")] - #[case("/dandisets/000123/draft/dandiset.yml", "dandiset.yml")] - #[case("/dandisets/000123/draft/foo", "foo")] - #[case("/dandisets/000123/draft/foo/bar", "foo/bar")] - #[case("/dandisets/000123/draft/foo%2fbar", "foo/bar")] - #[case("/dandisets/000123/draft/foo%20bar", "foo bar")] - #[case("/dandisets/000123/draft/foo/./bar", "foo/bar")] - #[case("/dandisets/000123/draft//foo//bar/", "foo/bar")] - #[case("/dandisets/000123/draft/foo/../bar", "bar")] - #[case("/dandisets/000123/draft/foo/%2e%2e/bar", "bar")] - fn test_dandiset_draft_resource(#[case] s: &str, #[case] respath: &str) { - let parts = split_uri_path(s).unwrap(); - assert_matches!(DavPath::from_components(parts), Some(DavPath::DandiResource {dandiset_id, version, path}) => { - assert_eq!(dandiset_id, "000123"); - assert_eq!(version, VersionSpec::Draft); - assert_eq!(path, respath); - }); - } + #[rstest] + #[case("/dandisets/000123/draft")] + #[case("/dandisets/000123/draft/")] + #[case("/Dandisets/000123/Draft")] + #[case("/DandiSets/000123/dRaFt/")] + fn test_dandiset_draft(#[case] path: &str) { + let parts = split_uri_path(path).unwrap(); + assert_matches!(DavPath::from_components(parts), Some(DavPath::Version {dandiset_id, version}) => { + assert_eq!(dandiset_id, "000123"); + assert_eq!(version, VersionSpec::Draft); + }); + } - #[rstest] - #[case("/dandisets/000123/latest/Dandiset.yaml", "Dandiset.yaml")] - #[case("/dandisets/000123/latest/dandiset.yml", "dandiset.yml")] - #[case("/dandisets/000123/latest/foo", "foo")] - #[case("/dandisets/000123/latest/foo/bar", "foo/bar")] - #[case("/dandisets/000123/latest/foo%2fbar", "foo/bar")] - #[case("/dandisets/000123/latest/foo%20bar", "foo bar")] - #[case("/dandisets/000123/latest/foo/./bar", "foo/bar")] - #[case("/dandisets/000123/latest//foo//bar/", "foo/bar")] - fn test_dandiset_latest_resource(#[case] s: &str, #[case] respath: &str) { - let parts = split_uri_path(s).unwrap(); - assert_matches!(DavPath::from_components(parts), Some(DavPath::DandiResource {dandiset_id, version, path}) => { - assert_eq!(dandiset_id, "000123"); - assert_eq!(version, VersionSpec::Latest); - assert_eq!(path, respath); - }); - } + #[rstest] + #[case("/dandisets/000123/latest")] + #[case("/dandisets/000123/latest/")] + #[case("/Dandisets/000123/Latest")] + #[case("/DandiSets/000123/LaTeST/")] + fn test_dandiset_latest(#[case] path: &str) { + let parts = split_uri_path(path).unwrap(); + assert_matches!(DavPath::from_components(parts), Some(DavPath::Version {dandiset_id, version}) => { + assert_eq!(dandiset_id, "000123"); + assert_eq!(version, VersionSpec::Latest); + }); + } - #[rstest] - #[case( - "/dandisets/000123/releases/0.240123.42/Dandiset.yaml", - "Dandiset.yaml" - )] - #[case("/dandisets/000123/releases/0.240123.42/dandiset.yml", "dandiset.yml")] - #[case("/dandisets/000123/releases/0.240123.42/foo", "foo")] - #[case("/dandisets/000123/Releases/0.240123.42/foo/bar", "foo/bar")] - #[case("/dandisets/000123/rElEaSeS/0.240123.42/foo%2fbar", "foo/bar")] - #[case("/dandisets/000123/ReLeAsEs/0.240123.42/foo%20bar", "foo bar")] - #[case("/dandisets/000123/RELEASES/0.240123.42/foo/./bar", "foo/bar")] - #[case("/dandisets/000123/releases/0.240123.42//foo//bar/", "foo/bar")] - fn test_dandiset_publish_version_resource(#[case] s: &str, #[case] respath: &str) { - let parts = split_uri_path(s).unwrap(); - assert_matches!(DavPath::from_components(parts), Some(DavPath::DandiResource {dandiset_id, version, path}) => { - assert_eq!(dandiset_id, "000123"); - assert_matches!(version, VersionSpec::Published(v) => { - assert_eq!(v, "0.240123.42"); + #[rstest] + #[case("/dandisets/000123/releases/0.240123.42")] + #[case("/dandisets/000123/releases/0.240123.42/")] + #[case("/Dandisets/000123/Releases//0.240123.42")] + #[case("/DandiSets/000123/ReLeAsEs/0.240123.42//")] + fn test_dandiset_published_version(#[case] path: &str) { + let parts = split_uri_path(path).unwrap(); + assert_matches!(DavPath::from_components(parts), Some(DavPath::Version {dandiset_id, version}) => { + assert_eq!(dandiset_id, "000123"); + assert_matches!(version, VersionSpec::Published(v) => { + assert_eq!(v, "0.240123.42"); + }); }); - assert_eq!(path, respath); - }); - } + } - #[rstest] - #[case("/zarrs")] - #[case("/zarrs/")] - #[case("/zarrs//")] - #[case("//zarrs/")] - #[case("/Zarrs")] - #[case("/ZARRS")] - fn test_zarr_index(#[case] path: &str) { - let parts = split_uri_path(path).unwrap(); - assert_eq!(DavPath::from_components(parts), Some(DavPath::ZarrIndex)); - } + #[rstest] + #[case("/dandisets/000123/draft/dandiset.yaml")] + #[case("/dandisets/000123/draft/dandiset.yaml/")] + #[case("/Dandisets/000123/Draft/dandiset.yaml")] + #[case("/DandiSets/000123/dRaFt/dandiset.yaml")] + fn test_dandiset_draft_dandiset_yaml(#[case] path: &str) { + let parts = split_uri_path(path).unwrap(); + assert_matches!(DavPath::from_components(parts), Some(DavPath::DandisetYaml {dandiset_id, version}) => { + assert_eq!(dandiset_id, "000123"); + assert_eq!(version, VersionSpec::Draft); + }); + } + + #[rstest] + #[case("/dandisets/000123/draft/Dandiset.yaml", "Dandiset.yaml")] + #[case("/dandisets/000123/draft/dandiset.yml", "dandiset.yml")] + #[case("/dandisets/000123/draft/foo", "foo")] + #[case("/dandisets/000123/draft/foo/bar", "foo/bar")] + #[case("/dandisets/000123/draft/foo%2fbar", "foo/bar")] + #[case("/dandisets/000123/draft/foo%20bar", "foo bar")] + #[case("/dandisets/000123/draft/foo/./bar", "foo/bar")] + #[case("/dandisets/000123/draft//foo//bar/", "foo/bar")] + #[case("/dandisets/000123/draft/foo/../bar", "bar")] + #[case("/dandisets/000123/draft/foo/%2e%2e/bar", "bar")] + fn test_dandiset_draft_resource(#[case] s: &str, #[case] respath: &str) { + let parts = split_uri_path(s).unwrap(); + assert_matches!(DavPath::from_components(parts), Some(DavPath::DandiResource {dandiset_id, version, path}) => { + assert_eq!(dandiset_id, "000123"); + assert_eq!(version, VersionSpec::Draft); + assert_eq!(path, respath); + }); + } + + #[rstest] + #[case("/dandisets/000123/latest/Dandiset.yaml", "Dandiset.yaml")] + #[case("/dandisets/000123/latest/dandiset.yml", "dandiset.yml")] + #[case("/dandisets/000123/latest/foo", "foo")] + #[case("/dandisets/000123/latest/foo/bar", "foo/bar")] + #[case("/dandisets/000123/latest/foo%2fbar", "foo/bar")] + #[case("/dandisets/000123/latest/foo%20bar", "foo bar")] + #[case("/dandisets/000123/latest/foo/./bar", "foo/bar")] + #[case("/dandisets/000123/latest//foo//bar/", "foo/bar")] + fn test_dandiset_latest_resource(#[case] s: &str, #[case] respath: &str) { + let parts = split_uri_path(s).unwrap(); + assert_matches!(DavPath::from_components(parts), Some(DavPath::DandiResource {dandiset_id, version, path}) => { + assert_eq!(dandiset_id, "000123"); + assert_eq!(version, VersionSpec::Latest); + assert_eq!(path, respath); + }); + } - #[rstest] - #[case("/zarrs/123", "123")] - #[case("/zarrs/123/", "123")] - #[case("/zarrs/123/abc", "123/abc")] - #[case("/ZARRS/123/ABC", "123/ABC")] - fn test_zarr_path(#[case] s: &str, #[case] respath: &str) { - let parts = split_uri_path(s).unwrap(); - assert_matches!(DavPath::from_components(parts), Some(DavPath::ZarrPath {path}) => { - assert_eq!(path, respath); - }); + #[rstest] + #[case( + "/dandisets/000123/releases/0.240123.42/Dandiset.yaml", + "Dandiset.yaml" + )] + #[case("/dandisets/000123/releases/0.240123.42/dandiset.yml", "dandiset.yml")] + #[case("/dandisets/000123/releases/0.240123.42/foo", "foo")] + #[case("/dandisets/000123/Releases/0.240123.42/foo/bar", "foo/bar")] + #[case("/dandisets/000123/rElEaSeS/0.240123.42/foo%2fbar", "foo/bar")] + #[case("/dandisets/000123/ReLeAsEs/0.240123.42/foo%20bar", "foo bar")] + #[case("/dandisets/000123/RELEASES/0.240123.42/foo/./bar", "foo/bar")] + #[case("/dandisets/000123/releases/0.240123.42//foo//bar/", "foo/bar")] + fn test_dandiset_publish_version_resource(#[case] s: &str, #[case] respath: &str) { + let parts = split_uri_path(s).unwrap(); + assert_matches!(DavPath::from_components(parts), Some(DavPath::DandiResource {dandiset_id, version, path}) => { + assert_eq!(dandiset_id, "000123"); + assert_matches!(version, VersionSpec::Published(v) => { + assert_eq!(v, "0.240123.42"); + }); + assert_eq!(path, respath); + }); + } + + #[rstest] + #[case("/zarrs")] + #[case("/zarrs/")] + #[case("/zarrs//")] + #[case("//zarrs/")] + #[case("/Zarrs")] + #[case("/ZARRS")] + fn test_zarr_index(#[case] path: &str) { + let parts = split_uri_path(path).unwrap(); + assert_eq!(DavPath::from_components(parts), Some(DavPath::ZarrIndex)); + } + + #[rstest] + #[case("/zarrs/123", "123")] + #[case("/zarrs/123/", "123")] + #[case("/zarrs/123/abc", "123/abc")] + #[case("/ZARRS/123/ABC", "123/ABC")] + fn test_zarr_path(#[case] s: &str, #[case] respath: &str) { + let parts = split_uri_path(s).unwrap(); + assert_matches!(DavPath::from_components(parts), Some(DavPath::ZarrPath {path}) => { + assert_eq!(path, respath); + }); + } } } From 98e37a23d1a88394295c8666385c4abfd4ce6163 Mon Sep 17 00:00:00 2001 From: "John T. Wodder II" Date: Wed, 14 Feb 2024 12:43:33 -0500 Subject: [PATCH 3/4] Test split_uri_path() --- src/dav/path.rs | 135 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 115 insertions(+), 20 deletions(-) diff --git a/src/dav/path.rs b/src/dav/path.rs index e4b627b..0092fff 100644 --- a/src/dav/path.rs +++ b/src/dav/path.rs @@ -62,7 +62,7 @@ impl DavPath { } else { return None; }; - match join_parts(iter)? { + match PurePath::from_components(iter) { None => Some(DavPath::Version { dandiset_id, version, @@ -78,7 +78,7 @@ impl DavPath { }), } } else if p1.eq_ignore_ascii_case("zarrs") { - match join_parts(iter)? { + match PurePath::from_components(iter) { None => Some(DavPath::ZarrIndex), Some(path) => Some(DavPath::ZarrPath { path }), } @@ -100,6 +100,9 @@ pub(super) fn split_uri_path(s: &str) -> Option> { let path = percent_encoding::percent_decode_str(s).decode_utf8().ok()?; let mut parts = Vec::new(); for p in SplitComponents::new(&path) { + if is_fast_not_exist(p) { + return None; + } match p.parse::() { Ok(c) => parts.push(c), Err(ParseComponentError::Empty) => unreachable!("part should not be empty"), @@ -146,18 +149,8 @@ impl<'a> Iterator for SplitComponents<'a> { impl std::iter::FusedIterator for SplitComponents<'_> {} fn is_fast_not_exist(s: &str) -> bool { - FAST_NOT_EXIST.binary_search(&s).is_ok() -} - -// - Returns `None` if path does not exist due to containing a `FAST_NOT_EXIST` -// component -// - Returns `Some(None)` if path is empty/root/has no components -fn join_parts>(iter: I) -> Option> { - let parts = iter.collect::>(); - parts - .iter() - .all(|c| !is_fast_not_exist(c)) - .then(|| PurePath::from_components(parts)) + let s = s.to_ascii_lowercase(); + FAST_NOT_EXIST.binary_search(&&*s).is_ok() } #[cfg(test)] @@ -238,6 +231,114 @@ mod tests { } } + mod split_uri_path { + use super::*; + use rstest::rstest; + + #[rstest] + #[case("")] + #[case("/")] + #[case("/.")] + #[case("%2f%2e")] + #[case("/..")] + #[case("/%2e.")] + fn root(#[case] s: &str) { + let parts = split_uri_path(s).unwrap(); + assert!(parts.is_empty()); + } + + #[rstest] + #[case("foo")] + #[case("/foo")] + #[case("/foo/")] + #[case("//foo//")] + fn foo(#[case] s: &str) { + let parts = split_uri_path(s).unwrap(); + assert_eq!(parts, ["foo"]); + } + + #[rstest] + #[case("foo/bar")] + #[case("/foo/bar")] + #[case("/foo%2fbar")] + #[case("/foo%2fbar%2f")] + #[case("/foo/bar/")] + #[case("//foo//bar//")] + fn foo_bar(#[case] s: &str) { + let parts = split_uri_path(s).unwrap(); + assert_eq!(parts, ["foo", "bar"]); + } + + #[rstest] + #[case("/foo/./bar")] + #[case("/./foo/bar")] + #[case("/foo/bar/.")] + #[case("/foo/bar/./")] + fn curdir(#[case] s: &str) { + let parts = split_uri_path(s).unwrap(); + assert_eq!(parts, ["foo", "bar"]); + } + + #[rstest] + #[case("/foo/../bar")] + #[case("/foo/%2e./bar")] + #[case("/foo/%2e%2e/bar")] + #[case("/foo/.%2e/bar")] + fn foo_parent_bar(#[case] s: &str) { + let parts = split_uri_path(s).unwrap(); + assert_eq!(parts, ["bar"]); + } + + #[rstest] + #[case("/foo/bar/..")] + #[case("/foo/bar/%2e.")] + #[case("/foo/bar/%2e%2e")] + #[case("/foo/bar/.%2e")] + #[case("/foo/bar/.%2E/")] + fn foo_bar_parent(#[case] s: &str) { + let parts = split_uri_path(s).unwrap(); + assert_eq!(parts, ["foo"]); + } + + #[rstest] + #[case("/../foo/bar")] + #[case("/%2e./foo/bar")] + #[case("/%2e%2e/foo/bar")] + #[case("/.%2e/foo/bar")] + fn parent_foo_bar(#[case] s: &str) { + let parts = split_uri_path(s).unwrap(); + assert_eq!(parts, ["foo", "bar"]); + } + + #[rstest] + #[case("/foo\0bar")] + #[case("/foo%00bar")] + fn nul(#[case] s: &str) { + assert_eq!(split_uri_path(s), None); + } + + #[test] + fn non_utf8() { + assert_eq!(split_uri_path("/f%f6%f6"), None); + } + + #[rstest] + #[case("/.git")] + #[case("/.bzr")] + #[case("/.nols")] + #[case("/.svn")] + #[case("/zarrs/.git/index")] + #[case("/zarrs/.git/../001")] + #[case("/zarrs/%2e%67%69%74")] + #[case("/foo/.GIT/config")] + #[case("/foo/.Svn/trunk")] + #[case("/foo/.NoLs/bar")] + #[case("/foo/.Bzr/cathedral")] + fn fast_not_exist(#[case] s: &str) { + assert_eq!(split_uri_path(s), None); + } + } + mod dav_path_from_components { use super::*; use assert_matches::assert_matches; @@ -249,12 +350,6 @@ mod tests { #[case("/dandisets/draft")] #[case("/dandisets/000123/0.201234.1")] #[case("/dandisets/000123/releases/draft")] - #[case("/dandisets/000123/draft/.git/index")] - #[case("/dandisets/000123/draft/foo/.svn")] - #[case("/dandisets/000123/draft/foo/%2esvn")] - #[case("/dandisets/000123/draft/foo%2f.svn")] - #[case("/dandisets/000123/draft/foo/.nols/bar")] - #[case("/dandisets/000123/draft/.bzr")] fn test_bad_uri_paths(#[case] path: &str) { let parts = split_uri_path(path).unwrap(); assert_eq!(DavPath::from_components(parts), None); From 66517756fb0a89f9b26e5314860966d622bf87db Mon Sep 17 00:00:00 2001 From: "John T. Wodder II" Date: Wed, 14 Feb 2024 12:45:58 -0500 Subject: [PATCH 4/4] Update docs --- CHANGELOG.md | 1 + src/consts.rs | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd94257..37bf49a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ In Development - Suppress noisy & irrelevant log messages from various dependencies - Log errors that cause 404 and 500 responses - Added breadcrumbs to HTML views of collections +- `FAST_NOT_EXIST` components are now checked for case-insensitively v0.2.0 (2024-02-07) ------------------- diff --git a/src/consts.rs b/src/consts.rs index e957b74..599cd5e 100644 --- a/src/consts.rs +++ b/src/consts.rs @@ -39,7 +39,8 @@ pub(crate) static HTML_TIMESTAMP_FORMAT: &[FormatItem<'_>] = format_description! ); /// If a client makes a request for a resource with one of these names as a -/// component, assume it doesn't exist without checking the Archive. +/// component (case insensitive), assume it doesn't exist without bothering to +/// check the backend. /// /// This list must be kept in sorted order; this is enforced by a test below. pub(crate) static FAST_NOT_EXIST: &[&str] = &[".bzr", ".git", ".nols", ".svn"]; @@ -51,7 +52,7 @@ mod tests { #[test] fn test_fast_not_exist_is_sorted() { assert!(FAST_NOT_EXIST.windows(2).all(|ab| { - assert!(ab.len() >= 2); + assert!(ab.len() == 2); ab[0] < ab[1] })); }
Name