Skip to content

Commit

Permalink
Merge pull request #7032 from jfinkels/seq-fmt-errors
Browse files Browse the repository at this point in the history
seq: improve error handling for invalid -f values
  • Loading branch information
sylvestre authored Dec 31, 2024
2 parents 646fc39 + cccab35 commit ed9e80e
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 4 deletions.
19 changes: 15 additions & 4 deletions src/uucore/src/lib/features/format/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
//! 3. Parse both `printf` specifiers and escape sequences (for e.g. `printf`)
//!
//! This module aims to combine all three use cases. An iterator parsing each
//! of these cases is provided by [`parse_escape_only`], [`parse_spec_only`]
//! of these cases is provided by [`parse_spec_only`], [`parse_escape_only`]
//! and [`parse_spec_and_escape`], respectively.
//!
//! There is a special [`Format`] type, which can be used to parse a format
Expand All @@ -46,6 +46,8 @@ use std::{
ops::ControlFlow,
};

use os_display::Quotable;

use crate::error::UError;

pub use self::{
Expand All @@ -63,6 +65,8 @@ pub enum FormatError {
NeedAtLeastOneSpec(Vec<u8>),
WrongSpecType,
InvalidPrecision(String),
/// The format specifier ends with a %, as in `%f%`.
EndsWithPercent(Vec<u8>),
}

impl Error for FormatError {}
Expand Down Expand Up @@ -92,6 +96,9 @@ impl Display for FormatError {
"format '{}' has no % directive",
String::from_utf8_lossy(s)
),
Self::EndsWithPercent(s) => {
write!(f, "format {} ends in %", String::from_utf8_lossy(s).quote())
}
Self::InvalidPrecision(precision) => write!(f, "invalid precision: '{precision}'"),
// TODO: Error message below needs some work
Self::WrongSpecType => write!(f, "wrong % directive type was given"),
Expand Down Expand Up @@ -190,6 +197,7 @@ pub fn parse_spec_only(
let mut current = fmt;
std::iter::from_fn(move || match current {
[] => None,
[b'%'] => Some(Err(FormatError::EndsWithPercent(fmt.to_vec()))),
[b'%', b'%', rest @ ..] => {
current = rest;
Some(Ok(FormatItem::Char(b'%')))
Expand Down Expand Up @@ -323,11 +331,14 @@ impl<F: Formatter> Format<F> {

let mut suffix = Vec::new();
for item in &mut iter {
match item? {
FormatItem::Spec(_) => {
match item {
// If the `format_string` is of the form `%f%f` or
// `%f%`, then return an error.
Ok(FormatItem::Spec(_)) | Err(FormatError::EndsWithPercent(_)) => {
return Err(FormatError::TooManySpecs(format_string.as_ref().to_vec()));
}
FormatItem::Char(c) => suffix.push(c),
Ok(FormatItem::Char(c)) => suffix.push(c),
Err(e) => return Err(e),
}
}

Expand Down
10 changes: 10 additions & 0 deletions tests/by-util/test_seq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,16 @@ fn test_invalid_format() {
.fails()
.no_stdout()
.stderr_contains("format '%g%g' has too many % directives");
new_ucmd!()
.args(&["-f", "%g%", "1"])
.fails()
.no_stdout()
.stderr_contains("format '%g%' has too many % directives");
new_ucmd!()
.args(&["-f", "%", "1"])
.fails()
.no_stdout()
.stderr_contains("format '%' ends in %");
}

#[test]
Expand Down

0 comments on commit ed9e80e

Please sign in to comment.