coreutils icon indicating copy to clipboard operation
coreutils copied to clipboard

Panics when using date with invalid formats

Open qarmin opened this issue 3 years ago • 7 comments

cargo run '+~\x0a~%\x0a' or cargo run "+␅+%+␄^%)" crashes with backtrace

thread 'main' panicked at 'a Display implementation returned an error unexpectedly: Error', /rustc/897e37553bba8b42751c67658967889d11ecd120/library/alloc/src/string.rs:2511:14
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

qarmin avatar Nov 27 '22 19:11 qarmin

Isnt this related to the chrono crate? Should we report it to them?

use chrono::prelude::*;
use chrono::Duration;
fn main() {
    let f_str = "+~\x0a~%\x0a";
    let date_time: DateTime<Utc> = Utc.with_ymd_and_hms(2017, 04, 02, 12, 50, 32).unwrap();
    let formatted = date_time.format(f_str).to_string(); ----> panics

This panics, and that is exactly what we try to do in in the date.rs.

I also noted that

uu_date '+~\x0a~%\x0a' --> panics

but

uu_date +~\x0a~%\x0a
~x0a~12/05/220a

So also related to how clap processes that incoming string? not sure :P

dmatos2012 avatar Dec 05 '22 10:12 dmatos2012

yeah, it should be reported upstream too but we could do a better job handling the error on our side

sylvestre avatar Dec 05 '22 10:12 sylvestre

They seem to be aware of the issue and want to fix it in version 0.5:

  • https://github.com/chronotope/chrono/issues/575
  • https://github.com/chronotope/chrono/issues/556
  • https://github.com/chronotope/chrono/issues/815

In the meantime, there's not much we can do here except validate the string ourselves before we call the function.

tertsdiepraam avatar Dec 05 '22 12:12 tertsdiepraam

But how could we validate the string ourselves? We dont know what kind of custom input would be given.

Anything that doesnt fall under the defined Format goes under the Format::Custom(String), and that string could be any invalid shape, such as '+~\x0a~%\x0a' or '+␅+%+␄^%)'. Any ideas?.

I also dont understand is how the use of '+~\x0a~%\x0a' vs +~\x0a~%\x0a (quotes vs no quotes) makes it panic or not.

dmatos2012 avatar Dec 05 '22 13:12 dmatos2012

Any ideas?

Not really. The only thing I can come up with is looking through the chrono source and check what makes them panic! and then write a custom function that does roughly the same.

I also dont understand is how the use of '+~\x0a~%\x0a' vs +~\x0a~%\x0a (quotes vs no quotes) makes it panic or not.

\x0a seems to be an escape code for the shell that expands to a newline (hexadecimal character 0a)

You can see it here (note the slight color change): image

clap gets the escaped version. This is all expected behaviour though. We only have to care about the string that clap gets.

Edit: I tested on fish before, but bash is slightly different: image

tertsdiepraam avatar Dec 05 '22 14:12 tertsdiepraam

one option using the existing code would be:

use chrono::format::{Item, strftime::StrftimeItems};
let valid = !Strftimeitems(<your_format_string_here>).any(|i| matches!(i, Item::Error));

If you wanted to get the items themselves while also doing validation you could do:

let items: Result<Vec<Item>, String> = StrftimeItems::new(format_string).try_fold(Vec::new(), |mut acc, x| {
        match x {
            Item::Error => Err(format_string.to_string()),
            otherwise => {
                acc.push(otherwise);
                Ok(acc)
            }
        } 
    });

and use alongside format_with_items

however we are keen to improve this, perhaps by providing some helper functions on StrftimeItems, so we are keen to receive any feedback on what kind of API would be suitable

esheppa avatar Dec 06 '22 01:12 esheppa

That's much better! Thanks!

tertsdiepraam avatar Dec 06 '22 07:12 tertsdiepraam

Fixed a while back by https://github.com/uutils/coreutils/pull/4240 if I'm not mistaken

tertsdiepraam avatar Aug 10 '23 21:08 tertsdiepraam