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

tai64: wrong results -- 10s is not right offset and doesn't account for past or future leap seconds in SystemTime conversion #675

Open
programmerjake opened this issue Jun 17, 2022 · 9 comments

Comments

@programmerjake
Copy link

programmerjake commented Jun 17, 2022

The offset between UTC time and TAI time isn't currently 10s, it's 37s and may change to something other than 37s in the future.

The current code assumes the offset is always 10s.

@programmerjake programmerjake changed the title tai64 crate doesn't account for past or future leap seconds in SystemTime conversion tai64 crate gives wrong result -- 10s is not right offset and doesn't account for past or future leap seconds in SystemTime conversion Jun 19, 2022
@programmerjake
Copy link
Author

comparison on my system (after installing ntp so linux has the correct offset):
note how the result from linux and Tai64N::now() differ -- linux: 1655618465 vs. tai64: 1655618438 -- SystemTime: 1655618428
code:

use tai64::Tai64N;
use std::time::SystemTime;
use libc::{clock_gettime, CLOCK_TAI, timespec};

fn now_tai() -> std::io::Result<timespec> {
    let mut retval = timespec { tv_sec: 0, tv_nsec: 0 };
    let r = unsafe { clock_gettime(CLOCK_TAI, &mut retval) };
    if r != 0 {
        Err(std::io::Error::last_os_error())
    } else {
        Ok(retval)
    }
}

fn main() {
    let t = dbg!(Tai64N::now());
    let t = (t.0.0 as i64).wrapping_sub(1 << 62);
    dbg!(t);
    dbg!(SystemTime::now());
    let tai = now_tai().unwrap();
    dbg!(tai.tv_sec);
    dbg!(tai.tv_nsec);
}

output:

[src/main.rs:16] Tai64N::now() = Tai64N(
    Tai64(
        4611686020083006342,
    ),
    259280636,
)
[src/main.rs:18] t = 1655618438
[src/main.rs:19] SystemTime::now() = SystemTime {
    tv_sec: 1655618428,
    tv_nsec: 259298279,
}
[src/main.rs:21] tai.tv_sec = 1655618465
[src/main.rs:22] tai.tv_nsec = 259305372

@programmerjake
Copy link
Author

@tarcieri
Copy link
Member

The implementation is indeed naive/broken. I thought we had an open tracking issue for this, but apparently not.

I'm not sure the best way to handle this, especially given the existing published crates.

@programmerjake
Copy link
Author

how about downloading the official leap-second table and converting it to rust in a build.rs? you could have that enabled by an enabled-by-default feature.

apparently an official source is https://www.ietf.org/timezones/data/leap-seconds.list

@tarcieri
Copy link
Member

I'd probably suggest against making network connections from a build script, especially in any security-related package (indeed there are renewed efforts underway to run build scripts under WASM).

However, I think it would be okay to use AOT conversion of the table, leveraging the build script as a sort of "time bomb" which makes the crate fail to compile after the table expires.

Or we could add some sort of automation to yank the crate releases containing expired tables.

@programmerjake
Copy link
Author

for wireguard compatibility, you could add a disabled-by-default feature assume-incorrect-10s-offset and release a new major version, or maybe instead just add a now_with_assumed_offset function (and similar for other UTC-based interfaces).

@programmerjake
Copy link
Author

I'd probably suggest against making network connections from a build script, especially in any security-related package (indeed there are renewed efforts underway to run build scripts under WASM).

ok, at least on linux you can usually get an up-to-date table from the tzdata package which is usually installed by default:
/usr/share/zoneinfo/leap-seconds.list

@programmerjake
Copy link
Author

@tarcieri
Copy link
Member

Perhaps it could try to query OS facilities to find the table, and if it can't, fall back on an internal table.

@tarcieri tarcieri changed the title tai64 crate gives wrong result -- 10s is not right offset and doesn't account for past or future leap seconds in SystemTime conversion tai64: wrong results -- 10s is not right offset and doesn't account for past or future leap seconds in SystemTime conversion Aug 18, 2024
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

No branches or pull requests

2 participants