Skip to content

Commit

Permalink
fix: regression that could cause non-linear parsing behaviour.
Browse files Browse the repository at this point in the history
  • Loading branch information
@Byron
Byron committed Aug 18, 2023
2 parents 2372437 + a743c5d commit 66dadf8
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 54 deletions.
113 changes: 60 additions & 53 deletions gix-config/src/parse/nom/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ fn section<'i>(
dispatch(Event::Newline(Cow::Borrowed(v.as_bstr())));
}

let _ = key_value_pair(i, node, dispatch);
key_value_pair(i, node, dispatch)?;

if let Some(comment) = opt(comment).parse_next(i)? {
dispatch(Event::Comment(comment));
Expand Down Expand Up @@ -213,16 +213,18 @@ fn key_value_pair<'i>(
dispatch: &mut impl FnMut(Event<'i>),
) -> PResult<(), NomError<&'i [u8]>> {
*node = ParseNode::Name;
let name = config_name.parse_next(i)?;
if let Some(name) = opt(config_name).parse_next(i)? {
dispatch(Event::SectionKey(section::Key(Cow::Borrowed(name))));

dispatch(Event::SectionKey(section::Key(Cow::Borrowed(name))));
if let Some(whitespace) = opt(take_spaces1).parse_next(i)? {
dispatch(Event::Whitespace(Cow::Borrowed(whitespace)));
}

if let Some(whitespace) = opt(take_spaces1).parse_next(i)? {
dispatch(Event::Whitespace(Cow::Borrowed(whitespace)));
*node = ParseNode::Value;
config_value(i, dispatch)
} else {
Ok(())
}

*node = ParseNode::Value;
config_value(i, dispatch)
}

/// Parses the config name of a config pair. Assumes the input has already been
Expand Down Expand Up @@ -265,62 +267,67 @@ fn value_impl<'i>(i: &mut &'i [u8], dispatch: &mut impl FnMut(Event<'i>)) -> PRe
// Used to determine if we return a Value or Value{Not,}Done
let mut partial_value_found = false;

while let Some(c) = i.next_token() {
match c {
b'\n' => {
value_end = Some(i.offset_from(&value_start_checkpoint) - 1);
break;
}
b';' | b'#' if !is_in_quotes => {
value_end = Some(i.offset_from(&value_start_checkpoint) - 1);
break;
}
b'\\' => {
let escaped_index = i.offset_from(&value_start_checkpoint);
let escape_index = escaped_index - 1;
let Some(mut c) = i.next_token() else {
i.reset(start_checkpoint);
return Err(winnow::error::ErrMode::from_error_kind(i, ErrorKind::Token));
};
let mut consumed = 1;
if c == b'\r' {
c = i.next_token().ok_or_else(|| {
i.reset(start_checkpoint);
winnow::error::ErrMode::from_error_kind(i, ErrorKind::Token)
})?;
if c != b'\n' {
loop {
let _ = take_while(0.., |c| !matches!(c, b'\n' | b'\\' | b'"' | b';' | b'#')).parse_next(i)?;
if let Some(c) = i.next_token() {
match c {
b'\n' => {
value_end = Some(i.offset_from(&value_start_checkpoint) - 1);
break;
}
b';' | b'#' if !is_in_quotes => {
value_end = Some(i.offset_from(&value_start_checkpoint) - 1);
break;
}
b'\\' => {
let escaped_index = i.offset_from(&value_start_checkpoint);
let escape_index = escaped_index - 1;
let Some(mut c) = i.next_token() else {
i.reset(start_checkpoint);
return Err(winnow::error::ErrMode::from_error_kind(i, ErrorKind::Slice));
return Err(winnow::error::ErrMode::from_error_kind(i, ErrorKind::Token));
};
let mut consumed = 1;
if c == b'\r' {
c = i.next_token().ok_or_else(|| {
i.reset(start_checkpoint);
winnow::error::ErrMode::from_error_kind(i, ErrorKind::Token)
})?;
if c != b'\n' {
i.reset(start_checkpoint);
return Err(winnow::error::ErrMode::from_error_kind(i, ErrorKind::Slice));
}
consumed += 1;
}
consumed += 1;
}

match c {
b'\n' => {
partial_value_found = true;
match c {
b'\n' => {
partial_value_found = true;

i.reset(value_start_checkpoint);
i.reset(value_start_checkpoint);

let value = i.next_slice(escape_index).as_bstr();
dispatch(Event::ValueNotDone(Cow::Borrowed(value)));
let value = i.next_slice(escape_index).as_bstr();
dispatch(Event::ValueNotDone(Cow::Borrowed(value)));

i.next_token();
i.next_token();

let nl = i.next_slice(consumed).as_bstr();
dispatch(Event::Newline(Cow::Borrowed(nl)));
let nl = i.next_slice(consumed).as_bstr();
dispatch(Event::Newline(Cow::Borrowed(nl)));

value_start_checkpoint = i.checkpoint();
value_end = None;
}
b'n' | b't' | b'\\' | b'b' | b'"' => {}
_ => {
i.reset(start_checkpoint);
return Err(winnow::error::ErrMode::from_error_kind(i, ErrorKind::Token));
value_start_checkpoint = i.checkpoint();
value_end = None;
}
b'n' | b't' | b'\\' | b'b' | b'"' => {}
_ => {
i.reset(start_checkpoint);
return Err(winnow::error::ErrMode::from_error_kind(i, ErrorKind::Token));
}
}
}
b'"' => is_in_quotes = !is_in_quotes,
_ => {}
}
b'"' => is_in_quotes = !is_in_quotes,
_ => {}
} else {
break;
}
}
if is_in_quotes {
Expand Down
9 changes: 8 additions & 1 deletion gix-config/src/parse/nom/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -808,7 +808,14 @@ mod key_value_pair {
fn nonascii_is_allowed_for_values_but_not_for_keys() {
let mut node = ParseNode::SectionHeader;
let mut vec = Default::default();
assert!(key_value("你好".as_bytes(), &mut node, &mut vec).is_err());
assert!(
key_value("你好".as_bytes(), &mut node, &mut vec).is_ok(),
"Verifying `is_ok` because bad keys get ignored, the caller parser handles this as error"
);
assert_eq!(vec, into_events(vec![]));

let mut node = ParseNode::SectionHeader;
let mut vec = Default::default();
assert!(key_value("a = 你好 ".as_bytes(), &mut node, &mut vec).is_ok());
assert_eq!(
vec,
Expand Down

0 comments on commit 66dadf8

Please sign in to comment.