uutils / parse_datetime Goto Github PK

View Code? Open in Web Editor NEW

13.0 13.0 15.0 123 KB

Parses a relative time string and returns a `Duration`

License: MIT License

Rust 100.00%

parse_datetime's People

Contributors

Stargazers

Watchers

Forkers

sylvestre cakebaker benjscho redfire75369 wanderinglethe undali simon04 krishnanagam philolo1 tertsdiepraam sruggier dcechano drmorr0 matrixhead

parse_datetime's Issues

fuzzing: thread 'main' panicked at 'Duration::seconds out of bounds'

Code:

fn main() {
let _ = parse_datetime::from_str("8888888888888h");
}

Backtrace:

thread 'main' panicked at 'Duration::seconds out of bounds', /home/sylvestre/.cargo/registry/src/github.com-1ecc6299db9ec823/chrono-0.4.26/src/oldtime.rs:121:13
stack backtrace:
   0: rust_begin_unwind
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/core/src/panicking.rs:64:14
   2: chrono::oldtime::Duration::seconds
             at /home/sylvestre/.cargo/registry/src/github.com-1ecc6299db9ec823/chrono-0.4.26/src/oldtime.rs:121:13
   3: chrono::oldtime::Duration::hours
             at /home/sylvestre/.cargo/registry/src/github.com-1ecc6299db9ec823/chrono-0.4.26/src/oldtime.rs:100:9
   4: parse_datetime::from_str_at_date
             at /home/sylvestre/.cargo/registry/src/github.com-1ecc6299db9ec823/parse_datetime-0.4.0/src/lib.rs:166:39
   5: parse_datetime::from_str
             at /home/sylvestre/.cargo/registry/src/github.com-1ecc6299db9ec823/parse_datetime-0.4.0/src/lib.rs:95:5
   6: b::main
             at ./src/main.rs:2:9
   7: core::ops::function::FnOnce::call_once
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Dependency Dashboard

This issue lists Renovate updates and detected dependencies. Read the Dependency Dashboard docs to learn more.

Open

These updates have all been created already. Click a checkbox below to force a retry/rebase of any.

fix(deps): update rust crate chrono to 0.4.38

Detected dependencies

cargo

Cargo.toml

regex 1.10.4

chrono 0.4

nom 7.1.3

fuzz/Cargo.toml

rand 0.8.5

libfuzzer-sys 0.4.7

regex 1.10.4

chrono 0.4

github-actions

.github/workflows/ci.yml

actions/checkout v4

actions/checkout v4

actions/checkout v4

actions/checkout v4

actions/checkout v4

codecov/codecov-action v4

actions/checkout v4

Swatinem/rust-cache v2

Check this box to trigger a request for Renovate to run again on this repository

Chrono's API is inherently inconsistent with GNU's

I am creating this issue to start a discussion about how to overcome some roadblocks I discovered while trying to address #3505 over at coreutils.

Issue 1

To get right to it, the implementation currently used by this library is not consistent with GNU's.

Currently that way that parse_datetime parses relative strings is that in runs the string through a regex and creates Duration objects and adds them together. The inconstancy arises when adding months: it simply adds 30 days; GNU does not do this. Under the existing (parse_datetime) API 2022-05-15 +1 month will land you on 2022-06-14 (After you do some refactoring to get a date back not a Duration) while running GNU's date -d'2022-05-15 +1 month lands you on 2022-06-15. Now, we could have our API increment months using Months::new which returns a Duration object but that would change the behavior of the existing API and cause a bunch of tests to fail. Why can't we change the tests? Let me explain.

Issue 2

Chrono's API is inconsistent with GNU

While working to solve #3505 I ran into a subtle bug. Addition and subtraction of Months in Chrono's API is non-commutative. The TL;DR is that chrono does clamping when adding months to their Date APIs. To who understand why this is an issue consider:

#[test]
    fn test_test(){
        let date0 = NaiveDate::from_ymd(2023, 1, 31).add(Months::new(1)).add(Months::new(1));
        let date1 = NaiveDate::from_ymd(2023, 1, 31).add(Months::new(2));
        assert_eq!(date1, date0);
    }

Output:

assertion `left == right` failed
  left: 2023-03-31
 right: 2023-03-28

While nothing is being commuted here, it shows that addition of months cannot be commutative under an implementation with this behavior. GNU does not do this. Consider:

[dylan@fedora parse_datetime]$ date -d'2023-01-31 + 1 month + 1 month'
Fri Mar 31 12:00:00 AM EDT 2023
[dylan@fedora parse_datetime]$ date -d'2023-01-31 + 2 months'
Fri Mar 31 12:00:00 AM EDT 2023

