fix!: validate::tag::name() now rejects names starting with slashes, and with empty components, too

It's a breaking change as it renames the `DoubleDot` variant of `tag::Error` to `RepatedDot`, while moving
a couple of variants from `reference::Error` to `tag::Error`

Author: Byron

gix-validate/src/reference.rs

@@ -11,12 +11,6 @@ pub mod name {
         Tag(#[from] crate::tag::name::Error),
         #[error("Standalone references must be all uppercased, like 'HEAD'")]
         SomeLowercase,
-        #[error("A reference name must not start with a slash '/'")]
-        StartsWithSlash,
-        #[error("Multiple slashes in a row are not allowed as they may change the reference's meaning")]
-        RepeatedSlash,
-        #[error("Path components must not start with '.'")]
-        StartsWithDot,
     }
 
     impl From<Infallible> for Error {
@@ -75,34 +69,10 @@ fn validate(path: &BStr, mode: Mode) -> Result<Cow<'_, BStr>, name::Error> {
         },
     )?;
     let sanitize = matches!(mode, Mode::PartialSanitize);
-    if path.get(0) == Some(&b'/') {
-        if sanitize {
-            out.to_mut()[0] = b'-';
-        } else {
-            return Err(name::Error::StartsWithSlash);
-        }
-    }
     let mut previous = 0;
     let mut saw_slash = false;
-    let mut out_ofs = 0;
-    for (mut byte_pos, byte) in path.iter().enumerate() {
-        byte_pos -= out_ofs;
+    for (byte_pos, byte) in path.iter().enumerate() {
         match *byte {
-            b'/' if previous == b'/' => {
-                if sanitize {
-                    out.to_mut().remove(byte_pos);
-                    out_ofs += 1;
-                } else {
-                    return Err(name::Error::RepeatedSlash);
-                }
-            }
-            b'.' if previous == b'/' => {
-                if sanitize {
-                    out.to_mut()[byte_pos] = b'-';
-                } else {
-                    return Err(name::Error::StartsWithDot);
-                }
-            }
             _ => {}
         }
 

gix-validate/src/tag.rs

@@ -12,8 +12,12 @@ pub mod name {
     pub enum Error {
         #[error("A ref must not contain invalid bytes or ascii control characters: {byte:?}")]
         InvalidByte { byte: BString },
+        #[error("A reference name must not start with a slash '/'")]
+        StartsWithSlash,
+        #[error("Multiple slashes in a row are not allowed as they may change the reference's meaning")]
+        RepeatedSlash,
         #[error("A ref must not contain '..' as it may be mistaken for a range")]
-        DoubleDot,
+        RepeatedDot,
         #[error("A ref must not end with '.lock'")]
         LockFileSuffix,
         #[error("A ref must not contain '@{{' which is a part of a ref-log")]
@@ -72,6 +76,13 @@ pub(crate) fn name_inner(input: &BStr, mode: Mode) -> Result<Cow<'_, BStr>, name
             return Err(name::Error::EndsWithSlash);
         }
     }
+    if input.get(0) == Some(&b'/') {
+        if sanitize {
+            out.to_mut()[0] = b'-';
+        } else {
+            return Err(name::Error::StartsWithSlash);
+        }
+    }
 
     let mut previous = 0;
     let mut out_ofs = 0;
@@ -100,7 +111,14 @@ pub(crate) fn name_inner(input: &BStr, mode: Mode) -> Result<Cow<'_, BStr>, name
                     out.to_mut().remove(byte_pos);
                     out_ofs += 1;
                 } else {
-                    return Err(name::Error::DoubleDot);
+                    return Err(name::Error::RepeatedDot);
+                }
+            }
+            b'.' if previous == b'/' => {
+                if sanitize {
+                    out.to_mut()[byte_pos] = b'-';
+                } else {
+                    return Err(name::Error::StartsWithDot);
                 }
             }
             b'{' if previous == b'@' => {
@@ -110,6 +128,21 @@ pub(crate) fn name_inner(input: &BStr, mode: Mode) -> Result<Cow<'_, BStr>, name
                     return Err(name::Error::ReflogPortion);
                 }
             }
