Re: [Django] #33589: Incomplete string escaping in formats for calendar.
Django
-------------------------------------+-------------------------------------
Reporter: Shrikant Dhayje | Owner: Shrikant
| Dhayje
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
bug,regex,Inaccurate |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by fabtjar):
Replying to [comment:1 Shrikant Dhayje]:
> "Has patch" https://github.com/django/django/pull/15460
Hey, are you still working on this ticket? The PR was closed in April due
to inactivity. Maybe someone else can take it.
--
Ticket URL: <https://code.djangoproject.com/ticket/33589#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Shrikant Dhayje | Owner: (none)
Type: Bug | Status: new
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Mariusz Felisiak):
* owner: Shrikant Dhayje => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/33589#comment:4>
Django
Reporter: Shrikant Dhayje | Owner: Ankur Roy
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution:
bug,regex,Inaccurate |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Ankur Roy):
* owner: (none) => Ankur Roy
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/33589#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Shrikant Dhayje | Owner: Ankur Roy
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
bug,regex,Inaccurate |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Fab):
Replying to [comment:2 Mariusz Felisiak]:
> Seems reasonable. Can you add a regression test? Do you have an example
of the format with multiple occurrence of the same char that should be
escaped?
I've looked and can't find any `DATE_INPUT_FORMATS` that contain escape
characters in any of the languages. If they are not used should we just
remove the replace?
--
Ticket URL: <https://code.djangoproject.com/ticket/33589#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Shrikant Dhayje | Owner: Ankur Roy
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
bug,regex,Inaccurate |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: David Wobrock (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/33589#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Shrikant Dhayje | Owner: Ankur Roy
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
bug,regex,Inaccurate |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by SanderBeekhuis):
Hi all, just chiming in without much django contribution experience
One can, in theory, set any `DATE_INPUT_FORMATS` in ones `settings.py`
I set the following
{{{
USE_L10N = False
DATE_INPUT_FORMATS = ["%Y-'''--%m-\t-%d"]
}}}
in my settings.py. It allows me to test the code paths discussed in this
patch. If I use admin date picker to pick dates they are formatted using
the logic in this JavaScript file.
However even with a single `'` or `\t` in the format the behavior does not
seem work in a very sensible way. Improving them would take more then the
proposed patch. So dropping these replaces might make the most sense.
When I get time to work on this issue again I can try write to write a JS
tests for this in the future. If that would be nice?
--
Ticket URL: <https://code.djangoproject.com/ticket/33589#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Shrikant Dhayje | Owner: Ankur Roy
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
bug,regex,Inaccurate |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: SanderBeekhuis (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/33589#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Shrikant Dhayje | Owner: Ankur Roy
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
bug,regex,Inaccurate |
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Sander Beekhuis):
I also only now see this has been worked on in
https://github.com/django/django/pull/15761/files#diff-
c28a20f3291264125c6c8083e1c44075bf4e75e37bddd26a43c20ddc0a5fe837 as well.
Don't put to much weight on my statments. :)
--
Ticket URL: <https://code.djangoproject.com/ticket/33589#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Shrikant Dhayje | Owner: Ankur Roy
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
bug,regex,Inaccurate |
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/33589#comment:11>
Django
-------------------------------------+-------------------------------------
Reporter: Shrikant Dhayje | Owner: Ankur Roy
Type: Bug | Status: assigned
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution:
bug,regex,Inaccurate | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/33589#comment:12>
Django
-------------------------------------+-------------------------------------
Reporter: Shrikant Dhayje | Owner: Ankur Roy
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
bug,regex,Inaccurate | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):
* status: assigned => closed
* resolution: => invalid
* stage: Ready for checkin => Unreviewed
Comment:
Replying to [comment:8 Sander Beekhuis]:
> Hi all, just chiming in without much django contribution experience
>
> One can, in theory, set any `DATE_INPUT_FORMATS` in ones `settings.py`
>
> I set the following
>
> {{{
> USE_L10N = False
> DATE_INPUT_FORMATS = ["%Y-'''--%m-\t-%d"]
> }}}
>
> in my settings.py. It allows me to test the code paths discussed in this
patch. If I use admin date picker to pick dates they are formatted using
the logic in this JavaScript file.
>
> However even with a single `'` or `\t` in the format the behavior does
not seem work in a very sensible way. Improving them would take more then
the proposed patch. So dropping these replaces might make the most sense.
>
> When I get time to work on this issue again I can try write to write a
JS tests for this in the future. If that would be nice?
Ah yes, we should drop replacements. It was added in
fa0653cd1d791a8bce835e8992cbeab6fd70d0e7 where we created a callback
function by concatenating strings. It's unnecessary and buggy since
d638cdc42acec608c1967f44af6be32a477c239f.
--
Ticket URL: <https://code.djangoproject.com/ticket/33589#comment:13>