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 panic for negative inputs to timestamp_millis. #292

Merged
merged 1 commit into from
Dec 4, 2018

Conversation

cmars
Copy link
Contributor

@cmars cmars commented Dec 2, 2018

This patch fixes the case where a negative millisecond offset is passed
to Timezone::timestamp_millis and timestamp_millis_opt and adds a test
case for it. Without this patch, calling timestamp_offset with a
negative value will panic with an overflow like this:

My editor (rust.vim) applies rustfmt automatically on save, which seems to have done some reformatting. Happy to back that out and reformat some other way if you prefer.

---- tests::test_parse_samples stdout ----
thread 'tests::test_parse_samples' panicked at 'attempt to multiply with
overflow',
/home/c/.cargo/registry/src/github.com-1ecc6299db9ec823/chrono-0.4.6/src/offset/mod.rs:349:34
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a
verbose backtrace.
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::print
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
   2: std::panicking::default_hook::{{closure}}
             at libstd/panicking.rs:211
   3: std::panicking::default_hook
             at libstd/panicking.rs:221
   4: std::panicking::rust_panic_with_hook
             at libstd/panicking.rs:477
   5: std::panicking::continue_panic_fmt
             at libstd/panicking.rs:391
   6: rust_begin_unwind
             at libstd/panicking.rs:326
   7: core::panicking::panic_fmt
             at libcore/panicking.rs:77
   8: core::panicking::panic
             at libcore/panicking.rs:52
   9: chrono::offset::TimeZone::timestamp_millis_opt
             at
/home/c/.cargo/registry/src/github.com-1ecc6299db9ec823/chrono-0.4.6/src/offset/mod.rs:349
  10: chrono::offset::TimeZone::timestamp_millis
             at
/home/c/.cargo/registry/src/github.com-1ecc6299db9ec823/chrono-0.4.6/src/offset/mod.rs:327

This patch fixes the case where a negative millisecond offset is passed
to Timezone::timestamp_millis and timestamp_millis_opt and adds a test
case for it. Without this patch, calling timestamp_offset with a
negative value will panic with an overflow like this:

```
---- tests::test_parse_samples stdout ----
thread 'tests::test_parse_samples' panicked at 'attempt to multiply with
overflow',
/home/c/.cargo/registry/src/github.com-1ecc6299db9ec823/chrono-0.4.6/src/offset/mod.rs:349:34
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a
verbose backtrace.
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::print
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
   2: std::panicking::default_hook::{{closure}}
             at libstd/panicking.rs:211
   3: std::panicking::default_hook
             at libstd/panicking.rs:221
   4: std::panicking::rust_panic_with_hook
             at libstd/panicking.rs:477
   5: std::panicking::continue_panic_fmt
             at libstd/panicking.rs:391
   6: rust_begin_unwind
             at libstd/panicking.rs:326
   7: core::panicking::panic_fmt
             at libcore/panicking.rs:77
   8: core::panicking::panic
             at libcore/panicking.rs:52
   9: chrono::offset::TimeZone::timestamp_millis_opt
             at
/home/c/.cargo/registry/src/github.com-1ecc6299db9ec823/chrono-0.4.6/src/offset/mod.rs:349
  10: chrono::offset::TimeZone::timestamp_millis
             at
/home/c/.cargo/registry/src/github.com-1ecc6299db9ec823/chrono-0.4.6/src/offset/mod.rs:327
```
cmars added a commit to cmars/prometheus-scrape that referenced this pull request Dec 2, 2018
@quodlibetor quodlibetor merged commit 77110ff into chronotope:master Dec 4, 2018
@quodlibetor
Copy link
Contributor

Awesome, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants