repo_path: reject leading slash and empty path components

Leading/trailing slashes would introduce a bit of complexity if we migrate
the backing type from Vec<String> to String. Empty components are okay, but
let's reject them as they are cryptic and invalid.
This commit is contained in:
Yuya Nishihara 2023-11-26 07:34:54 +09:00
parent 755af75c30
commit e14b31a033

View file

@ -43,7 +43,7 @@ impl From<&str> for RepoPathComponent {
impl From<String> for RepoPathComponent { impl From<String> for RepoPathComponent {
fn from(value: String) -> Self { fn from(value: String) -> Self {
assert!(!value.contains('/')); assert!(is_valid_repo_path_component_str(&value));
RepoPathComponent { value } RepoPathComponent { value }
} }
} }
@ -64,8 +64,12 @@ impl RepoPath {
RepoPath { components: vec![] } RepoPath { components: vec![] }
} }
/// Creates `RepoPath` from valid string representation.
///
/// The input `value` must not contain empty path components. For example,
/// `"/"`, `"/foo"`, `"foo/"`, `"foo//bar"` are all invalid.
pub fn from_internal_string(value: &str) -> Self { pub fn from_internal_string(value: &str) -> Self {
assert!(!value.ends_with('/')); assert!(is_valid_repo_path_str(value));
if value.is_empty() { if value.is_empty() {
RepoPath::root() RepoPath::root()
} else { } else {
@ -193,8 +197,18 @@ pub enum FsPathParseError {
InputNotInRepo(PathBuf), InputNotInRepo(PathBuf),
} }
fn is_valid_repo_path_component_str(value: &str) -> bool {
!value.is_empty() && !value.contains('/')
}
fn is_valid_repo_path_str(value: &str) -> bool {
!value.starts_with('/') && !value.ends_with('/') && !value.contains("//")
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use std::panic;
use super::*; use super::*;
fn repo_path(value: &str) -> RepoPath { fn repo_path(value: &str) -> RepoPath {
@ -208,6 +222,15 @@ mod tests {
assert!(!repo_path("foo").is_root()); assert!(!repo_path("foo").is_root());
} }
#[test]
fn test_from_internal_string() {
assert_eq!(repo_path(""), RepoPath::root());
assert!(panic::catch_unwind(|| repo_path("/")).is_err());
assert!(panic::catch_unwind(|| repo_path("/x")).is_err());
assert!(panic::catch_unwind(|| repo_path("x/")).is_err());
assert!(panic::catch_unwind(|| repo_path("x//y")).is_err());
}
#[test] #[test]
fn test_to_internal_string() { fn test_to_internal_string() {
assert_eq!(RepoPath::root().to_internal_file_string(), ""); assert_eq!(RepoPath::root().to_internal_file_string(), "");