+            b'/' if previous == b'/' => {
+                if sanitize {
+                    out.to_mut().remove(byte_pos);
+                    out_ofs += 1;
+                } else {
+                    return Err(name::Error::RepeatedSlash);
+                }
+            }
+            b'.' if previous == b'/' => {
+                if sanitize {
+                    out.to_mut()[byte_pos] = b'-';
+                } else {
+                    return Err(name::Error::StartsWithDot);
+                }
+            }
             _ => {}
         }
         previous = *byte;

gix-validate/tests/reference/mod.rs

@@ -94,7 +94,7 @@ mod name_partial {
         mktest!(
             refs_path_double_dot,
             b"refs/../somewhere",
-            RefError::Tag(TagError::DoubleDot)
+            RefError::Tag(TagError::StartsWithDot)
         );
         mktests!(refs_path_double_dot_san, b"refs/../somewhere", "refs/-/somewhere");
         mktest!(
@@ -105,7 +105,7 @@ mod name_partial {
         mktest!(
             refs_path_name_starts_with_multi_dot,
             b"..refs/somewhere",
-            RefError::Tag(TagError::DoubleDot)
+            RefError::Tag(TagError::RepeatedDot)
         );
         mktests!(
             refs_path_name_starts_with_multi_dot_san,
@@ -120,18 +120,26 @@ mod name_partial {
         mktest!(
             refs_path_component_is_singular_dot,
             b"refs/./still-inside-but-not-cool",
-            RefError::StartsWithDot
+            RefError::Tag(TagError::StartsWithDot)
         );
         mktests!(
             refs_path_component_is_singular_dot_san,
             b"refs/./still-inside-but-not-cool",
             "refs/-/still-inside-but-not-cool"
         );
-        mktest!(any_path_starts_with_slash, b"/etc/foo", RefError::StartsWithSlash);
+        mktest!(
+            any_path_starts_with_slash,
+            b"/etc/foo",
+            RefError::Tag(TagError::StartsWithSlash)
+        );
         mktests!(any_path_starts_with_slash_san, b"/etc/foo", "-etc/foo");
         mktest!(empty_path, b"", RefError::Tag(TagError::Empty));
         mktests!(empty_path_san, b"", "-");
-        mktest!(refs_starts_with_slash, b"/refs/heads/main", RefError::StartsWithSlash);
+        mktest!(
+            refs_starts_with_slash,
+            b"/refs/heads/main",
+            RefError::Tag(TagError::StartsWithSlash)
+        );
         mktests!(refs_starts_with_slash_san, b"/refs/heads/main", "-refs/heads/main");
         mktest!(
             ends_with_slash,
@@ -142,12 +150,12 @@ mod name_partial {
         mktest!(
             path_with_duplicate_slashes,
             b"refs//heads/main",
-            RefError::RepeatedSlash
+            RefError::Tag(TagError::RepeatedSlash)
         );
         mktests!(path_with_duplicate_slashes_san, b"refs//heads/main", "refs/heads/main");
         mktest!(
             path_with_spaces,
-            b"refs//heads/name with spaces",
+            b"refs/heads/name with spaces",
             RefError::Tag(TagError::InvalidByte { .. })
         );
         mktests!(
@@ -255,7 +263,7 @@ mod name {
         mktest!(
             refs_path_double_dot,
             b"refs/../somewhere",
-            RefError::Tag(TagError::DoubleDot)
+            RefError::Tag(TagError::StartsWithDot)
         );
         mktests!(refs_path_double_dot_san, b"refs/../somewhere", "refs/-/somewhere");
         mktest!(refs_name_special_case_upload_pack, b"(null)", RefError::SomeLowercase);
@@ -274,7 +282,7 @@ mod name {
         mktest!(
             refs_path_name_starts_with_dot_in_name,
             b"refs/.somewhere",
-            RefError::StartsWithDot
+            RefError::Tag(TagError::StartsWithDot)
         );
         mktests!(
             refs_path_name_starts_with_dot_in_name_san,
@@ -294,7 +302,7 @@ mod name {
         mktest!(
             refs_path_component_is_singular_dot,
             b"refs/./still-inside-but-not-cool",
-            RefError::StartsWithDot
+            RefError::Tag(TagError::StartsWithDot)
         );
         mktests!(
             refs_path_component_is_singular_dot_an,
@@ -305,11 +313,19 @@ mod name {
         mktests!(capitalized_name_without_path_san, b"Main", "Main");
         mktest!(lowercase_name_without_path, b"main", RefError::SomeLowercase);
         mktests!(lowercase_name_without_path_san, b"main", "main");
-        mktest!(any_path_starts_with_slash, b"/etc/foo", RefError::StartsWithSlash);
+        mktest!(
+            any_path_starts_with_slash,
+            b"/etc/foo",
+            RefError::Tag(TagError::StartsWithSlash)
+        );
         mktests!(any_path_starts_with_slash_san, b"/etc/foo", "-etc/foo");
         mktest!(empty_path, b"", RefError::Tag(TagError::Empty));
         mktests!(empty_path_san, b"", "-");
-        mktest!(refs_starts_with_slash, b"/refs/heads/main", RefError::StartsWithSlash);
+        mktest!(
+            refs_starts_with_slash,
+            b"/refs/heads/main",
+            RefError::Tag(TagError::StartsWithSlash)
+        );
         mktests!(refs_starts_with_slash_san, b"/refs/heads/main", "-refs/heads/main");
         mktest!(
             ends_with_slash,
@@ -330,7 +346,7 @@ mod name {
         mktest!(
             a_path_with_duplicate_slashes,
             b"refs//heads/main",
-            RefError::RepeatedSlash
+            RefError::Tag(TagError::RepeatedSlash)
         );
         mktests!(
             a_path_with_duplicate_slashes_san,

gix-validate/tests/tag/mod.rs

@@ -77,24 +77,28 @@ mod name {
             "this_looks_like_a_@-reflog}"
         );
         mktest!(suffix_is_dot_lock, b"prefix.lock", LockFileSuffix);
-        mktest!(too_many_dots, b"......", DoubleDot);
+        mktest!(too_many_dots, b"......", RepeatedDot);
         mktests!(too_many_dots_san, b"......", "-");
         mktests!(too_many_dots_and_slashes_san, b"//....///....///", "-");
         mktests!(suffix_is_dot_lock_san, b"prefix.lock", "prefix");
         mktest!(suffix_is_dot_lock_multiple, b"prefix.lock.lock", LockFileSuffix);
         mktests!(suffix_is_dot_lock_multiple_san, b"prefix.lock.lock", "prefix");
         mktest!(ends_with_slash, b"prefix/", EndsWithSlash);
+        mktest!(empty_component, b"prefix//suffix", RepeatedSlash);
+        mktests!(empty_component_san, b"prefix//suffix", "prefix/suffix");
         mktests!(ends_with_slash_san, b"prefix/", "prefix");
         mktest!(is_dot_lock, b".lock", StartsWithDot);
         mktests!(is_dot_lock_san, b".lock", "-lock");
-        mktest!(contains_double_dot, b"with..double-dot", DoubleDot);
+        mktest!(contains_double_dot, b"with..double-dot", RepeatedDot);
         mktests!(contains_double_dot_san, b"with..double-dot", "with.double-dot");
-        mktest!(starts_with_double_dot, b"..with-double-dot", DoubleDot);
+        mktest!(starts_with_double_dot, b"..with-double-dot", RepeatedDot);
         mktests!(starts_with_double_dot_san, b"..with-double-dot", "-with-double-dot");
-        mktest!(ends_with_double_dot, b"with-double-dot..", DoubleDot);
+        mktest!(ends_with_double_dot, b"with-double-dot..", RepeatedDot);
         mktests!(ends_with_double_dot_san, b"with-double-dot..", "with-double-dot-");
         mktest!(starts_with_asterisk, b"*suffix", Asterisk);
         mktests!(starts_with_asterisk_san, b"*suffix", "-suffix");
+        mktest!(starts_with_slash, b"/suffix", StartsWithSlash);
+        mktests!(starts_with_slash_san, b"/suffix", "-suffix");
         mktest!(ends_with_asterisk, b"prefix*", Asterisk);
         mktests!(ends_with_asterisk_san, b"prefix*", "prefix-");
         mktest!(contains_asterisk, b"prefix*suffix", Asterisk);