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

Restrict ISO parsing to be closer to standard #878

Merged
merged 1 commit into from
Oct 20, 2022

Conversation

berland
Copy link
Contributor

@berland berland commented Oct 20, 2022

ISO8601 requires exactly four digits for the year, while the current ISO parser in ecl allowed any number of digits. This commit will restrict the parser on this detail.

Issue
Resolves #877

Approach
Add an extra sscanf that must pass before an ISO date is validated.

@berland berland marked this pull request as ready for review October 20, 2022 07:56
@berland berland self-assigned this Oct 20, 2022
@berland berland added the bug label Oct 20, 2022
@ertomatic
Copy link
Collaborator

Can one of the admins verify this patch?

ISO8601 requires exactly four digits for the year, while the current ISO
parser in ecl allowed any number of digits. This commit will restrict
the parser on this detail.
@hnformentin
Copy link
Contributor

How do you think this will impact users? Should we add labels for releases-notes to communicate it?

@berland
Copy link
Contributor Author

berland commented Oct 20, 2022

How do you think this will impact users? Should we add labels for releases-notes to communicate it?

Not sure, the bug fixed by this is only present on Mac, on which we have no users, only developers (?) It will be mentioned in the release notes for ecl though.

Copy link
Contributor

@hnformentin hnformentin left a comment

Choose a reason for hiding this comment

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

LGTM! Good job identifying this issue.

@berland berland merged commit 93cc585 into equinor:main Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

ISO dates are parsed differently on mac vs linux
3 participants