From e14b31a033430a23238fd65a7facc01d91d453b0 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Sun, 26 Nov 2023 07:34:54 +0900
Subject: [PATCH] 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.
---
 lib/src/repo_path.rs | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/lib/src/repo_path.rs b/lib/src/repo_path.rs
index 8d4c82500..c62dbc979 100644
--- a/lib/src/repo_path.rs
+++ b/lib/src/repo_path.rs
@@ -43,7 +43,7 @@ impl From<&str> for RepoPathComponent {
 
 impl From<String> for RepoPathComponent {
     fn from(value: String) -> Self {
-        assert!(!value.contains('/'));
+        assert!(is_valid_repo_path_component_str(&value));
         RepoPathComponent { value }
     }
 }
@@ -64,8 +64,12 @@ impl RepoPath {
         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 {
-        assert!(!value.ends_with('/'));
+        assert!(is_valid_repo_path_str(value));
         if value.is_empty() {
             RepoPath::root()
         } else {
@@ -193,8 +197,18 @@ pub enum FsPathParseError {
     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)]
 mod tests {
+    use std::panic;
+
     use super::*;
 
     fn repo_path(value: &str) -> RepoPath {
@@ -208,6 +222,15 @@ mod tests {
         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]
     fn test_to_internal_string() {
         assert_eq!(RepoPath::root().to_internal_file_string(), "");