Skip to content

fix: Validate constructed date has year within typemin/max #54933

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

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

mcmcgrath13
Copy link
Contributor

Check that a constructed Date has a year that's within the range of the type's min and max values.

Closes #50981

@dkarrasch dkarrasch added the dates Dates, times, and the Dates stdlib module label Jun 25, 2024
@mcmcgrath13 mcmcgrath13 marked this pull request as draft June 25, 2024 23:41
Copy link
Member

@LilithHafner LilithHafner left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, this looks mostly mergeable to me, but I'm not sure what the len changes are for.

Comment on lines +18 to +24
hi -= st # Prevent overshooting
while v <= hi && prev <= v
prev = v
v += st
i += 1
end
return i - 1
return i
Copy link
Member

Choose a reason for hiding this comment

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

Why is this change necessary?

Copy link
Contributor Author

@mcmcgrath13 mcmcgrath13 Aug 12, 2024

Choose a reason for hiding this comment

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

ah, this is still a work in progress to figure out how to do this. The range implementations currently rely on being able to go past the actual end of the supported range, and now that that errors, this needs to be rethought (it doesn't quite work correctly yet)

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps it would help to extend the constructable dates from typemin(Date):typemax(Date) to the slightly larger set of representable dates typemin(Date)-Day(161):typemax(Date)+Day(280)

Comment on lines +252 to +253
const DATETIME_YEAR_TYPEMAX = 146138512
const DATETIME_YEAR_TYPEMIN = -146138511
Copy link
Member

Choose a reason for hiding this comment

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

I appreciate factoring out these magic numbers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dates Dates, times, and the Dates stdlib module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Date and DateTime should reject year inputs that they can't store
4 participants