Skip to content

Commit 0f3bc3f

Browse files
andrewliebenowandrewliebenow
committed
Fix error handling. Add test.
1 parent f697649 commit 0f3bc3f

File tree

6 files changed

+105
-70
lines changed

6 files changed

+105
-70
lines changed

‎src/uu/echo/src/echo.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ fn execute(
355355
arguments_after_options: ValuesRef<'_, OsString>,
356356
) -> UResult<()> {
357357
for (i, input) in arguments_after_options.enumerate() {
358-
let bytes = uucore::format::bytes_from_os_str(input)?;
358+
let bytes = uucore::format::try_get_bytes_from_os_str(input)?;
359359

360360
if i > 0 {
361361
stdout_lock.write_all(b" ")?;

‎src/uu/printf/src/printf.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@ use std::ffi::OsString;
1111
use std::io::stdout;
1212
use std::ops::ControlFlow;
1313
use uucore::error::{UResult, UUsageError};
14-
use uucore::format::{bytes_from_os_str, parse_spec_and_escape, FormatArgument, FormatItem};
14+
use uucore::format::{
15+
parse_spec_and_escape, try_get_bytes_from_os_str, FormatArgument, FormatItem,
16+
};
1517
use uucore::{format_usage, help_about, help_section, help_usage};
1618

1719
const VERSION: &str = "version";
@@ -33,7 +35,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
3335
.get_one::<OsString>(options::FORMAT)
3436
.ok_or_else(|| UUsageError::new(1, "missing operand"))?;
3537

36-
let format_bytes = bytes_from_os_str(format)?;
38+
let format_bytes = try_get_bytes_from_os_str(format)?;
3739

3840
let values = match matches.get_many::<OsString>(options::ARGUMENT) {
3941
Some(os_string) => os_string

‎src/uucore/src/lib/features/format/argument.rs‎

Lines changed: 70 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,19 @@
33
// For the full copyright and license information, please view the LICENSE
44
// file that was distributed with this source code.
55

6+
use super::FormatError;
67
use crate::{
7-
error::{set_exit_code, UResult, USimpleError},
8+
error::{set_exit_code, UError},
89
features::format::num_parser::{ParseError, ParsedNumber},
910
quoting_style::{escape_name, Quotes, QuotingStyle},
10-
show, show_error, show_warning,
11+
show_error, show_warning,
1112
};
1213
use os_display::Quotable;
13-
use std::ffi::{OsStr, OsString};
14+
use std::{
15+
error::Error,
16+
ffi::{OsStr, OsString},
17+
fmt::Display,
18+
};
1419

1520
/// An argument for formatting
1621
///
@@ -31,70 +36,70 @@ pub enum FormatArgument {
3136
}
3237

3338
pub trait ArgumentIter<'a>: Iterator<Item = &'a FormatArgument> {
34-
fn get_char(&mut self) -> u8;
35-
fn get_i64(&mut self) -> i64;
36-
fn get_u64(&mut self) -> u64;
37-
fn get_f64(&mut self) -> f64;
39+
fn get_char(&mut self) -> Result<u8, FormatError>;
40+
fn get_i64(&mut self) -> Result<i64, FormatError>;
41+
fn get_u64(&mut self) -> Result<u64, FormatError>;
42+
fn get_f64(&mut self) -> Result<f64, FormatError>;
3843
fn get_str(&mut self) -> &'a OsStr;
3944
}
4045

4146
impl<'a, T: Iterator<Item = &'a FormatArgument>> ArgumentIter<'a> for T {
42-
fn get_char(&mut self) -> u8 {
47+
fn get_char(&mut self) -> Result<u8, FormatError> {
4348
let Some(next) = self.next() else {
44-
return b'\0';
49+
return Ok(b'\0');
4550
};
4651
match next {
47-
FormatArgument::Char(c) => *c as u8,
48-
FormatArgument::Unparsed(os) => match bytes_from_os_str(os).unwrap().first() {
49-
Some(&byte) => byte,
50-
None => b'\0',
52+
FormatArgument::Char(c) => Ok(*c as u8),
53+
FormatArgument::Unparsed(os) => match try_get_bytes_from_os_str(os)?.first() {
54+
Some(&byte) => Ok(byte),
55+
None => Ok(b'\0'),
5156
},
52-
_ => b'\0',
57+
_ => Ok(b'\0'),
5358
}
5459
}
5560

56-
fn get_u64(&mut self) -> u64 {
61+
fn get_u64(&mut self) -> Result<u64, FormatError> {
5762
let Some(next) = self.next() else {
58-
return 0;
63+
return Ok(0);
5964
};
6065
match next {
61-
FormatArgument::UnsignedInt(n) => *n,
66+
FormatArgument::UnsignedInt(n) => Ok(*n),
6267
FormatArgument::Unparsed(os) => {
63-
let str = get_str_or_exit_with_error(os);
68+
let str = try_get_str_from_os_str(os)?;
6469

65-
extract_value(ParsedNumber::parse_u64(str), str)
70+
Ok(extract_value(ParsedNumber::parse_u64(str), str))
6671
}
67-
_ => 0,
72+
_ => Ok(0),
6873
}
6974
}
7075

71-
fn get_i64(&mut self) -> i64 {
76+
fn get_i64(&mut self) -> Result<i64, FormatError> {
7277
let Some(next) = self.next() else {
73-
return 0;
78+
return Ok(0);
7479
};
7580
match next {
76-
FormatArgument::SignedInt(n) => *n,
81+
FormatArgument::SignedInt(n) => Ok(*n),
7782
FormatArgument::Unparsed(os) => {
78-
let str = get_str_or_exit_with_error(os);
83+
let str = try_get_str_from_os_str(os)?;
7984

80-
extract_value(ParsedNumber::parse_i64(str), str)
85+
Ok(extract_value(ParsedNumber::parse_i64(str), str))
8186
}
82-
_ => 0,
87+
_ => Ok(0),
8388
}
8489
}
8590

86-
fn get_f64(&mut self) -> f64 {
91+
fn get_f64(&mut self) -> Result<f64, FormatError> {
8792
let Some(next) = self.next() else {
88-
return 0.0;
93+
return Ok(0.0);
8994
};
9095
match next {
91-
FormatArgument::Float(n) => *n,
96+
FormatArgument::Float(n) => Ok(*n),
9297
FormatArgument::Unparsed(os) => {
93-
let str = get_str_or_exit_with_error(os);
98+
let str = try_get_str_from_os_str(os)?;
9499

95-
extract_value(ParsedNumber::parse_f64(str), str)
100+
Ok(extract_value(ParsedNumber::parse_f64(str), str))
96101
}
97-
_ => 0.0,
102+
_ => Ok(0.0),
98103
}
99104
}
100105

@@ -135,14 +140,41 @@ fn extract_value<T: Default>(p: Result<T, ParseError<'_, T>>, input: &str) -> T
135140
} else {
136141
show_error!("{}: value not completely converted", input.quote());
137142
}
143+
138144
v
139145
}
140146
}
141147
}
142148
}
143149
}
144150

145-
pub fn bytes_from_os_str(input: &OsStr) -> UResult<&[u8]> {
151+
#[derive(Debug)]
152+
pub struct NonUtf8OsStr(pub String);
153+
154+
impl Display for NonUtf8OsStr {
155+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
156+
f.write_fmt(format_args!(
157+
"invalid (non-UTF-8) string like {} encountered",
158+
self.0.quote(),
159+
))
160+
}
161+
}
162+
163+
impl Error for NonUtf8OsStr {}
164+
impl UError for NonUtf8OsStr {}
165+
166+
pub fn try_get_str_from_os_str(os_str: &OsStr) -> Result<&str, NonUtf8OsStr> {
167+
match os_str.to_str() {
168+
Some(st) => Ok(st),
169+
None => {
170+
let cow = os_str.to_string_lossy();
171+
172+
Err(NonUtf8OsStr(cow.to_string()))
173+
}
174+
}
175+
}
176+
177+
pub fn try_get_bytes_from_os_str(input: &OsStr) -> Result<&[u8], NonUtf8OsStr> {
146178
let result = {
147179
#[cfg(target_family = "unix")]
148180
{
@@ -153,38 +185,18 @@ pub fn bytes_from_os_str(input: &OsStr) -> UResult<&[u8]> {
153185

154186
#[cfg(not(target_family = "unix"))]
155187
{
156-
use crate::error::USimpleError;
157-
158188
// TODO
159189
// Verify that this works correctly on these platforms
160190
match input.to_str().map(|st| st.as_bytes()) {
161191
Some(sl) => Ok(sl),
162-
None => Err(USimpleError::new(
163-
1,
164-
"non-UTF-8 string encountered when not allowed",
165-
)),
192+
None => {
193+
let cow = input.to_string_lossy();
194+
195+
Err(NonUtf8OsStr(cow.to_string()))
196+
}
166197
}
167198
}
168199
};
169200

170201
result
171202
}
172-
173-
fn get_str_or_exit_with_error(os_str: &OsStr) -> &str {
174-
match os_str.to_str() {
175-
Some(st) => st,
176-
None => {
177-
let cow = os_str.to_string_lossy();
178-
179-
let quoted = cow.quote();
180-
181-
let error = format!(
182-
"argument like {quoted} is not a valid UTF-8 string, and could not be parsed as an integer",
183-
);
184-
185-
show!(USimpleError::new(1, error.clone()));
186-
187-
panic!("{error}");
188-
}
189-
}
190-
}

‎src/uucore/src/lib/features/format/mod.rs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ pub mod num_parser;
3838
mod spec;
3939

4040
pub use argument::*;
41+
use os_display::Quotable;
4142
use spec::Spec;
4243
use std::{
4344
error::Error,
@@ -63,6 +64,7 @@ pub enum FormatError {
6364
NeedAtLeastOneSpec(Vec<u8>),
6465
WrongSpecType,
6566
InvalidPrecision(String),
67+
InvalidEncoding(NonUtf8OsStr),
6668
}
6769

6870
impl Error for FormatError {}
@@ -74,6 +76,12 @@ impl From<std::io::Error> for FormatError {
7476
}
7577
}
7678

79+
impl From<NonUtf8OsStr> for FormatError {
80+
fn from(value: NonUtf8OsStr) -> FormatError {
81+
FormatError::InvalidEncoding(value)
82+
}
83+
}
84+
7785
impl Display for FormatError {
7886
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
7987
match self {
@@ -98,6 +106,13 @@ impl Display for FormatError {
98106
Self::IoError(_) => write!(f, "io error"),
99107
Self::NoMoreArguments => write!(f, "no more arguments"),
100108
Self::InvalidArgument(_) => write!(f, "invalid argument"),
109+
Self::InvalidEncoding(no) => {
110+
write!(
111+
f,
112+
"invalid (non-UTF-8) argument like {} encountered",
113+
no.0.quote()
114+
)
115+
}
101116
}
102117
}
103118
}

‎src/uucore/src/lib/features/format/spec.rs‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,11 @@
66
// spell-checker:ignore (vars) intmax ptrdiff padlen
77

88
use super::{
9-
bytes_from_os_str,
109
num_format::{
1110
self, Case, FloatVariant, ForceDecimal, Formatter, NumberAlignment, PositiveSign, Prefix,
1211
UnsignedIntVariant,
1312
},
14-
parse_escape_only, ArgumentIter, FormatChar, FormatError,
13+
parse_escape_only, try_get_bytes_from_os_str, ArgumentIter, FormatChar, FormatError,
1514
};
1615
use crate::quoting_style::{escape_name, QuotingStyle};
1716
use std::{io::Write, ops::ControlFlow};
@@ -315,7 +314,7 @@ impl Spec {
315314
match self {
316315
Self::Char { width, align_left } => {
317316
let width = resolve_asterisk(*width, &mut args)?.unwrap_or(0);
318-
write_padded(writer, &[args.get_char()], width, *align_left)
317+
write_padded(writer, &[args.get_char()?], width, *align_left)
319318
}
320319
Self::String {
321320
width,
@@ -334,7 +333,7 @@ impl Spec {
334333

335334
let os_str = args.get_str();
336335

337-
let bytes = bytes_from_os_str(os_str).unwrap();
336+
let bytes = try_get_bytes_from_os_str(os_str).unwrap();
338337

339338
let truncated = match precision {
340339
Some(p) if p < os_str.len() => &bytes[..p],
@@ -346,7 +345,7 @@ impl Spec {
346345
Self::EscapedString => {
347346
let os_str = args.get_str();
348347

349-
let bytes = bytes_from_os_str(os_str).unwrap();
348+
let bytes = try_get_bytes_from_os_str(os_str).unwrap();
350349

351350
let mut parsed = Vec::<u8>::new();
352351

@@ -386,7 +385,7 @@ impl Spec {
386385
} => {
387386
let width = resolve_asterisk(*width, &mut args)?.unwrap_or(0);
388387
let precision = resolve_asterisk(*precision, &mut args)?.unwrap_or(0);
389-
let i = args.get_i64();
388+
let i = args.get_i64()?;
390389

391390
if precision as u64 > i32::MAX as u64 {
392391
return Err(FormatError::InvalidPrecision(precision.to_string()));
@@ -409,7 +408,7 @@ impl Spec {
409408
} => {
410409
let width = resolve_asterisk(*width, &mut args)?.unwrap_or(0);
411410
let precision = resolve_asterisk(*precision, &mut args)?.unwrap_or(0);
412-
let i = args.get_u64();
411+
let i = args.get_u64()?;
413412

414413
if precision as u64 > i32::MAX as u64 {
415414
return Err(FormatError::InvalidPrecision(precision.to_string()));
@@ -435,7 +434,7 @@ impl Spec {
435434
} => {
436435
let width = resolve_asterisk(*width, &mut args)?.unwrap_or(0);
437436
let precision = resolve_asterisk(*precision, &mut args)?.unwrap_or(6);
438-
let f = args.get_f64();
437+
let f = args.get_f64()?;
439438

440439
if precision as u64 > i32::MAX as u64 {
441440
return Err(FormatError::InvalidPrecision(precision.to_string()));
@@ -463,7 +462,7 @@ fn resolve_asterisk<'a>(
463462
) -> Result<Option<usize>, FormatError> {
464463
Ok(match option {
465464
None => None,
466-
Some(CanAsterisk::Asterisk) => Some(usize::try_from(args.get_u64()).ok().unwrap_or(0)),
465+
Some(CanAsterisk::Asterisk) => Some(usize::try_from(args.get_u64()?).ok().unwrap_or(0)),
467466
Some(CanAsterisk::Fixed(w)) => Some(w),
468467
})
469468
}

‎tests/by-util/test_printf.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -941,4 +941,11 @@ fn non_utf_8_input() {
941941
.arg(os_str)
942942
.succeeds()
943943
.stdout_only_bytes(INPUT_AND_OUTPUT);
944+
945+
new_ucmd!()
946+
.arg("%d")
947+
.arg(os_str)
948+
.fails()
949+
.stderr_only("printf: invalid (non-UTF-8) argument like 'Swer an rehte g�ete wendet s�n gem�ete, dem volget s�lde und �re.' encountered
950+
");
944951
}

0 commit comments

Comments
 (0)