While GNU is consistent, Chrono is not. In fact it is 3 whole days off (If "off" is even the right word. What is "correct" is highly debatable). The addition of months is complicated because what it means to add months is somewhat arbitrary. Even how GNU adds months seems inconsistent sometimes. Adding 30 days vs. 1 month, 60 days vs. 2 months results in different dates that ostensibly don't have any obvious connective logic. This behavior further serves to frustrate the issue under discussion!

Whats the Point?

I appears that with these issues, bringing coreutils's touch and date APIs consistent with GNU's under any arbitrary string will be extremely difficult due to how inherently different chrono and GNU handle the addition and subtraction of months (maybe even other units of time).

Options

As touched on earlier, we could tweak parse_relative_time.rs to use chrono's Months::new instead of Durations::days and accept that it will cause a slight change in behavior and just rewrite the tests (What I was tempted to ask for permission to do).

The other option I considered was having a separate implementation for when you want a date back vs. a Duration but that seems silly. Users would rightfully assume that the separate functions are two sides of the same coin and would be consistent with each other.

The core issue of this discussion is that either option the result would be behavior that is inconsistent with GNU is some cases. It doesn't appear possible to get away from it. Given any arbitrary string what is the correct order of operations to get the correct date (as defined by GNU) and is it possible to do it with chrono under their existing implementation???

For what it is worth my intitial pull request to address #3505 did appear to have the requisite functionality but it is unclear if that is the case in for any arbitrary string. And using that would, again, result in a API behavior change.

These issues don't necessarily have obvious solutions which is why I wanted to bring it to the uutils community.

"this x" is not recognized

GNU date recognizes "this x", for example:

$ date --date="this year"
Tue Jun 20 10:50:27 AM CEST 2023
$ date --date="this week"
Tue Jun 20 10:50:51 AM CEST 2023

uutils date, using parse_datetime, doesn't recognize "this x" and returns an "invalid date" error instead.

epoch parsing with timezone other than UTC

hi, i think our epoch parsing is not working properly.

if we try to run test_epoch_seconds it would work but if we try changing the timezone to something like env::set_var("TZ", "EST") it would fail. it shouldn't be doing that right? because epoch's should be same even if we change the timezone.

i think it is happening because we are treating NaiveDateTime returned by from_timestamp_opt (which is in UTC) as local time in naive_dt_to_fixed_offset and i think we could fix it by treating it as UTC

if you guys think i'm right can i put together a pr with the necessary changes

Potential segfault in the time crate (cargo deny warning)

cargo deny in uutils/coreutils currently shows the following warning for uutils/coreutils#4936:

warning[vulnerability]: Potential segfault in the time crate
    ┌─ /github/workspace/Cargo.lock:234:1
    │
