gh-83461: Don't allow datetime parsing to accept non-ASCII digits - #131008
gh-83461: Don't allow datetime parsing to accept non-ASCII digits#131008StanFromIreland wants to merge 30 commits into
Conversation
|
The original issue was marked with a security bug but I haven't looked at the entire thread so we might consider it a simple bug fix |
|
No it needs to be relabeled: -versions -type-security +type-bug |
picnixz
left a comment
There was a problem hiding this comment.
Is there a need to update the C implementation?
|
C calls the Python implementation. |
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
picnixz
left a comment
There was a problem hiding this comment.
I actually wonder but what about languages for which their input has non-ASCII strings such as Japanese?
picnixz
left a comment
There was a problem hiding this comment.
I think the ASCII flag restriction is too broad here as it applies to all formats, not just the digit part.
picnixz
left a comment
There was a problem hiding this comment.
Please carefully address the following suggestions and please make sure that typos lines are correctly formatted.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
Note: I'll review more this PR once I'm back because it will be easier when I'm on a laptop and not on mobile (so on Wednesday/Thursday) |
Oh I wasn't aware of this so thank you for the corrections. |
picnixz
left a comment
There was a problem hiding this comment.
Maybe in Lib/_striptime.py, you can add a small comment saying that the O* formats are locale-specific? (just above `for d in 'dmyHIMS'?
| calculations when the day of the week and the year are specified. | ||
|
|
||
| (5) | ||
| The :func:`strptime` function does not accept non-ASCII digits for numeric values. |
There was a problem hiding this comment.
Should we mention the "non-locale-specific numeric format codes" here or, since it's not officially supported, we can be a bit lazy?
There was a problem hiding this comment.
@pganssle Do you want to do this too? Or should I?
|
@pganssle friendly ping :-) |
pganssle
left a comment
There was a problem hiding this comment.
OK, this looks good to me now. Since @serhiy-storchaka asked us to wait on it, though, I'll leave it up to him as to when we merge it or if other changes are necessary.
|
Friendly ping @serhiy-storchaka |
|
Any update on this @serhiy-storchaka ? |
|
This PR is stale because it has been open for 30 days with no activity. |
|
@StanFromIreland, could you please update this PR? |
Documentation build overview
90 files changed ·
|
|
The new tests pass without the change: four of them raise >>> strptime('2025-02-29', '%Y-%m-%d') # day 29 must be in range 1..28 for month 2 in year 2025
>>> strptime('2555', '%G') # ISO year directive '%G' must be used with the ISO week directive '%V' ...
>>> strptime('2/02 02a2', '%j/%y %I%p:%M:%S') # time data does not match format
>>> strptime('02/2/2002', '%U/%V') # unconverted data remains: /2002Only These parse with ASCII digits, so the non-ASCII variants of them fail for the right reason: >>> strptime('2025-02-28', '%Y-%m-%d')
>>> strptime('2555 01 1', '%G %V %u')
>>> strptime('2/02 02AM', '%j/%y %I%p')
>>> strptime('02', '%U') |
Uh oh!
There was an error while loading. Please reload this page.