Code

#18728 closed Bug (fixed)

dateparse datetime regex should accept colon in TZ offset

Reported by: thaxter Owned by: aaugustin
Component: Core (Other) Version: 1.4
Severity: Normal Keywords: datetime dateparse iso8601
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

A legitimate ISO8601 datetime such as 2012-08-07T15:17:59.000+0000 will be rejected as input to a DateTime field. The dateparse.datetime_re matches with the colon: 2012-08-07T15:17:59.000+00:00.

According to spec, both are acceptable. Simply adding a ? after the colon in the regex fixes it:

datetime_re = re.compile(
    r'(?P<year>\d{4})-(?P<month>\d{1,2})-(?P<day>\d{1,2})'
    r'[T ](?P<hour>\d{1,2}):(?P<minute>\d{1,2})'
    r'(?::(?P<second>\d{1,2})(?:\.(?P<microsecond>\d{1,6})\d{0,6})?)?'
    r'(?P<tzinfo>Z|[+-]\d{1,2}:?\d{1,2})?$'
)

Diff format:

21c21
<     r'(?P<tzinfo>Z|[+-]\d{1,2}:\d{1,2})?$'
---
>     r'(?P<tzinfo>Z|[+-]\d{1,2}:?\d{1,2})?$'

Attachments (1)

django-dateparse.patch (916 bytes) - added by jason@… 20 months ago.
patch file for this issue, also found at github thaxter/django

Download all attachments as: .zip

Change History (6)

comment:1 Changed 21 months ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to aaugustin
  • Patch needs improvement unset

comment:2 Changed 21 months ago by thaxter

  • Easy pickings set

The patch needs an enhancement. When tzinfo is converted to an offset, it uses string subscripting to a string that is now variable/different length. This patch removes the colon and changes the subscript in addition to updating the regex. This prevents a bug that would occur when the minutes in the offset are not zero.

21c21
<     r'(?P<tzinfo>Z|[+-]\d{1,2}:\d{1,2})?$'
---
>     r'(?P<tzinfo>Z|[+-]\d{1,2}:?\d{1,2})?$'
79c79,80
<             offset = 60 * int(tzinfo[1:3]) + int(tzinfo[4:6])
---
>             tzinfo = re.sub(r':','',tzinfo)
>             offset = 60 * int(tzinfo[1:3]) + int(tzinfo[3:5])

This demo code shows that it works.

from django.utils.dateparse import parse_datetime

foo = '2012-08-06T01:00:00+01:02'
bar = '2012-08-06T01:00:00+0102'

print parse_datetime(foo).isoformat()
print parse_datetime(bar).isoformat()


Output:

2012-08-06T01:00:00+01:02
2012-08-06T01:00:00+01:02

Changed 20 months ago by jason@…

patch file for this issue, also found at github thaxter/django

comment:3 Changed 20 months ago by aaugustin

How is Django supposed to interpret "+123" -- which matches the regexp with your change?

comment:4 Changed 20 months ago by aaugustin

  • Component changed from Uncategorized to Core (Other)
  • Easy pickings unset
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Bug

That said, the current function won't work if the hours and minutes in the offset aren't specified with 2 digits each, and that's a problem we have to fix one way or another.

The two issues are tightly related.

comment:5 Changed 20 months ago by Aymeric Augustin <aymeric.augustin@…>

  • Resolution set to fixed
  • Status changed from new to closed

In [2f59e94a4150c84c8b4e387fd82f0509eadc4f4b]:

Fixed #18728 -- Made colon optional in tzinfo

Made two-digit hours and minutes mandatory in tzinfo (the code used
to crash if a one-digit representation was provided).

Added standalone tests for django.utils.dateparse.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.