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.

Sign up for GitHub

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

Jump to bottom

GetUTCEpochNanoseconds: Missing checks for too large dates #2985

Closed
anba opened this issue Sep 30, 2024 · 2 comments
Closed

GetUTCEpochNanoseconds: Missing checks for too large dates #2985

anba opened this issue Sep 30, 2024 · 2 comments
Assignees
@ptomato
Labels
editorial spec-text Specification text involved

Comments

@anba
Copy link
Contributor

anba commented Sep 30, 2024

It's not valid to call GetUTCEpochNanoseconds with too large dates, see #2729.

When time zones and calendars were removed, some CreateTemporalDate and CreateTemporalDateTime calls were also replaced with ISO date (-time) records, which don't perform the necessary checks to ensure all inputs are in range.

All calls to GetUTCEpochNanoseconds and GetNamedTimeZoneEpochNanoseconds need to be rechecked to ensure that no too large inputs are possible.
@ptomato ptomato self-assigned this Sep 30, 2024
@ptomato ptomato added spec-text Specification text involved editorial labels Sep 30, 2024
ptomato added a commit that referenced this issue Oct 1, 2024
@ptomato
Polyfill: 0-based month in ISODateToEpochDays
cf046bb 
While investigating #2985 I noticed that ISODateToEpochDays was defined
differently between the spec text and the reference code. Use 0-based
months as in the spec text, to avoid confusion when comparing the two.

Also I noticed that it can now be implemented more efficiently with
GetUTCEpochMilliseconds.
ptomato added a commit that referenced this issue Oct 1, 2024
@ptomato
Editorial: Adjust RangeError condition in ToTemporalInstant
43bc062 
We must avoid calling GetUTCEpochNanoseconds with dates that are too large
because it is ill-defined in ECMA-262 what is supposed to happen. (See
tc39/ecma262#1087). We had a condition in
ToTemporalInstant that was supposed to prevent this, but was incorrect
because subtracting the UTC offset could push the date out of range.

Note that this is editorial, because IsValidEpochNanoseconds would throw
anyway in this case even if GetUTCEpochNanoseconds was fully defined for
large inputs.

See: #2985
@ptomato ptomato mentioned this issue Oct 1, 2024
Merged
ptomato added a commit that referenced this issue Oct 2, 2024
@ptomato
Polyfill: 0-based month in ISODateToEpochDays
5eff6c4 
While investigating #2985 I noticed that ISODateToEpochDays was defined
differently between the spec text and the reference code. Use 0-based
months as in the spec text, to avoid confusion when comparing the two.

Also I noticed that it can now be implemented more efficiently with
GetUTCEpochMilliseconds.
ptomato added a commit that referenced this issue Oct 2, 2024
@ptomato
Editorial: Adjust RangeError condition in ToTemporalInstant
e37c33a 
We must avoid calling GetUTCEpochNanoseconds with dates that are too large
because it is ill-defined in ECMA-262 what is supposed to happen. (See
tc39/ecma262#1087). We had a condition in
ToTemporalInstant that was supposed to prevent this, but was incorrect
because subtracting the UTC offset could push the date out of range.

Note that this is editorial, because IsValidEpochNanoseconds would throw
anyway in this case even if GetUTCEpochNanoseconds was fully defined for
large inputs.

See: #2985
ptomato added a commit to ptomato/test262 that referenced this issue Oct 3, 2024
@ptomato
Temporal: Consistently test all string entry points for representable…
6fa96d4 
… range

Remove the existing "instant-string-limits.js" that only applied to APIs
where ToTemporalInstant was called. Add "argument-string-limits.js" tests
everywhere ISO strings are converted.

Related to tc39/proposal-temporal#2985
ptomato added a commit to ptomato/test262 that referenced this issue Oct 3, 2024
@ptomato
Temporal: Additional tests for cases where date addition goes out of …
f0e0411 
…range

See tc39/proposal-temporal#2985
@ptomato ptomato mentioned this issue Oct 3, 2024
Merged
@ptomato
Copy link
Collaborator

ptomato commented Oct 3, 2024

I audited all the uses of GetUTCEpochNanoseconds and GetNamedTimeZoneEpochNanoseconds and I'm confident that the commits in #2989 fix them. I've added more comprehensive test coverage for these cases in tc39/test262#4245.

ptomato added a commit to ptomato/test262 that referenced this issue Oct 3, 2024
@ptomato
Temporal: Consistently test all string entry points for representable…
a9c026b 
… range

Remove the existing "instant-string-limits.js" that only applied to APIs
where ToTemporalInstant was called. Add "argument-string-limits.js" tests
everywhere ISO strings are converted.

Related to tc39/proposal-temporal#2985
ptomato added a commit to ptomato/test262 that referenced this issue Oct 3, 2024
@ptomato
Temporal: Additional tests for cases where date addition goes out of …
b490361 
…range

See tc39/proposal-temporal#2985
Ms2ger pushed a commit that referenced this issue Oct 3, 2024
@ptomato @Ms2ger
Polyfill: 0-based month in ISODateToEpochDays
981ab0f 
While investigating #2985 I noticed that ISODateToEpochDays was defined
differently between the spec text and the reference code. Use 0-based
months as in the spec text, to avoid confusion when comparing the two.

Also I noticed that it can now be implemented more efficiently with
GetUTCEpochMilliseconds.
Ms2ger pushed a commit that referenced this issue Oct 3, 2024
@ptomato @Ms2ger
Editorial: Adjust RangeError condition in ToTemporalInstant
a2a16bc 
We must avoid calling GetUTCEpochNanoseconds with dates that are too large
because it is ill-defined in ECMA-262 what is supposed to happen. (See
tc39/ecma262#1087). We had a condition in
ToTemporalInstant that was supposed to prevent this, but was incorrect
because subtracting the UTC offset could push the date out of range.

Note that this is editorial, because IsValidEpochNanoseconds would throw
anyway in this case even if GetUTCEpochNanoseconds was fully defined for
large inputs.

See: #2985
Ms2ger pushed a commit to tc39/test262 that referenced this issue Oct 3, 2024
@ptomato @Ms2ger
Temporal: Consistently test all string entry points for representable…
326cdb8 
… range

Remove the existing "instant-string-limits.js" that only applied to APIs
where ToTemporalInstant was called. Add "argument-string-limits.js" tests
everywhere ISO strings are converted.

Related to tc39/proposal-temporal#2985
Ms2ger pushed a commit to tc39/test262 that referenced this issue Oct 3, 2024
@ptomato @Ms2ger
Temporal: Additional tests for cases where date addition goes out of …
f4a987f 
…range

See tc39/proposal-temporal#2985
@ptomato
Copy link
Collaborator

ptomato commented Oct 3, 2024

Closed by #2989.

@ptomato ptomato closed this as completed Oct 3, 2024
@ptomato ptomato mentioned this issue Oct 8, 2024
Merged
ptomato added a commit that referenced this issue Oct 8, 2024
@ptomato
Editorial: Abstract days range check into CheckISODaysRange
4fdf028 
This pattern is there because of #2729 and #2985. Abstract it into its own
operation, for documentation purposes and because it can disappear after
tc39/ecma262#1087 is fixed.

See: #3005
ptomato added a commit that referenced this issue Oct 9, 2024
@ptomato
Editorial: Abstract days range check into CheckISODaysRange
14e85b6 
This pattern is there because of #2729 and #2985. Abstract it into its own
operation, for documentation purposes and because it can disappear after
tc39/ecma262#1087 is fixed.

See: #3005
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ptomato ptomato

Labels
editorial spec-text Specification text involved
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ptomato @anba