Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 20 additions & 11 deletions gix-refspec/src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,11 @@ pub(crate) mod function {
*spec = "HEAD".into();
}
}
let (src, src_had_pattern) = validated(src, operation == Operation::Push && dst.is_some())?;
let (dst, dst_had_pattern) = validated(dst, false)?;
if mode != Mode::Negative && src_had_pattern != dst_had_pattern {
let is_one_sided = dst.is_none();
let (src, src_had_pattern) = validated(src, operation == Operation::Push && dst.is_some(), is_one_sided)?;
let (dst, dst_had_pattern) = validated(dst, false, false)?;
// For one-sided refspecs, we don't need to check for pattern balance
if !is_one_sided && mode != Mode::Negative && src_had_pattern != dst_had_pattern {
return Err(Error::PatternUnbalanced);
}

Expand Down Expand Up @@ -149,20 +151,27 @@ pub(crate) mod function {
spec.len() >= gix_hash::Kind::shortest().len_in_hex() && spec.iter().all(u8::is_ascii_hexdigit)
}

fn validated(spec: Option<&BStr>, allow_revspecs: bool) -> Result<(Option<&BStr>, bool), Error> {
fn validated(spec: Option<&BStr>, allow_revspecs: bool, is_one_sided: bool) -> Result<(Option<&BStr>, bool), Error> {
match spec {
Some(spec) => {
let glob_count = spec.iter().filter(|b| **b == b'*').take(2).count();
if glob_count > 1 {
return Err(Error::PatternUnsupported { pattern: spec.into() });
// For one-sided refspecs, allow any number of globs without validation
if !is_one_sided {
return Err(Error::PatternUnsupported { pattern: spec.into() });
}
}
let has_globs = glob_count == 1;
// Check if there are any globs (one or more asterisks)
let has_globs = glob_count > 0;
if has_globs {
let mut buf = smallvec::SmallVec::<[u8; 256]>::with_capacity(spec.len());
buf.extend_from_slice(spec);
let glob_pos = buf.find_byte(b'*').expect("glob present");
buf[glob_pos] = b'a';
gix_validate::reference::name_partial(buf.as_bstr())?;
// For one-sided refspecs, skip validation of glob patterns
if !is_one_sided {
let mut buf = smallvec::SmallVec::<[u8; 256]>::with_capacity(spec.len());
buf.extend_from_slice(spec);
let glob_pos = buf.find_byte(b'*').expect("glob present");
buf[glob_pos] = b'a';
gix_validate::reference::name_partial(buf.as_bstr())?;
}
} else {
gix_validate::reference::name_partial(spec)
.map_err(Error::from)
Expand Down
99 changes: 99 additions & 0 deletions gix-refspec/tests/refspec/match_group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,3 +184,102 @@ mod multiple {
);
}
}

mod complex_globs {
use gix_refspec::{parse::Operation, MatchGroup};
use gix_hash::ObjectId;
use bstr::BString;
use std::borrow::Cow;

#[test]
fn one_sided_complex_glob_patterns_can_be_parsed() {
// The key change is that complex glob patterns with multiple asterisks
// can now be parsed for one-sided refspecs
let spec1 = gix_refspec::parse("refs/*/foo/*".into(), Operation::Fetch);
assert!(spec1.is_ok(), "Should parse complex glob pattern for one-sided refspec");

let spec2 = gix_refspec::parse("refs/*/*/bar".into(), Operation::Fetch);
assert!(spec2.is_ok(), "Should parse complex glob pattern with multiple asterisks");

let spec3 = gix_refspec::parse("refs/heads/*/release/*".into(), Operation::Fetch);
assert!(spec3.is_ok(), "Should parse complex glob pattern");

// Two-sided refspecs with multiple asterisks should still fail
let spec4 = gix_refspec::parse("refs/*/foo/*:refs/remotes/*".into(), Operation::Fetch);
assert!(spec4.is_err(), "Two-sided refspecs with multiple asterisks should fail");
}

#[test]
fn one_sided_simple_glob_patterns_match() {
// Test that simple glob patterns (one asterisk) work correctly with matching
let refs = vec![
create_ref("refs/heads/feature/foo", "1111111111111111111111111111111111111111"),
create_ref("refs/heads/bugfix/bar", "2222222222222222222222222222222222222222"),
create_ref("refs/tags/v1.0", "3333333333333333333333333333333333333333"),
create_ref("refs/pull/123", "4444444444444444444444444444444444444444"),
];
let items: Vec<_> = refs.iter().map(|r| r.to_item()).collect();

// Test: refs/heads/* should match all refs under refs/heads/
let spec = gix_refspec::parse("refs/heads/*".into(), Operation::Fetch).unwrap();
let group = MatchGroup::from_fetch_specs([spec]);
let outcome = group.match_lhs(items.iter().copied());
let mappings = outcome.mappings;

assert_eq!(mappings.len(), 2, "Should match two refs under refs/heads/");

// Test: refs/tags/* should match all refs under refs/tags/
let items2: Vec<_> = refs.iter().map(|r| r.to_item()).collect();
let spec2 = gix_refspec::parse("refs/tags/*".into(), Operation::Fetch).unwrap();
let group2 = MatchGroup::from_fetch_specs([spec2]);
let outcome2 = group2.match_lhs(items2.iter().copied());
let mappings2 = outcome2.mappings;

assert_eq!(mappings2.len(), 1, "Should match one ref under refs/tags/");
}

#[test]
fn one_sided_glob_with_suffix_matches() {
// Test that glob patterns with suffix work correctly
let refs = vec![
create_ref("refs/heads/feature", "1111111111111111111111111111111111111111"),
create_ref("refs/heads/feat", "2222222222222222222222222222222222222222"),
create_ref("refs/heads/main", "3333333333333333333333333333333333333333"),
];
let items: Vec<_> = refs.iter().map(|r| r.to_item()).collect();

// Test: refs/heads/feat* should match refs/heads/feature and refs/heads/feat
let spec = gix_refspec::parse("refs/heads/feat*".into(), Operation::Fetch).unwrap();
let group = MatchGroup::from_fetch_specs([spec]);
let outcome = group.match_lhs(items.iter().copied());
let mappings = outcome.mappings;

assert_eq!(mappings.len(), 2, "Should match two refs starting with feat");
}

// Helper function to create a ref
fn create_ref(name: &str, id_hex: &str) -> Ref {
Ref {
name: name.into(),
target: ObjectId::from_hex(id_hex.as_bytes()).unwrap(),
object: None,
}
}

#[derive(Debug, Clone)]
struct Ref {
name: BString,
target: ObjectId,
object: Option<ObjectId>,
}

impl Ref {
fn to_item(&self) -> gix_refspec::match_group::Item<'_> {
gix_refspec::match_group::Item {
full_ref_name: self.name.as_ref(),
target: &self.target,
object: self.object.as_deref(),
}
}
}
}
41 changes: 41 additions & 0 deletions gix-refspec/tests/refspec/parse/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,3 +174,44 @@ fn ampersand_on_left_hand_side_is_head() {
fn empty_refspec_is_enough_for_fetching_head_into_fetchhead() {
assert_parse("", Instruction::Fetch(Fetch::Only { src: b("HEAD") }));
}

#[test]
fn complex_glob_patterns_are_allowed_in_one_sided_refspecs() {
// Complex patterns with multiple asterisks should work for one-sided refspecs
assert_parse(
"refs/*/foo/*",
Instruction::Fetch(Fetch::Only {
src: b("refs/*/foo/*"),
}),
);

assert_parse(
"+refs/heads/*/release/*",
Instruction::Fetch(Fetch::Only {
src: b("refs/heads/*/release/*"),
}),
);

// Even more complex patterns
assert_parse(
"refs/*/*/branch",
Instruction::Fetch(Fetch::Only {
src: b("refs/*/*/branch"),
}),
);
}

#[test]
fn complex_glob_patterns_still_fail_for_two_sided_refspecs() {
// Two-sided refspecs with complex patterns (multiple asterisks) should still fail
for spec in [
"refs/*/foo/*:refs/remotes/origin/*",
"refs/*/*:refs/remotes/*",
"a/*/c/*:b/*",
] {
assert!(matches!(
try_parse(spec, Operation::Fetch).unwrap_err(),
Error::PatternUnsupported { .. }
));
}
}
20 changes: 17 additions & 3 deletions gix-refspec/tests/refspec/parse/invalid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,31 +27,45 @@ fn whitespace() {

#[test]
fn complex_patterns_with_more_than_one_asterisk() {
// For one-sided refspecs, complex patterns are now allowed
for op in [Operation::Fetch, Operation::Push] {
for spec in ["a/*/c/*", "a**:**b", "+:**/"] {
assert!(try_parse("a/*/c/*", op).is_ok());
}

// For two-sided refspecs, complex patterns should still fail
for op in [Operation::Fetch, Operation::Push] {
for spec in ["a/*/c/*:x/*/y/*", "a**:**b", "+:**/"] {
assert!(matches!(
try_parse(spec, op).unwrap_err(),
Error::PatternUnsupported { .. }
));
}
}

// Negative specs with multiple patterns still fail
assert!(matches!(
try_parse("^*/*", Operation::Fetch).unwrap_err(),
Error::PatternUnsupported { .. }
Error::NegativeGlobPattern
));
}

#[test]
fn both_sides_need_pattern_if_one_uses_it() {
// For two-sided refspecs, both sides still need patterns if one uses it
for op in [Operation::Fetch, Operation::Push] {
for spec in ["refs/*/a", ":a/*", "+:a/*", "a*:b/c", "a:b/*"] {
for spec in [":a/*", "+:a/*", "a*:b/c", "a:b/*"] {
assert!(
matches!(try_parse(spec, op).unwrap_err(), Error::PatternUnbalanced),
"{}",
spec
);
}
}

// One-sided refspecs with patterns are now allowed
for op in [Operation::Fetch, Operation::Push] {
assert!(try_parse("refs/*/a", op).is_ok());
}
}

#[test]
Expand Down
9 changes: 9 additions & 0 deletions gix-refspec/tests/refspec/parse/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ fn baseline() {
),
true,
) => {} // we prefer failing fast, git let's it pass
// We now allow complex glob patterns in one-sided refspecs
(None, false) if is_one_sided_glob_pattern(spec, op) => {
// This is an intentional behavior change: we allow complex globs in one-sided refspecs
}
_ => {
eprintln!("{err_code} {res:?} {} {:?}", kind.as_bstr(), spec.as_bstr());
mismatch += 1;
Expand All @@ -66,6 +70,11 @@ fn baseline() {
panics
);
}

fn is_one_sided_glob_pattern(spec: &[u8], op: Operation) -> bool {
use bstr::ByteSlice;
matches!(op, Operation::Fetch) && spec.to_str().map(|s| s.contains('*') && !s.contains(':')).unwrap_or(false)
}
}

#[test]
Expand Down