234 │ time 0.1.45 registry+https://github.com/rust-lang/crates.io-index
    │ ----------------------------------------------------------------- security vulnerability detected
    │
    = ID: RUSTSEC-2020-0071
    = Advisory: https://rustsec.org/advisories/RUSTSEC-2020-0071
    = ### Impact
      
      Unix-like operating systems may segfault due to dereferencing a dangling pointer in specific circumstances. This requires an environment variable to be set in a different thread than the affected functions. This may occur without the user's knowledge, notably in a third-party library.
      
      The affected functions from time 0.2.7 through 0.2.22 are:
      
      - `time::UtcOffset::local_offset_at`
      - `time::UtcOffset::try_local_offset_at`
      - `time::UtcOffset::current_local_offset`
      - `time::UtcOffset::try_current_local_offset`
      - `time::OffsetDateTime::now_local`
      - `time::OffsetDateTime::try_now_local`
      
      The affected functions in time 0.1 (all versions) are:
      
      - `at`
      - `at_utc`
      - `now`
      
      Non-Unix targets (including Windows and wasm) are unaffected.
      
      ### Patches
      
      Pending a proper fix, the internal method that determines the local offset has been modified to always return `None` on the affected operating systems. This has the effect of returning an `Err` on the `try_*` methods and `UTC` on the non-`try_*` methods.
      
      Users and library authors with time in their dependency tree should perform `cargo update`, which will pull in the updated, unaffected code.
      
      Users of time 0.1 do not have a patch and should upgrade to an unaffected version: time 0.2.23 or greater or the 0.3 series.
      
      ### Workarounds
      
      A possible workaround for crates affected through the transitive dependency in `chrono`, is to avoid using the default `oldtime` feature dependency of the `chrono` crate by disabling its `default-features` and manually specifying the required features instead.
      
      #### Examples:
      
      `Cargo.toml`:  
      
      ```toml
      chrono = { version = "0.4", default-features = false, features = ["serde"] }
      ```
      
      ```toml
      chrono = { version = "0.4.22", default-features = false, features = ["clock"] }
      ```
      
      Commandline:  
      
      ```bash
      cargo add chrono --no-default-features -F clock
      ```
      
      Sources:  
       - [chronotope/chrono#602 (comment)](https://github.com/chronotope/chrono/issues/602#issuecomment-1242[14](https://github.com/uutils/coreutils/actions/runs/5164696311/jobs/9303762538?pr=4936#step:4:15)9249)  
       - [vityafx/serde-aux#21](https://github.com/vityafx/serde-aux/issues/21)
    = Announcement: https://github.com/time-rs/time/issues/293
    = Solution: Upgrade to >=0.2.23
    = time v0.1.45
      └── chrono v0.4.26
          ├── (dev) coreutils v0.0.[18](https://github.com/uutils/coreutils/actions/runs/5164696311/jobs/9303762538?pr=4936#step:4:19)
          ├── humantime_to_duration v0.3.0
          │   ├── uu_date v0.0.18
          │   │   └── coreutils v0.0.18 (*)
          │   └── uu_touch v0.0.18
          │       └── coreutils v0.0.18 (*)
          ├── uu_date v0.0.18 (*)
          ├── uu_du v0.0.18
          │   └── coreutils v0.0.18 (*)
          ├── uu_ls v0.0.18
          │   ├── coreutils v0.0.18 (*)
          │   ├── uu_dir v0.0.18
          │   │   └── coreutils v0.0.18 (*)
          │   └── uu_vdir v0.0.18
          │       └── coreutils v0.0.18 (*)
          └── uu_uptime v0.0.18
              └── coreutils v0.0.18 (*)

parse_datetime should support "@" syntax

We would like parse_time to support "@" syntax, that indicates the seconds passed from the "Thu Jan 1 00:00:00 UTC 1970".

See related issue: uutils/coreutils#5177

I am intending on working on this issue.

Days of the week are not recognized

GNU date recognizes days of the week and their three-letter abbreviations (plus tues, thur, and thurs), for example:

$ date --date="monday"
Mon Jun 26 12:00:00 AM CEST 2023
$ date --date="mon"
Mon Jun 26 12:00:00 AM CEST 2023
$ date --date="tuesday"
Tue Jun 20 12:00:00 AM CEST 2023
$ date --date="tue"
Tue Jun 20 12:00:00 AM CEST 2023

uutils date, using parse_datetime, doesn't recognize any days of the week and returns an "invalid date" error instead.

Military timezones are not being recognized

GNU date recognizes military timezones (one letter).

Reference: https://github.com/coreutils/gnulib/blob/master/lib/parse-datetime.y#L1155

Ubuntu:
date -d "a" 
Wed Aug 30 08:00:00 JST 2023

Utils:
date: invalid date 'a'

"next x" and "last x" are not recognized

GNU date recognizes "next x" and "last x", for example:

$ date --date="next month"
Fri Jul  7 11:44:46 AM CEST 2023
$ date --date="last month"
Sun May  7 11:44:52 AM CEST 2023

uutils date, using humantime_to_duration, doesn't recognize such dates and returns an "invalid date" error.

Negative numbers for @ are not being recognized

While "@0" and "@positive" integer work, "@-1" does not work.

On GNU date it returns the time in seconds before JAN 01 1970.

For example:

"date -d '@-1'"     
Thu Jan  1 08:59:59 JST 1970

"date -d '@-1000000'"           
Sat Dec 20 19:13:20 JST 1969

Cargo test fails at 0:20 JST

I ran cargo test on the current repo and got two errors:

failures:

---- test_from_str_at_date_day stdout ----
thread 'test_from_str_at_date_day' panicked at 'assertion failed: `(left == right)`
  left: `Duration { secs: 0, nanos: 0 }`,
 right: `Duration { secs: 86400, nanos: 0 }`', tests/simple.rs:133:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- test_duration_parsing stdout ----
thread 'test_duration_parsing' panicked at 'assertion failed: `(left == right)`
  left: `Duration { secs: 31449600, nanos: 0 }`,
 right: `Duration { secs: 31536000, nanos: 0 }`', tests/simple.rs:16:5


failures:
    test_duration_parsing
    test_from_str_at_date_day

test result: FAILED. 4 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s

error: test failed, to rerun pass `--test simple`

I did not get the errors before, so it might have to do with the my timezone. It looks like there might be an error or a different usage between JST and UTC.