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

Fix out-of-range panics of Date #1075

Merged
merged 9 commits into from
Jan 20, 2021

Conversation

jevancc
Copy link
Contributor

@jevancc jevancc commented Jan 17, 2021

This Pull Request fixes/closes #1054

It changes the following:

  • Add function time_clip
  • Apply time_clip to methods of Date
  • Add overflow checks when creating and setting Date

This PR resolves all known and unexplored out-of-range panics - more stable now. However, the test mentioned in the issue still failed. That is because the valid date range of chrono::naive::NaiveDate in Rust is smaller than the range of Date in JavaScript, which makes the behavior different.

@codecov
Copy link

codecov bot commented Jan 17, 2021

Codecov Report

Merging #1075 (4b34f98) into master (1098b41) will increase coverage by 0.02%.
The diff coverage is 78.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1075      +/-   ##
==========================================
+ Coverage   58.68%   58.70%   +0.02%     
==========================================
  Files         176      176              
  Lines       12395    12407      +12     
==========================================
+ Hits         7274     7284      +10     
- Misses       5121     5123       +2     
Impacted Files Coverage Δ
boa/src/builtins/date/mod.rs 67.88% <78.33%> (-0.20%) ⬇️
boa/src/realm.rs 87.50% <0.00%> (-5.36%) ⬇️
boa/src/builtins/mod.rs 21.21% <0.00%> (-2.32%) ⬇️
boa/src/builtins/string/string_iterator.rs 76.31% <0.00%> (-0.61%) ⬇️
boa/src/builtins/map/map_iterator.rs 69.64% <0.00%> (-0.54%) ⬇️
boa/src/builtins/array/array_iterator.rs 78.72% <0.00%> (-0.45%) ⬇️
boa/src/builtins/object/for_in_iterator.rs 77.77% <0.00%> (-0.41%) ⬇️
boa/src/builtins/iterable/mod.rs 91.66% <0.00%> (-0.34%) ⬇️
boa/src/value/mod.rs 73.68% <0.00%> (+0.24%) ⬆️
boa/src/object/internal_methods.rs 57.08% <0.00%> (+0.35%) ⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1098b41...4b34f98. Read the comment docs.

@RageKnify
Copy link
Contributor

2 less panics as expected.

Test262 conformance changes:

Test result master count PR count difference
Total 78,497 78,497 0
Passed 24,805 24,805 0
Ignored 15,587 15,587 0
Failed 38,105 38,105 0
Panics 26 24 -2
Conformance 31.60 31.60 0.00%

@RageKnify RageKnify requested review from Razican and tofpie January 17, 2021 20:05
Copy link
Contributor

@tofpie tofpie left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks!

@RageKnify RageKnify added bug Something isn't working builtins PRs and Issues related to builtins/intrinsics labels Jan 17, 2021
@RageKnify RageKnify added this to the v0.12.0 milestone Jan 19, 2021
@RageKnify RageKnify merged commit db68c12 into boa-dev:master Jan 20, 2021
Razican pushed a commit that referenced this pull request May 22, 2021
* Add overflow check when fixing year, month, and day

* Add timeclip

* Add timeclip to `Date.UTC()`

* Minor fix

* Fix comment

* Add more range check

* Add timeclip to all date creation

* Remove redundant overflow check

* Fix clippy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working builtins PRs and Issues related to builtins/intrinsics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic: "invalid or out-of-range date"
4 participants