-
Notifications
You must be signed in to change notification settings - Fork 347
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 int64 overflow in newNumericDateFromSeconds #112
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks to be a good alternative.
Thanks, can you merge it? I don't have such a possibility. |
Are you using 200y tokens, i.e., overflowing sometime in 2200's 😛 Thanks @PiotrKozimor |
Hello, I just debugged the same line locally :-) However, I think there is no need to extract nanoseconds, because the NewNumericDate function which is called truncates the time... with a TimePrecision of 1 second. Thus, I was going to create a PR that would simply have the following: func newNumericDateFromSeconds(f float64) *NumericDate {
return NewNumericDate(time.Unix(int64(f), 0))
} |
yep, in unit-tests, to be absolutely sure a token would never expire in tests. I know, it's a bit overkill, but well... |
You can configure the truncating precision. Default is second but there was a request to support adjusting it. So the original approach in this PR should be ok. |
Thanks! Indeed, if the truncating precision is meant to be modified, my suggestion is not okay 🙂 |
int64(f*float64(time.Second))
may easily overflow (this bug occured for me when validating token). I suggest here alternative approach to parseNumericDate
.