[Django] #29667: path converters don't handle spaces well.

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[Django] #29667: path converters don't handle spaces well.

Django
#29667: path converters don't handle spaces well.
-------------------------------------+-------------------------------------
               Reporter:  Keryn      |          Owner:  nobody
  Knight                             |
                   Type:  Bug        |         Status:  new
              Component:  Core       |        Version:  master
  (URLs)                             |       Keywords:  converters path
               Severity:  Normal     |  _route_to_regex
           Triage Stage:             |      Has patch:  0
  Unreviewed                         |
    Needs documentation:  0          |    Needs tests:  0
Patch needs improvement:  0          |  Easy pickings:  0
                  UI/UX:  0          |
-------------------------------------+-------------------------------------
 This came up for someone on IRC last week, but I can't see that they
 raised a ticket about it.

 Correct:
 {{{
 >>> from django.urls.resolvers import _route_to_regex
 >>> _route_to_regex("<uuid:test>")
 ('^(?P<test>[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})',
  {'test': <django.urls.converters.UUIDConverter at 0x1055e8c88>})
 }}}

 Also working correctly:
 {{{
 >>> from django.urls.resolvers import _route_to_regex
 >>> _route_to_regex("<uuid:2>")
 ImproperlyConfigured: URL route '<uuid:2>' uses parameter name '2' which
 isn't a valid Python identifier.
 }}}

 however, constructing a valid **looking** converter reference apparently
 hits neither the happy nor the unhappy path, and also I presume passes any
 system checks in place that might otherwise warn about the sensitivity:
 {{{
 >>> from django.urls.resolvers import _route_to_regex
 >>> _route_to_regex("<uuid: test>")  # note the preceeding space
 ('^\\<uuid\\:\\ test\\>', {})
 }}}
 - the regex is invalid
 - the kwargs dictionary is (sort of rightly) empty.
 - the same is true with "test " and "te st"

 Unless I'm misunderstanding the code therein, "test ", " test" and "te st"
 should all be hitting the invalid identifier part, and personally I feel
 like leading/trailing spaces at least could just be sanitised (stripped)
 as they're almost certainly accidental or for display purposes.

 Tested in a shell against master @
 7eb556a6c2b2ac9313158f8b812eebea02a43f20.

--
Ticket URL: <https://code.djangoproject.com/ticket/29667>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/052.7c3cdca8ec8089ff68b84007c546d99e%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29667: path converters don't handle spaces well.

Django
#29667: path converters don't handle spaces well.
-------------------------------------+-------------------------------------
     Reporter:  Keryn Knight         |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Core (URLs)          |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  converters path      |             Triage Stage:  Accepted
  _route_to_regex                    |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Carlton Gibson):

 * stage:  Unreviewed => Accepted


Comment:

 Thanks for the report Keryn. This looks reasonable.

--
Ticket URL: <https://code.djangoproject.com/ticket/29667#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.702e3ff49d4f5cf02421f32e5d0c0902%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29667: path converters don't handle spaces well.

Django
In reply to this post by Django
#29667: path converters don't handle spaces well.
-------------------------------------+-------------------------------------
     Reporter:  Keryn Knight         |                    Owner:  Jeff
         Type:  Bug                  |                   Status:  assigned
    Component:  Core (URLs)          |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  converters path      |             Triage Stage:  Accepted
  _route_to_regex                    |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Jeff):

 * owner:  nobody => Jeff
 * status:  new => assigned


Comment:

 happy to look into this.

--
Ticket URL: <https://code.djangoproject.com/ticket/29667#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.e8f880516dcf57306a96a4e2b3772658%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29667: path converters don't handle spaces well.

Django
In reply to this post by Django
#29667: path converters don't handle spaces well.
-------------------------------------+-------------------------------------
     Reporter:  Keryn Knight         |                    Owner:  Jeff
         Type:  Bug                  |                   Status:  assigned
    Component:  Core (URLs)          |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  converters path      |             Triage Stage:  Accepted
  _route_to_regex                    |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Tom Forbes):

 I ran into this recently and ended up diagnosing the failure so I really
 hope you do not mind me jumping in and offering a PR Jeff. The path
 parameter regex was just a bit too strict about parsing spaces, and so
 ended up ignoring the pattern entirely

 PR: https://github.com/django/django/pull/10336

 I believe this fixes the issue but I'm not sure how much we want to
 document this. Should we raise a check failure?

--
Ticket URL: <https://code.djangoproject.com/ticket/29667#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.1bcfaea3983ef92bbbc26031a4b30062%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29667: path converters don't handle spaces well.

Django
In reply to this post by Django
#29667: path converters don't handle spaces well.
-------------------------------------+-------------------------------------
     Reporter:  Keryn Knight         |                    Owner:  Jeff
         Type:  Bug                  |                   Status:  assigned
    Component:  Core (URLs)          |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  converters path      |             Triage Stage:  Accepted
  _route_to_regex                    |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Tom Forbes):

 * cc: Tom Forbes (added)


--
Ticket URL: <https://code.djangoproject.com/ticket/29667#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.aa800f119efab2a9973eda5600a50909%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29667: path converters don't handle spaces well.

Django
In reply to this post by Django
#29667: path converters don't handle spaces well.
-------------------------------------+-------------------------------------
     Reporter:  Keryn Knight         |                    Owner:  Jeff
         Type:  Bug                  |                   Status:  assigned
    Component:  Core (URLs)          |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  converters path      |             Triage Stage:  Accepted
  _route_to_regex                    |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Tim Graham):

 I think it would be be best to prohibit whitespace.

--
Ticket URL: <https://code.djangoproject.com/ticket/29667#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.410db0b8690a919dae9cd56ef873d978%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29667: path converters don't handle spaces well.

