Skip to content
Merged
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
3 changes: 3 additions & 0 deletions changelog.d/1848.fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fixed a panic in `parse_key_value`, `parse_cef`, `decode_mime_q`, and `parse_ruby_hash` on inputs with lines ≥ 65,535 bytes. This is a workaround until [rust-bakery/nom#1867](https://github.com/rust-bakery/nom/issues/1867) is fixed.

authors: pront
1 change: 1 addition & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,5 @@ arithmetic-side-effects-allowed = [
# https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_method
disallowed-methods = [
{ path = "std::io::Write::write", reason = "This doesn't handle short writes, use `write_all` instead." },
{ path = "nom_language::error::convert_error", reason = "Panics on Rust ≥ 1.87 (https://github.com/rust-bakery/nom/issues/1867). Use crate::parsing::safe_convert_error instead." },
]
42 changes: 42 additions & 0 deletions src/parsing/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,45 @@
pub mod query_string;
pub mod ruby_hash;
pub mod xml;

// nom's convert_error formats a caret at column `n` using `{caret:>n$}`, which
// panics in Rust ≥ 1.87 when n ≥ 65536 (fmt width is capped at 0xffff).
// Until nom#1868 lands, we guard against that here.
pub(crate) fn safe_convert_error(
input: &str,
e: nom_language::error::VerboseError<&str>,
) -> String {
use nom::Offset;

// For each error entry, check whether its column would overflow the limit.
let overflows = e.errors.iter().any(|(substring, _)| {
let offset = input.offset(substring);
let line_begin = input.as_bytes()[..offset]
.iter()
.rev()
.position(|&b| b == b'\n')
.map(|pos| offset - pos)
.unwrap_or(0);
let column = input[line_begin..].offset(substring) + 1;
column >= 65535
});

if overflows {
// Compute position of the first error for a useful message.
let (substring, _) = &e.errors[0];
let offset = input.offset(substring);
let prefix = &input.as_bytes()[..offset];
let line = prefix.iter().filter(|&&b| b == b'\n').count() + 1;
let line_begin = prefix
.iter()
.rev()
.position(|&b| b == b'\n')
.map(|pos| offset - pos)
.unwrap_or(0);
let column = input[line_begin..].offset(substring) + 1;
format!("parse error at line {line}, column {column} (line too long to display context)")
} else {
#[allow(clippy::disallowed_methods)]
nom_language::error::convert_error(input, e)
}
}
Comment thread
pront marked this conversation as resolved.
3 changes: 1 addition & 2 deletions src/parsing/ruby_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@ pub fn parse_ruby_hash(input: &str) -> ExpressionResult<Value> {
let result = parse_hash(input)
.map_err(|err| match err {
nom::Err::Error(err) | nom::Err::Failure(err) => {
// Create a descriptive error message if possible.
nom_language::error::convert_error(input, err)
crate::parsing::safe_convert_error(input, err)
}
nom::Err::Incomplete(_) => err.to_string(),
})
Expand Down
5 changes: 1 addition & 4 deletions src/stdlib/decode_mime_q.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,7 @@ fn decode_mime_q(bytes: &Value) -> Resolved {
))
.parse(input)
.map_err(|e| match e {
nom::Err::Error(e) | nom::Err::Failure(e) => {
// Create a descriptive error message if possible.
nom_language::error::convert_error(input, e)
}
nom::Err::Error(e) | nom::Err::Failure(e) => crate::parsing::safe_convert_error(input, e),
nom::Err::Incomplete(_) => e.to_string(),
})?;

Expand Down
2 changes: 1 addition & 1 deletion src/stdlib/parse_cef.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ fn parse(
.parse(input)
.map_err(|e| match e {
nom::Err::Error(e) | nom::Err::Failure(e) => {
nom_language::error::convert_error(input, e)
crate::parsing::safe_convert_error(input, e)
}
nom::Err::Incomplete(_) => e.to_string(),
})?;
Expand Down
21 changes: 17 additions & 4 deletions src/stdlib/parse_key_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,10 +351,7 @@ fn parse<'a>(
standalone_key,
)
.map_err(|e| match e {
nom::Err::Error(e) | nom::Err::Failure(e) => {
// Create a descriptive error message if possible.
nom_language::error::convert_error(input, e)
}
nom::Err::Error(e) | nom::Err::Failure(e) => crate::parsing::safe_convert_error(input, e),
nom::Err::Incomplete(_) => e.to_string(),
})?;

Expand Down Expand Up @@ -610,6 +607,22 @@ fn type_def() -> TypeDef {
mod test {
use super::*;

#[test]
fn test_long_line_does_not_panic() {
// nom's convert_error panics on Rust ≥ 1.87 when any column offset
// ≥ 65535 (fmt width capped at 0xffff). With standalone_key=false a
// line that has no `=` delimiter forces a nom Err at EOF, which used
// to invoke convert_error and panic.
let input = "a".repeat(65535);
let err = parse(&input, "=", " ", Whitespace::Lenient, false)
.expect_err("expected parse error for long-line input");
let msg = err.to_string();
assert!(
msg.contains("line 1") && msg.contains("column 65536"),
"unexpected error message: {msg}"
);
}

#[test]
fn test_quote_and_escape_char() {
assert_eq!(
Expand Down
Loading