Fix ls-refs ref-prefix'ing

gix-protocol/src/handshake/mod.rs

@@ -71,6 +71,7 @@ pub(crate) mod hero {
 
     #[cfg(feature = "fetch")]
     mod fetch {
+        use crate::ls_refs::function::RefPrefixes;
         #[cfg(feature = "async-client")]
         use crate::transport::client::async_io;
         #[cfg(feature = "blocking-client")]
@@ -146,10 +147,12 @@ pub(crate) mod hero {
                     )?));
                 }
 
-                let all_refspecs = refmap_context.aggregate_refspecs();
-                let prefix_refspecs = prefix_from_spec_as_filter_on_remote.then_some(&all_refspecs[..]);
+                let prefix_refs = prefix_from_spec_as_filter_on_remote.then(|| {
+                    let all_refspecs = refmap_context.aggregate_refspecs();
+                    RefPrefixes::from_refspecs(&all_refspecs)
+                });
                 Ok(ObtainRefMap::LsRefsCommand(
-                    crate::LsRefsCommand::new(prefix_refspecs, &self.capabilities, user_agent),
+                    crate::LsRefsCommand::new(prefix_refs, &self.capabilities, user_agent),
                     refmap_context,
                 ))
             }

gix-protocol/src/ls_refs.rs

