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

topdown: Fixing nanos int overflow issue for time.* built-in functions #4117

Conversation

johanfylling
Copy link
Contributor

The go Time.UnixNano() function result is undefined for dates that
cannot fit into an int64 when converted to Unix time in nanoseconds.

Updating built-in functions to return error if date is too low/high.

Fixes: #4098
Signed-off-by: Johan Fylling johan.dev@fylling.se

The go Time.UnixNano() function result is undefined for dates that
cannot fit into an int64 when converted to Unix time in nanoseconds.

Updating built-in functions to return error if date is too low/high.

Fixes: open-policy-agent#4098
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
topdown/time.go Outdated Show resolved Hide resolved
Adjusting min allowed time to be '1677-09-21 00:12:43.145224192'.

Signed-off-by: Johan Fylling <johan.dev@fylling.se>
@srenatus srenatus self-requested a review December 9, 2021 19:03
Copy link
Contributor

@srenatus srenatus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nitpicks inside 🙃

topdown/time.go Outdated Show resolved Hide resolved
topdown/time.go Outdated Show resolved Hide resolved
topdown/time.go Outdated Show resolved Hide resolved
Making nanosecond overflow error message more concise.
Updating relevant time.* built-ins to be registered with RegisterBuiltinFunc().

Signed-off-by: Johan Fylling <johan.dev@fylling.se>
srenatus
srenatus previously approved these changes Dec 10, 2021
Copy link
Contributor

@srenatus srenatus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Would we need a small note in the built-in docs? Just saying that it those built-ins turn into undefined if their result is outside the valid range...?

Updating docs for built-in functions.

Signed-off-by: Johan Fylling <johan.dev@fylling.se>
Copy link
Contributor

@srenatus srenatus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@johanfylling johanfylling merged commit edf5f25 into open-policy-agent:main Dec 10, 2021
@johanfylling johanfylling deleted the fix/4098/builtin-parse_rfc3339_ns_int_overflow branch December 15, 2021 09:45
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.

Integer overflow with parse_rfc3339_ns
2 participants