Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unfill and refill crashes #467

Merged
merged 3 commits into from
Sep 15, 2022
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
2 changes: 2 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ jobs:
- wrap_first_fit
- wrap_optimal_fit
- wrap_optimal_fit_usize
- unfill
- refill

steps:
- name: Checkout repository
Expand Down
12 changes: 12 additions & 0 deletions fuzz/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,15 @@ name = "wrap_optimal_fit_usize"
path = "fuzz_targets/wrap_optimal_fit_usize.rs"
test = false
doc = false

[[bin]]
name = "refill"
path = "fuzz_targets/refill.rs"
test = false
doc = false

[[bin]]
name = "unfill"
path = "fuzz_targets/unfill.rs"
test = false
doc = false
6 changes: 6 additions & 0 deletions fuzz/fuzz_targets/refill.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#![no_main]
use libfuzzer_sys::fuzz_target;

fuzz_target!(|input: (String, usize)| {
let _ = textwrap::refill(&input.0, input.1);
});
6 changes: 6 additions & 0 deletions fuzz/fuzz_targets/unfill.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#![no_main]
use libfuzzer_sys::fuzz_target;

fuzz_target!(|input: String| {
let _ = textwrap::unfill(&input);
});
51 changes: 39 additions & 12 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,9 @@ where
/// comments (`'#'` and `'/'`).
///
/// The text must come from a single wrapped paragraph. This means
/// that there can be no `"\n\n"` within the text.
/// that there can be no empty lines (`"\n\n"` or `"\r\n\r\n"`) within
/// the text. It is unspecified what happens if `unfill` is called on
/// more than one paragraph of text.
///
/// # Examples
///
Expand All @@ -651,12 +653,10 @@ where
/// assert_eq!(options.line_ending, LineEnding::LF);
/// ```
pub fn unfill(text: &str) -> (String, Options<'_>) {
let line_ending_pat: &[_] = &['\r', '\n'];
let trimmed = text.trim_end_matches(line_ending_pat);
let prefix_chars: &[_] = &[' ', '-', '+', '*', '>', '#', '/'];

let mut options = Options::new(0);
for (idx, line) in trimmed.lines().enumerate() {
for (idx, line) in text.lines().enumerate() {
options.width = std::cmp::max(options.width, core::display_width(line));
let without_prefix = line.trim_start_matches(prefix_chars);
let prefix = &line[..line.len() - without_prefix.len()];
Expand All @@ -681,8 +681,8 @@ pub fn unfill(text: &str) -> (String, Options<'_>) {
let mut unfilled = String::with_capacity(text.len());
let mut detected_line_ending = None;

for (line, ending) in line_ending::NonEmptyLines(text) {
if unfilled.is_empty() {
for (idx, (line, ending)) in line_ending::NonEmptyLines(text).enumerate() {
if idx == 0 {
unfilled.push_str(&line[options.initial_indent.len()..]);
} else {
unfilled.push(' ');
Expand All @@ -694,7 +694,13 @@ pub fn unfill(text: &str) -> (String, Options<'_>) {
_ => (),
}
}
unfilled.push_str(&text[trimmed.len()..]);

// Add back a line ending if `text` ends with the one we detect.
if let Some(line_ending) = detected_line_ending {
if text.ends_with(line_ending.as_str()) {
unfilled.push_str(line_ending.as_str());
}
}

options.line_ending = detected_line_ending.unwrap_or(LineEnding::LF);
(unfilled, options)
Expand Down Expand Up @@ -1772,9 +1778,6 @@ mod tests {
assert_eq!(options.line_ending, LineEnding::CRLF);
}

/// If mixed new line sequence is encountered, we want to fallback to `\n`
/// 1. it is the default
/// 2. it still matches both `\n` and `\r\n` unlike `\r\n` which will not match `\n`
#[test]
fn unfill_mixed_new_lines() {
let (text, options) = unfill("foo\r\nbar\nbaz");
Expand All @@ -1786,14 +1789,14 @@ mod tests {
#[test]
fn unfill_trailing_newlines() {
let (text, options) = unfill("foo\nbar\n\n\n");
assert_eq!(text, "foo bar\n\n\n");
assert_eq!(text, "foo bar\n");
assert_eq!(options.width, 3);
}

#[test]
fn unfill_mixed_trailing_newlines() {
let (text, options) = unfill("foo\r\nbar\n\r\n\n");
assert_eq!(text, "foo bar\n\r\n\n");
assert_eq!(text, "foo bar\n");
assert_eq!(options.width, 3);
assert_eq!(options.line_ending, LineEnding::LF);
}
Expand Down Expand Up @@ -1850,6 +1853,30 @@ mod tests {
assert_eq!(options.subsequent_indent, "> ");
}

#[test]
fn unfill_only_prefixes_issue_466() {
// Test that we don't crash if the first line has only prefix
// chars *and* the second line is shorter than the first line.
let (text, options) = unfill("######\nfoo");
assert_eq!(text, " foo");
assert_eq!(options.width, 6);
assert_eq!(options.initial_indent, "######");
assert_eq!(options.subsequent_indent, "");
}

#[test]
fn unfill_trailing_newlines_issue_466() {
// Test that we don't crash on a '\r' following a string of
// '\n'. The problem was that we removed both kinds of
// characters in one code path, but not in the other.
let (text, options) = unfill("foo\n##\n\n\r");
// The \n\n changes subsequent_indent to "".
assert_eq!(text, "foo ## \r");
assert_eq!(options.width, 3);
assert_eq!(options.initial_indent, "");
assert_eq!(options.subsequent_indent, "");
}

#[test]
fn unfill_whitespace() {
assert_eq!(unfill("foo bar").0, "foo bar");
Expand Down