@@ -29,6 +29,9 @@ mod error {
 #[cfg(any(feature = "blocking-client", feature = "async-client"))]
 pub use error::Error;
 
+#[cfg(any(feature = "blocking-client", feature = "async-client"))]
+pub use self::function::RefPrefixes;
+
 #[cfg(any(feature = "blocking-client", feature = "async-client"))]
 pub(crate) mod function {
     use std::{borrow::Cow, collections::HashSet};
@@ -47,6 +50,68 @@ pub(crate) mod function {
         Command,
     };
 
+    /// [`RefPrefixes`] are the set of prefixes that are sent to the server for
+    /// filtering purposes.
+    ///
+    /// These are communicated by sending zero or more `ref-prefix` values, and
+    /// are documented in [gitprotocol-v2.adoc#ls-refs].
+    ///
+    /// These prefixes can be constructed from a set of [`RefSpec`]'s using
+    /// [`RefPrefixes::from_refspecs`].
+    ///
+    /// Alternatively, they can be constructed using [`RefsPrefixes::new`] and
+    /// using [`RefPrefixes::extend`] to add new prefixes. Note that any
+    /// references not starting with `refs/` will be filtered out.
+    ///
+    /// [`RefSpec`]: gix_refspec::RefSpec
+    /// [gitprotocol-v2.adoc#ls-refs]: https://github.com/git/git/blob/master/Documentation/gitprotocol-v2.adoc#ls-refs
+    pub struct RefPrefixes {
+        prefixes: HashSet<BString>,
+    }
+
+    impl RefPrefixes {
+        /// Create an empty set of [`RefPrefixes`].
+        pub fn new() -> RefPrefixes {
+            RefPrefixes {
+                prefixes: HashSet::new(),
+            }
+        }
+
+        /// Convert a series of [`RefSpec`]'s into a set of [`RefPrefixes`].
+        ///
+        /// It attempts to expand each [`RefSpec`] into prefix references, e.g.
+        /// `refs/heads/`, `refs/remotes/`, `refs/namespaces/foo/`, etc.
+        ///
+        /// [`RefSpec`]: gix_refspec::RefSpec
+        pub fn from_refspecs<'a>(refspecs: impl IntoIterator<Item = &'a gix_refspec::RefSpec>) -> Self {
+            let mut seen = HashSet::new();
+            let mut prefixes = HashSet::new();
+            for spec in refspecs.into_iter() {
+                let spec = spec.to_ref();
+                if seen.insert(spec.instruction()) {
+                    let mut out = Vec::with_capacity(1);
+                    spec.expand_prefixes(&mut out);
+                    prefixes.extend(out);
+                }
+            }
+            Self { prefixes }
+        }
+
+        fn into_args(self) -> impl Iterator<Item = BString> {
+            self.prefixes.into_iter().map(|mut prefix| {
+                prefix.insert_str(0, "ref-prefix ");
+                prefix
+            })
+        }
+    }
+
+    impl Extend<BString> for RefPrefixes {
+        fn extend<T: IntoIterator<Item = BString>>(&mut self, iter: T) {
+            self.prefixes
+                .extend(iter.into_iter().filter(|prefix| prefix.starts_with(b"refs/")));
+        }
+    }
+
     /// A command to list references from a remote Git repository.
     ///
     /// It acts as a utility to separate the invocation into the shared blocking portion,
@@ -61,7 +126,7 @@ pub(crate) mod function {
         /// Build a command to list refs from the given server `capabilities`,
         /// using `agent` information to identify ourselves.
         pub fn new(
-            prefix_refspecs: Option<&[gix_refspec::RefSpec]>,
+            prefix_refspecs: Option<RefPrefixes>,
             capabilities: &'a Capabilities,
             agent: (&'static str, Option<Cow<'static, str>>),
         ) -> Self {
@@ -78,18 +143,7 @@ pub(crate) mod function {
             }
 
             if let Some(refspecs) = prefix_refspecs {
-                let mut seen = HashSet::new();
-                for spec in refspecs {
-                    let spec = spec.to_ref();
-                    if seen.insert(spec.instruction()) {
-                        let mut prefixes = Vec::with_capacity(1);
-                        spec.expand_prefixes(&mut prefixes);
-                        for mut prefix in prefixes {
-                            prefix.insert_str(0, "ref-prefix ");
-                            arguments.push(prefix);
-                        }
-                    }
-                }
+                arguments.extend(refspecs.into_args());
             }
 
             Self {

gix-refspec/src/spec.rs

@@ -138,10 +138,17 @@ impl<'a> RefSpecRef<'a> {
         if source == "HEAD" {
             return source.into();
         }
-        let suffix = source.strip_prefix(b"refs/")?;
-        let slash_pos = suffix.find_byte(b'/')?;
-        let prefix = source[..="refs/".len() + slash_pos].as_bstr();
-        (!prefix.contains(&b'*')).then_some(prefix)
+
+        let sans_refs_prefix = source.strip_prefix(b"refs/")?;
+        if let Some(star_pos) = sans_refs_prefix.find_byte(b'*') {
+            // Disallow `*` glob star components after `refs/`
+            if star_pos == 0 {
+                return None;
+            }
+            let prefix = &source[.."refs/".len() + star_pos];
+            return (!prefix.is_empty()).then_some(prefix.as_bstr());
+        }
+        Some(source)
     }
 
     /// As opposed to [`prefix()`][Self::prefix], if the latter is `None` it will expand to all possible prefixes and place them in `out`.

gix-refspec/tests/refspec/spec.rs

@@ -18,7 +18,7 @@ mod prefix {
 
     #[test]
     fn short_absolute_refs_have_no_prefix() {
-        assert_eq!(parse("refs/short").to_ref().prefix(), None);
+        assert_eq!(parse("refs/short").to_ref().prefix().unwrap(), "refs/short");
     }
 
     #[test]
@@ -28,14 +28,14 @@ mod prefix {
                 .unwrap()
                 .prefix()
                 .unwrap(),
-            "refs/remote/"
+            "refs/remote/main"
         );
     }
 
     #[test]
     fn full_names_have_a_prefix() {
-        assert_eq!(parse("refs/heads/main").to_ref().prefix().unwrap(), "refs/heads/");
-        assert_eq!(parse("refs/foo/bar").to_ref().prefix().unwrap(), "refs/foo/");
+        assert_eq!(parse("refs/heads/main").to_ref().prefix().unwrap(), "refs/heads/main");
+        assert_eq!(parse("refs/foo/bar").to_ref().prefix().unwrap(), "refs/foo/bar");
         assert_eq!(
             parse("refs/heads/*:refs/remotes/origin/*").to_ref().prefix().unwrap(),
             "refs/heads/"
@@ -95,8 +95,8 @@ mod expand_prefixes {
 
     #[test]
     fn full_names_expand_to_their_prefix() {
-        assert_eq!(parse("refs/heads/main"), ["refs/heads/"]);
-        assert_eq!(parse("refs/foo/bar"), ["refs/foo/"]);
+        assert_eq!(parse("refs/heads/main"), ["refs/heads/main"]);
+        assert_eq!(parse("refs/foo/bar"), ["refs/foo/bar"]);
         assert_eq!(parse("refs/heads/*:refs/remotes/origin/*"), ["refs/heads/"]);
     }
 
@@ -106,7 +106,7 @@ mod expand_prefixes {
         gix_refspec::parse("refs/local/main:refs/remote/main".into(), Operation::Push)
             .unwrap()
             .expand_prefixes(&mut out);
-        assert_eq!(out, ["refs/remote/"]);
+        assert_eq!(out, ["refs/remote/main"]);
     }
 
     #[test]