NumericDate.MarshalJSON overflows on large unix timestamps

[qqiao]

Marshalling NewNumericDate(time.Unix(253402271999, 0)) will result in -4852145033 instead of 253402271999

Updates:
This is due to the fact that Time.UnixNano() overflows, and NumericDate.MarshalJSON() internally uses this method.

PR #200 created to workaround this std library limitation.

[mfridman]

I can't seem to reproduce this on my local machine. Can you provide a repro (ideally as a test that runs in CI)? Also, can you specify the system you're on and arch?

package main

import (
	"fmt"
	"time"

	"github.com/golang-jwt/jwt/v4"
)

func main() {
	original := time.Unix(253402271999, 0)
	date := jwt.NewNumericDate(original)
	fmt.Println(date.Unix())
	// 253402271999
}

[qqiao]

This will consistently fail on a M1 Mac:

func TestNumericDate_MarshalJSON(t *testing.T) {
	const ts = 253402271999
	expected := fmt.Sprintf("%d", ts)
	date := jwt2.NewNumericDate(time.Unix(ts, 0))

	b, err := date.MarshalJSON()
	if err != nil {
		t.Errorf("Failed to marshal numeric date %s. Error: %v",
			date.String(), err)
	}
	got := fmt.Sprintf("%s", b)

	if got != expected {
		t.Errorf("Expected: %s, got: %s", expected, got)
	}
}

Output would be Expected: 253402271999, got: -4852145033

[qqiao]

I figured out the reason: the timestamp is too large, which causes UnixNano to overflow. Since MarshalJSON uses UnixNano, it fails too. I'll see if there's a workaround.

[qqiao]

PR #200 created, @mfridman could you help review?

[mfridman]

Thanks for providing a repro and submitting a PR.

Indeed that's the result of UnixNano, which makes this valid up until 2262. Would using UnixMicro be more practical?

[qqiao]

Using UnixMicro would work for more cases than using UnixNano, however, it does not attack the issue at its root.

The issue with using either nano or micro is that they overflow at some point, which leaves us with cases where a valid time.Time cannot be correctly marshalled.

Internally, the time.Time implementation stores seconds and nano seconds separately, the PR that I submitted took advantage of that, by formatting them separately and concatinating the strings. That way, any valid time.Time would get marshalled correctly.

The other note-worthy issue with directly taking either unixnano or unixmicro is that Go tag on a small delta value in order to achieve monotonic time, that is to ensure that the following snippet always work:

time1 := time.Unix(0, 0)
time2 := time.Unix(0, 0)
time1.Before(time2) // results in true

When we use the value from UnixNano, it returns the original time in nanoseconds plus this tiny delta, as we see in some of the expected results in type_test.go, when we go down to the nanosecond time precision, we would notice that with the current implementation, some of the expected results are off by a few nanoseconds.

However, the PR that I submitted avoids this issue since the value returned by Time.Nanosecond() would not contain that small delta, it makes the result precise and preditable.

To summarize, by treating the second component and nanosecond component of a time.Time separately, we will be able to accurately marshal any valid time.Time instance, with no caveats.