Django
In reply to this post by Django
#29667: path converters don't handle spaces well.
-------------------------------------+-------------------------------------
     Reporter:  Keryn Knight         |                    Owner:  Jeff
         Type:  Bug                  |                   Status:  assigned
    Component:  Core (URLs)          |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  converters path      |             Triage Stage:  Accepted
  _route_to_regex                    |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Adam (Chainz) Johnson):

 I agree, prohibiting whitespace would be best, it would avoid 'dialects'
 of urlconfs appearing and the potential for future problems.

--
Ticket URL: <https://code.djangoproject.com/ticket/29667#comment:6>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.6999705aa08bb5f8416ad8b791a45d31%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29667: path converters don't handle spaces well.

Django
In reply to this post by Django
#29667: path converters don't handle spaces well.
-------------------------------------+-------------------------------------
     Reporter:  Keryn Knight         |                    Owner:  Jeff
         Type:  Bug                  |                   Status:  assigned
    Component:  Core (URLs)          |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  converters path      |             Triage Stage:  Accepted
  _route_to_regex                    |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Vishvajit Pathak):

 Tim, Adam,
 So do we want to raise `ImproperlyConfigured` when whitespace is used ?

--
Ticket URL: <https://code.djangoproject.com/ticket/29667#comment:7>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.df582be2e362cbf9a21fe59b0556f18b%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29667: path converters don't handle spaces well.

Django
In reply to this post by Django
#29667: path converters don't handle spaces well.
-------------------------------------+-------------------------------------
     Reporter:  Keryn Knight         |                    Owner:  Jeff
         Type:  Bug                  |                   Status:  assigned
    Component:  Core (URLs)          |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  converters path      |             Triage Stage:  Accepted
  _route_to_regex                    |
    Has patch:  0                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Vishvajit Pathak):

 * cc: Vishvajit Pathak (added)


--
Ticket URL: <https://code.djangoproject.com/ticket/29667#comment:8>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.807409beba1e3a435eea7cddc52f7b7a%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29667: path converters don't handle spaces well.

Django
In reply to this post by Django
#29667: path converters don't handle spaces well.
-------------------------------------+-------------------------------------
     Reporter:  Keryn Knight         |                    Owner:  Hasan
                                     |  Ramezani
         Type:  Bug                  |                   Status:  assigned
    Component:  Core (URLs)          |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  converters path      |             Triage Stage:  Accepted
  _route_to_regex                    |
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Hasan Ramezani):

 * owner:  Jeff => Hasan Ramezani
 * has_patch:  0 => 1


Comment:

 PR [https://github.com/django/django/pull/11688]

--
Ticket URL: <https://code.djangoproject.com/ticket/29667#comment:9>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.1474ff2a3e6c33e5809cbd4983314a0e%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29667: path converters don't handle spaces well.

Django
In reply to this post by Django
#29667: path converters don't handle spaces well.
-------------------------------------+-------------------------------------
     Reporter:  Keryn Knight         |                    Owner:  Hasan
                                     |  Ramezani
         Type:  Bug                  |                   Status:  assigned
    Component:  Core (URLs)          |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  converters path      |             Triage Stage:  Accepted
  _route_to_regex                    |
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  1
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Carlton Gibson):

 * needs_better_patch:  0 => 1


Comment:

 Patch looks good: just a few adjustments and ready to go. (I'll mark RFC
 to keep it on my rader: I assume Hasan will turn them around quickly, as
 ever 🙂) Thanks Hasan!

--
Ticket URL: <https://code.djangoproject.com/ticket/29667#comment:10>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.f8a1f5b59b51dd43b798d155300a6dc8%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29667: path converters don't handle spaces well.

Django
In reply to this post by Django
#29667: path converters don't handle spaces well.
-------------------------------------+-------------------------------------
     Reporter:  Keryn Knight         |                    Owner:  Hasan
                                     |  Ramezani
         Type:  Bug                  |                   Status:  assigned
    Component:  Core (URLs)          |                  Version:  master
     Severity:  Normal               |               Resolution:
     Keywords:  converters path      |             Triage Stage:  Ready for
  _route_to_regex                    |  checkin
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Carlton Gibson):

 * needs_better_patch:  1 => 0
 * stage:  Accepted => Ready for checkin


--
Ticket URL: <https://code.djangoproject.com/ticket/29667#comment:11>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.7a092ae2f5784ce16859eb49897e9715%40djangoproject.com.
Reply | Threaded
Open this post in threaded view
|

Re: [Django] #29667: path converters don't handle spaces well.

Django
In reply to this post by Django
#29667: path converters don't handle spaces well.
-------------------------------------+-------------------------------------
     Reporter:  Keryn Knight         |                    Owner:  Hasan
                                     |  Ramezani
         Type:  Bug                  |                   Status:  closed
    Component:  Core (URLs)          |                  Version:  master
     Severity:  Normal               |               Resolution:  fixed
     Keywords:  converters path      |             Triage Stage:  Ready for
  _route_to_regex                    |  checkin
    Has patch:  1                    |      Needs documentation:  0
  Needs tests:  0                    |  Patch needs improvement:  0
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------
Changes (by Carlton Gibson <carlton.gibson@…>):

 * status:  assigned => closed
 * resolution:   => fixed


Comment:

 In [changeset:"22394bd3a18a7d9a8957a0b431f8ae4e5ca03a8c" 22394bd3]:
 {{{
 #!CommitTicketReference repository=""
 revision="22394bd3a18a7d9a8957a0b431f8ae4e5ca03a8c"
 Fixed #29667 -- Prohibited whitespaces in path() URLs.
 }}}

--
Ticket URL: <https://code.djangoproject.com/ticket/29667#comment:12>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/067.2ba82297e254a3903953e12db6b603c8%40djangoproject.com.