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: Replace MakeDay/MakeDate with ISODateToEpochDays/EpochDaysToEpochMs #2729

Closed
anba opened this issue Dec 11, 2023 · 2 comments
Closed

GetUTCEpochNanoseconds: Replace MakeDay/MakeDate with ISODateToEpochDays/EpochDaysToEpochMs #2729

anba opened this issue Dec 11, 2023 · 2 comments
Assignees
@ptomato

Comments

@anba
Copy link
Contributor

anba commented Dec 11, 2023

Continuation of #2315: GetUTCEpochNanoseconds can be called with user-defined year, which can be too large for MakeDay/MakeDate. Instead ISODateToEpochDays and EpochDaysToEpochMs have to be used in GetUTCEpochNanoseconds.
@ptomato
Copy link
Collaborator

ptomato commented Dec 13, 2023

I'd like to avoid rewriting GetUTCEpochNanoseconds in ECMA-262 because until the question about large Number values (tc39/ecma262#1087) is clarified, we don't know whether that would be a normative change for legacy Date objects.

As far as I can tell, the places where this potentially happens is in ISODateTimeWithinLimits and the string branch of ToTemporalInstant. The other places where GetUTCEpochNanoseconds is called:

  • InterpretISODateTimeOffset → called with user-defined year, but CreateTemporalDateTime in step 4 already throws if the year is out of range.
  • Temporal.TimeZone.prototype.getPossibleInstantsFor → called with values from internal slots
  • DisambiguatePossibleInstants → called with values from internal slots

So I think we can handle this with an extra "If abs(ISODateToEpochDays(year, month - 1, day)) > 108" check in ISODateTimeWithinLimits and ToTemporalInstant. Does that sound good to you?

@ptomato ptomato self-assigned this Dec 13, 2023
ptomato added a commit that referenced this issue Dec 13, 2023
@ptomato
Editorial: Avoid hitting assertion in GetUTCEpochNanoseconds
f46e302 
GetUTCEpochNanoseconds has an assertion that MakeDate, MakeTime, and
MakeDay all result in a finite value. However, several places call
GetUTCEpochNanoseconds with a year value that may be excessively large,
notably from parsing an ISO string such as +999999-01-01, which would
cause that assertion to be hit.

Add checks in the appropriate places to make sure that the year value is
not excessively large.

Closes: #2729
@anba
Copy link
Contributor Author

anba commented Dec 13, 2023

Ah, I forgot that GetUTCEpochNanoseconds is defined in ECMA-262. Your idea to use ISODateToEpochDays sounds good to me.

@Ms2ger Ms2ger closed this as completed in c4b17d8 Jan 8, 2024
@anba anba mentioned this issue Sep 30, 2024
Closed
@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
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ptomato @anba