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 inconsistencies in date check #2070

Merged
merged 1 commit into from
Nov 18, 2023
Merged

Fix inconsistencies in date check #2070

merged 1 commit into from
Nov 18, 2023

Conversation

Famlam
Copy link
Collaborator

@Famlam Famlam commented Nov 14, 2023

  1. Although the regex checks deny any date past the year 2999, the convert2date function check (which is called if the regex checks fail) still allowed any year except 9999. As there's no logical reason to expect any years >=3000 in OSM, mark them as bad too. (Reason: in the Dutch community we spotted some very strange years in the start_date tag of buildings, containing start_dates far in the future)

  2. Add some extra safety for the "~", "s" and " BC" year checks. If the string consists only of those characters it should be treated as an invalid date. (The check function returns True on empty strings due to the handling of e.g. 1920.., so a string ~ would return True after stripping the ~)

1. Although the regex checks deny any date past the year 2999, the convert2date function check still allowed any year except 9999. As there's no logical reason to expect any years >=3000 in OSM, mark them as bad too. (Reason: in the Dutch community we spotted some very strange years in the `start_date` tag of buildings)

2. Add some extra safety for the "~", "s" and " BC" year checks. If the string consists only of those characters it should be treated as an invalid date. (The check function returns True on empty strings due to the handling of e.g. `1920..`, so a string `~` would return True after stripping the `~`)
@Famlam Famlam marked this pull request as draft November 14, 2023 22:03
@Famlam
Copy link
Collaborator Author

Famlam commented Nov 14, 2023

Hmm, I see that 2019-99-77 is also considered a valid date by the regexes, despite that there is no 99th month. @frodrigo I can fix the regexes, but is there a reason why we don't just call convert2date all the time? (E.g. dropping the regexes)

@Famlam Famlam marked this pull request as ready for review November 16, 2023 22:12
@Famlam
Copy link
Collaborator Author

Famlam commented Nov 16, 2023

I'll fix the month/day check in a separate PR. This one is ready for review (next PR with the fix for invalid months is ready on my computer ;) )

@frodrigo frodrigo merged commit b1ce311 into osm-fr:dev Nov 18, 2023
3 checks passed
@frodrigo
Copy link
Member

Thank you

@Famlam Famlam deleted the date branch November 18, 2023 15:14
Famlam added a commit to Famlam/osmose-backend that referenced this pull request Nov 18, 2023
Instead of fixing the regexes, just let dateutil take care of it

See osm-fr#2070 (comment)
frodrigo pushed a commit that referenced this pull request Nov 18, 2023
Instead of fixing the regexes, just let dateutil take care of it

See #2070 (comment)
tykayn pushed a commit to vortex-a-chats/osmose-backend that referenced this pull request Dec 11, 2023
Instead of fixing the regexes, just let dateutil take care of it

See osm-fr#2070 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants