[Django] #32904: Tighten up the regular expression used by parse_time to accept less 'invalid' options.
Django
'invalid' options.
------------------------------------------------+------------------------
Reporter: Keryn Knight | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Utilities | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
As per discussion in the ticket #32892 and on the Github comments for
same, currently the `time_re` allows for some variations which it arguably
shouldn't.
For the historical record's sake, the current regex is:
`(?P<hour>\d{1,2}):(?P<minute>\d{1,2})(?::(?P<second>\d{1,2})(?:[\.,](?P<microsecond>\d{1,6})\d{0,6})?)?`
where you can see a whole lot of it ends up optional, and there are some
ways in which that can be made to accept what we'd probably call 'invalid'
(though strictly speaking the result is correct for the input portions):
{{{
>>> from django.utils.dateparse import parse_time
>>> parse_time('0:5: ')
datetime.time(0, 5)
}}}
If possible, we should derive examples of which strings might current pass
and decide which, if any of them, shouldn't be accepted. It's probably
also fine to leave the whole thing as-is (be liberal in what you accept
etc) and just add them as necessary to the examples of valid inputs, so in
future it doesn't come up again beyond "thats just an accepted quirk"
--
Ticket URL: <https://code.djangoproject.com/ticket/32904>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
'invalid' options.
Reporter: Keryn Knight | Owner: nobody
Cleanup/optimization |
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Keryn Knight):
Here, for example, is one which parses into a `datetime.time` but I
wouldn't really expect it to, and whilst the input is nonsense, doesn't
cause an error like `ValueError: second must be in 0..59` which ''would''
match my expectations:
{{{
>>> from django.utils.dateparse import parse_time
>>> parse_time('4:18:101')
datetime.time(4, 18, 10)
# captured data was {'hour': '4', 'minute': '18', 'second': '10',
'microsecond': None}
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32904#comment:1>
Django
'invalid' options.
Reporter: Keryn Knight | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Utilities | Version: dev
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Unreviewed => Accepted
Comment:
IMO the main issue is that `$` is missing.
--
Ticket URL: <https://code.djangoproject.com/ticket/32904#comment:2>
Django
'invalid' options.
Reporter: Keryn Knight | Owner: nobody
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* type: Cleanup/optimization => Bug
--
Ticket URL: <https://code.djangoproject.com/ticket/32904#comment:3>
Django
'invalid' options.
Reporter: Keryn Knight | Owner: nobody
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Nick Pope):
Replying to [comment:2 Mariusz Felisiak]:
> IMO the main issue is that `$` is missing.
I came to the same conclusion:
https://github.com/django/django/pull/14582#discussion_r664075337
--
Ticket URL: <https://code.djangoproject.com/ticket/32904#comment:4>
Django
'invalid' options.
Reporter: Keryn Knight | Owner: Abhyudai
Type: Bug | Status: assigned
Component: Utilities | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Abhyudai):
* owner: nobody => Abhyudai
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/32904#comment:5>
Django
'invalid' options.
Reporter: Keryn Knight | Owner: Abhyudai
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Abhyudai):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/14602/ pull request]
--
Ticket URL: <https://code.djangoproject.com/ticket/32904#comment:6>
Django
'invalid' options.
Reporter: Keryn Knight | Owner: Abhyudai
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/32904#comment:7>
Django
'invalid' options.
Reporter: Keryn Knight | Owner: Abhyudai
Component: Utilities | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"cf6774a53b40243d35183b4300a9385b68fd8c75" cf6774a5]:
{{{
#!CommitTicketReference repository=""
revision="cf6774a53b40243d35183b4300a9385b68fd8c75"
Fixed #32904 -- Made parse_time() more strict.
Thanks Keryn Knight for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32904#comment:8>