Skip to content

Commit 9683186

Browse files
committed
Use additive approach to avoid issues around removal of indices
1 parent 13c7922 commit 9683186

File tree

4 files changed

+70
-95
lines changed

4 files changed

+70
-95
lines changed

gix-validate/src/reference.rs

Lines changed: 15 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use bstr::{BStr, BString, ByteSlice};
2+
13
///
24
#[allow(clippy::empty_docs)]
35
pub mod name {
@@ -20,15 +22,12 @@ pub mod name {
2022
}
2123
}
2224

23-
use bstr::BStr;
24-
use std::borrow::Cow;
25-
2625
/// Validate a reference name running all the tests in the book. This disallows lower-case references like `lower`, but also allows
2726
/// ones like `HEAD`, and `refs/lower`.
2827
pub fn name(path: &BStr) -> Result<&BStr, name::Error> {
2928
match validate(path, Mode::Complete)? {
30-
Cow::Borrowed(inner) => Ok(inner),
31-
Cow::Owned(_) => {
29+
None => Ok(path),
30+
Some(_) => {
3231
unreachable!("Without sanitization, there is no chance a sanitized version is returned.")
3332
}
3433
}
@@ -38,8 +37,8 @@ pub fn name(path: &BStr) -> Result<&BStr, name::Error> {
3837
/// even though these would be disallowed with when using [`name()`].
3938
pub fn name_partial(path: &BStr) -> Result<&BStr, name::Error> {
4039
match validate(path, Mode::Partial)? {
41-
Cow::Borrowed(inner) => Ok(inner),
42-
Cow::Owned(_) => {
40+
None => Ok(path),
41+
Some(_) => {
4342
unreachable!("Without sanitization, there is no chance a sanitized version is returned.")
4443
}
4544
}
@@ -49,8 +48,10 @@ pub fn name_partial(path: &BStr) -> Result<&BStr, name::Error> {
4948
/// partial name, which would also pass [`name_partial()`].
5049
///
5150
/// Note that an empty `path` is replaced with a `-` in order to be valid.
52-
pub fn name_partial_or_sanitize(path: &BStr) -> Cow<'_, BStr> {
53-
validate(path, Mode::PartialSanitize).expect("BUG: errors cannot happen as any issue is fixed instantly")
51+
pub fn name_partial_or_sanitize(path: &BStr) -> BString {
52+
validate(path, Mode::PartialSanitize)
53+
.expect("BUG: errors cannot happen as any issue is fixed instantly")
54+
.expect("we always rebuild the path")
5455
}
5556

5657
enum Mode {
@@ -60,30 +61,18 @@ enum Mode {
6061
PartialSanitize,
6162
}
6263

63-
fn validate(path: &BStr, mode: Mode) -> Result<Cow<'_, BStr>, name::Error> {
64-
let mut out = crate::tag::name_inner(
64+
fn validate(path: &BStr, mode: Mode) -> Result<Option<BString>, name::Error> {
65+
let out = crate::tag::name_inner(
6566
path,
6667
match mode {
6768
Mode::Complete | Mode::Partial => crate::tag::Mode::Validate,
6869
Mode::PartialSanitize => crate::tag::Mode::Sanitize,
6970
},
7071
)?;
71-
let sanitize = matches!(mode, Mode::PartialSanitize);
72-
let mut previous = 0;
73-
let mut saw_slash = false;
74-
for (byte_pos, byte) in path.iter().enumerate() {
75-
match *byte {
76-
_ => {}
77-
}
78-
79-
if *byte == b'/' {
80-
saw_slash = true;
81-
}
82-
previous = *byte;
83-
}
84-
8572
if let Mode::Complete = mode {
86-
if !saw_slash && !path.iter().all(|c| c.is_ascii_uppercase() || *c == b'_') {
73+
let input = out.as_ref().map_or(path, |b| b.as_bstr());
74+
let saw_slash = input.find_byte(b'/').is_some();
75+
if !saw_slash && !input.iter().all(|c| c.is_ascii_uppercase() || *c == b'_') {
8776
return Err(name::Error::SomeLowercase);
8877
}
8978
}

gix-validate/src/tag.rs

Lines changed: 46 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
use bstr::{BStr, ByteSlice};
2-
use std::borrow::Cow;
1+
use bstr::{BStr, BString};
32

43
///
54
#[allow(clippy::empty_docs)]
@@ -39,8 +38,8 @@ pub mod name {
3938
/// Tag names are provided as names, lik` v1.0` or `alpha-1`, without paths.
4039
pub fn name(input: &BStr) -> Result<&BStr, name::Error> {
4140
match name_inner(input, Mode::Validate)? {
42-
Cow::Borrowed(inner) => Ok(inner),
43-
Cow::Owned(_) => {
41+
None => Ok(input),
42+
Some(_) => {
4443
unreachable!("When validating, the input isn't changed")
4544
}
4645
}
@@ -52,121 +51,112 @@ pub(crate) enum Mode {
5251
Validate,
5352
}
5453

55-
pub(crate) fn name_inner(input: &BStr, mode: Mode) -> Result<Cow<'_, BStr>, name::Error> {
56-
let mut out = Cow::Borrowed(input);
57-
let sanitize = matches!(mode, Mode::Sanitize);
54+
pub(crate) fn name_inner(input: &BStr, mode: Mode) -> Result<Option<BString>, name::Error> {
55+
let mut out: Option<BString> =
56+
matches!(mode, Mode::Sanitize).then(|| BString::from(Vec::with_capacity(input.len())));
5857
if input.is_empty() {
59-
return if sanitize {
60-
out.to_mut().push(b'-');
61-
Ok(out)
58+
return if let Some(mut out) = out {
59+
out.push(b'-');
60+
Ok(Some(out))
6261
} else {
6362
Err(name::Error::Empty)
6463
};
6564
}
66-
if *input.last().expect("non-empty") == b'/' {
67-
if sanitize {
68-
while out.last() == Some(&b'/') {
69-
out.to_mut().pop();
70-
}
71-
let bytes_from_end = out.to_mut().as_bytes_mut().iter_mut().rev();
72-
for b in bytes_from_end.take_while(|b| **b == b'/') {
73-
*b = b'-';
74-
}
75-
} else {
76-
return Err(name::Error::EndsWithSlash);
77-
}
65+
if *input.last().expect("non-empty") == b'/' && out.is_none() {
66+
return Err(name::Error::EndsWithSlash);
7867
}
79-
if input.get(0) == Some(&b'/') {
80-
if sanitize {
81-
out.to_mut()[0] = b'-';
82-
} else {
83-
return Err(name::Error::StartsWithSlash);
84-
}
68+
if input.first() == Some(&b'/') && out.is_none() {
69+
return Err(name::Error::StartsWithSlash);
8570
}
8671

8772
let mut previous = 0;
88-
let mut out_ofs = 0;
89-
for (mut byte_pos, byte) in input.iter().enumerate() {
90-
byte_pos -= out_ofs;
73+
for byte in input.iter() {
9174
match byte {
9275
b'\\' | b'^' | b':' | b'[' | b'?' | b' ' | b'~' | b'\0'..=b'\x1F' | b'\x7F' => {
93-
if sanitize {
94-
out.to_mut()[byte_pos] = b'-';
76+
if let Some(out) = out.as_mut() {
77+
out.push(b'-');
9578
} else {
9679
return Err(name::Error::InvalidByte {
9780
byte: (&[*byte][..]).into(),
9881
});
9982
}
10083
}
10184
b'*' => {
102-
if sanitize {
103-
out.to_mut()[byte_pos] = b'-';
85+
if let Some(out) = out.as_mut() {
86+
out.push(b'-');
10487
} else {
10588
return Err(name::Error::Asterisk);
10689
}
10790
}
10891

10992
b'.' if previous == b'.' => {
110-
if sanitize {
111-
out.to_mut().remove(byte_pos);
112-
out_ofs += 1;
113-
} else {
93+
if out.is_none() {
11494
return Err(name::Error::RepeatedDot);
11595
}
11696
}
11797
b'.' if previous == b'/' => {
118-
if sanitize {
119-
out.to_mut()[byte_pos] = b'-';
98+
if let Some(out) = out.as_mut() {
99+
out.push(b'-');
120100
} else {
121101
return Err(name::Error::StartsWithDot);
122102
}
123103
}
124104
b'{' if previous == b'@' => {
125-
if sanitize {
126-
out.to_mut()[byte_pos] = b'-';
105+
if let Some(out) = out.as_mut() {
106+
out.push(b'-');
127107
} else {
128108
return Err(name::Error::ReflogPortion);
129109
}
130110
}
131111
b'/' if previous == b'/' => {
132-
if sanitize {
133-
out.to_mut().remove(byte_pos);
134-
out_ofs += 1;
135-
} else {
112+
if out.is_none() {
136113
return Err(name::Error::RepeatedSlash);
137114
}
138115
}
139116
b'.' if previous == b'/' => {
140-
if sanitize {
141-
out.to_mut()[byte_pos] = b'-';
117+
if let Some(out) = out.as_mut() {
118+
out.push(b'-');
142119
} else {
143120
return Err(name::Error::StartsWithDot);
144121
}
145122
}
146-
_ => {}
123+
c => {
124+
if let Some(out) = out.as_mut() {
125+
out.push(*c)
126+
}
127+
}
147128
}
148129
previous = *byte;
149130
}
131+
132+
if let Some(out) = out.as_mut() {
133+
while out.last() == Some(&b'/') {
134+
out.pop();
135+
}
136+
while out.first() == Some(&b'/') {
137+
out.remove(0);
138+
}
139+
}
150140
if input[0] == b'.' {
151-
if sanitize {
152-
out.to_mut()[0] = b'-';
141+
if let Some(out) = out.as_mut() {
142+
out[0] = b'-';
153143
} else {
154144
return Err(name::Error::StartsWithDot);
155145
}
156146
}
157147
if input[input.len() - 1] == b'.' {
158-
if sanitize {
148+
if let Some(out) = out.as_mut() {
159149
let last = out.len() - 1;
160-
out.to_mut()[last] = b'-';
150+
out[last] = b'-';
161151
} else {
162152
return Err(name::Error::EndsWithDot);
163153
}
164154
}
165155
if input.ends_with(b".lock") {
166-
if sanitize {
156+
if let Some(out) = out.as_mut() {
167157
while out.ends_with(b".lock") {
168158
let len_without_suffix = out.len() - b".lock".len();
169-
out.to_mut().truncate(len_without_suffix);
159+
out.truncate(len_without_suffix);
170160
}
171161
} else {
172162
return Err(name::Error::LockFileSuffix);

gix-validate/tests/reference/mod.rs

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ macro_rules! mktests {
33
#[test]
44
fn $name() {
55
let actual = gix_validate::reference::name_partial_or_sanitize($input.as_bstr());
6-
assert_eq!(actual.as_ref(), $expected);
6+
assert_eq!(actual, $expected);
77
assert!(gix_validate::reference::name_partial(actual.as_ref()).is_ok());
88
}
99
};
@@ -132,15 +132,15 @@ mod name_partial {
132132
b"/etc/foo",
133133
RefError::Tag(TagError::StartsWithSlash)
134134
);
135-
mktests!(any_path_starts_with_slash_san, b"/etc/foo", "-etc/foo");
135+
mktests!(any_path_starts_with_slash_san, b"/etc/foo", "etc/foo");
136136
mktest!(empty_path, b"", RefError::Tag(TagError::Empty));
137137
mktests!(empty_path_san, b"", "-");
138138
mktest!(
139139
refs_starts_with_slash,
140140
b"/refs/heads/main",
141141
RefError::Tag(TagError::StartsWithSlash)
142142
);
143-
mktests!(refs_starts_with_slash_san, b"/refs/heads/main", "-refs/heads/main");
143+
mktests!(refs_starts_with_slash_san, b"/refs/heads/main", "refs/heads/main");
144144
mktest!(
145145
ends_with_slash,
146146
b"refs/heads/main/",
@@ -318,15 +318,15 @@ mod name {
318318
b"/etc/foo",
319319
RefError::Tag(TagError::StartsWithSlash)
320320
);
321-
mktests!(any_path_starts_with_slash_san, b"/etc/foo", "-etc/foo");
321+
mktests!(any_path_starts_with_slash_san, b"/etc/foo", "etc/foo");
322322
mktest!(empty_path, b"", RefError::Tag(TagError::Empty));
323323
mktests!(empty_path_san, b"", "-");
324324
mktest!(
325325
refs_starts_with_slash,
326326
b"/refs/heads/main",
327327
RefError::Tag(TagError::StartsWithSlash)
328328
);
329-
mktests!(refs_starts_with_slash_san, b"/refs/heads/main", "-refs/heads/main");
329+
mktests!(refs_starts_with_slash_san, b"/refs/heads/main", "refs/heads/main");
330330
mktest!(
331331
ends_with_slash,
332332
b"refs/heads/main/",
@@ -338,11 +338,7 @@ mod name {
338338
b"refs/heads/main///",
339339
RefError::Tag(TagError::EndsWithSlash)
340340
);
341-
mktests!(
342-
ends_with_slash_multiple_san,
343-
b"refs/heads/main///",
344-
"refs/heads/main---"
345-
);
341+
mktests!(ends_with_slash_multiple_san, b"refs/heads/main///", "refs/heads/main");
346342
mktest!(
347343
a_path_with_duplicate_slashes,
348344
b"refs//heads/main",

gix-validate/tests/tag/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ mod name {
44
#[test]
55
fn $name() {
66
let actual = gix_validate::reference::name_partial_or_sanitize($input.as_bstr());
7-
assert_eq!(actual.as_ref(), $expected);
7+
assert_eq!(actual, $expected);
88
assert!(gix_validate::reference::name_partial(actual.as_ref()).is_ok());
99
}
1010
};
@@ -79,7 +79,7 @@ mod name {
7979
mktest!(suffix_is_dot_lock, b"prefix.lock", LockFileSuffix);
8080
mktest!(too_many_dots, b"......", RepeatedDot);
8181
mktests!(too_many_dots_san, b"......", "-");
82-
mktests!(too_many_dots_and_slashes_san, b"//....///....///", "-");
82+
mktests!(too_many_dots_and_slashes_san, b"//....///....///", "-/-");
8383
mktests!(suffix_is_dot_lock_san, b"prefix.lock", "prefix");
8484
mktest!(suffix_is_dot_lock_multiple, b"prefix.lock.lock", LockFileSuffix);
8585
mktests!(suffix_is_dot_lock_multiple_san, b"prefix.lock.lock", "prefix");
@@ -98,7 +98,7 @@ mod name {
9898
mktest!(starts_with_asterisk, b"*suffix", Asterisk);
9999
mktests!(starts_with_asterisk_san, b"*suffix", "-suffix");
100100
mktest!(starts_with_slash, b"/suffix", StartsWithSlash);
101-
mktests!(starts_with_slash_san, b"/suffix", "-suffix");
101+
mktests!(starts_with_slash_san, b"/suffix", "suffix");
102102
mktest!(ends_with_asterisk, b"prefix*", Asterisk);
103103
mktests!(ends_with_asterisk_san, b"prefix*", "prefix-");
104104
mktest!(contains_asterisk, b"prefix*suffix", Asterisk);

0 commit comments

Comments
 (0)