Opened 9 years ago

Closed 5 years ago

#6892 closed Cleanup/optimization (wontfix)

Warn about raw "%" in redirect_to strings.

Reported by: daonb <bennydaon@…> Owned by: nobody
Component: Generic views Version: master
Severity: Normal Keywords: unicode, url
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

from my urls.py:

url(r'^password/change/done/$',
    django.views.generic.simple.redirect_to,
    {'url': '/toviva/myroom/?' + urlencode({'message': _('Password change successful')})})

When I access the url I get an error:

ValueError at /toviva/accounts/password/change/done/
unsupported format character 'D' (0x44) at index 25

printing the url parameters in redirect_to:

/toviva/myroom/?message=%D7%94%D7%A1%D7%99%D7%A1%D7%9E%D7%94+%D7%A9%D7%95%D7%A0%D7%AA%D7%94+%D7%91%D7%94%D7%A6%D7%9C%D7%97%D7%94

In redirect_to the line that fails is:

   return HttpResponsePermanentRedirect(url % kwargs)

I found another way to do it:

  url(r'^password/change/done/$',
    django.views.generic.simple.redirect_to,
    {'url': '/toviva/myroom/?message=%(message)s', 'message': urlquote_plus(_('Password change successful'))}),

Which works for me, but still - I'd be happy if someone can find a way to make redirect_to more robust.

Attachments (3)

6892v1.diff (1.3 KB) - added by daonb <bennydaon@…> 9 years ago.
adding a call to iri_to_uri in redirect_to
6892v2.diff (736 bytes) - added by daonb <bennydaon@…> 8 years ago.
A documentation patch
simple.diff (1.3 KB) - added by furryRevenge 6 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 9 years ago by Philippe Raoult

Resolution: invalid
Status: newclosed

AFAIK urls aren't supposed to be in unicode. urlquote is the way to go.

comment:2 Changed 9 years ago by daonb <bennydaon@…>

Resolution: invalid
Status: closedreopened

Seems like I wasn't clear enough, sorry. For the love of Django, I'll try once more:

According to urllib.qoute documentation: "Replace special characters in string using the "%xx" escape....Example: quote('/~connolly/') yields '/%7econnolly/'"

After urlquote you get an ascii string with "%xx" escape for every special or unicode character. Using the "%" operator on this string crashes because it has too many '%' characters, usually with unsupported format characters - such as 7 in the Python Library Reference example (redirect_to does it in "url % kwargs") .

Hope it makes sense, in any case I'm re-opening the ticket. Hope I'm not violating the protocol, just want to make sure someone reads this.

comment:3 Changed 9 years ago by James Bennett

Changed 9 years ago by daonb <bennydaon@…>

Attachment: 6892v1.diff added

adding a call to iri_to_uri in redirect_to

comment:4 Changed 9 years ago by daonb <bennydaon@…>

Has patch: set
Summary: unicode support in redirect_toIRI support in redirect_to

6892v1 changes redirect_to so it can accept an IRI in the url parameter. To keep it backward compatible, the url parameter name can not be changed. The patch is based on Django's unicode documentation (thanks ubernostrum):

>>> iri_to_uri(u'/favorites/François/%s' % urlquote(u'Paris & Orléans'))
'/favorites/Fran%C3%A7ois/Paris%20%26%20Orl%C3%A9ans'

comment:5 Changed 8 years ago by Simon Greenhill

Triage Stage: UnreviewedReady for checkin

comment:6 Changed 8 years ago by Malcolm Tredinnick

Has patch: unset
Needs documentation: set
Summary: IRI support in redirect_toWarn about raw "%" in redirect_to strings.
Triage Stage: Ready for checkinAccepted

This patch doesn't look like it's fixing the right problem. The problem is this: the url parameter passed to redirect_to() can contain dictionary-based format strings to accept parameters from the URL. In the example in the example in the description, the reporter is also including %-encoded characters as part of the URL. So we need to tell the difference between to uses of "%":

url = "%(param)s %7D"

The first one requires a substitution and the second should be left alone when we are subtituting the kwargs (and hence that "%" should really be "%%" in the string).

I suspect this is really a documentation issue. We could try to automatically detect the context of "%" signs in the url string, but that sounds like solving the problem at the wrong level. People using redirect_to() need to remember that they are passing in a format string and so any stray % signs need to be doubled (escaped as "%%"). Something like

my_string.replace("%", "%%")

So a documentation patch will be welcomed, but the current patch here isn't fixing the problem, since it looks like it doesn't solve the original error as described.

Changed 8 years ago by daonb <bennydaon@…>

Attachment: 6892v2.diff added

A documentation patch

comment:7 Changed 8 years ago by daonb <bennydaon@…>

Has patch: set

Changed 6 years ago by furryRevenge

Attachment: simple.diff added

comment:8 Changed 6 years ago by furryRevenge

Needs documentation: unset
Needs tests: set
Patch needs improvement: set

comment:9 Changed 6 years ago by Julien Phalip

Severity: Normal
Type: Cleanup/optimization

comment:10 Changed 5 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:11 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:12 Changed 5 years ago by Claude Paroz

Resolution: wontfix
Status: reopenedclosed

The documentation for the new class-based view RedirectView is now clear:
Because keyword interpolation is always done (even if no arguments are passed in), any "%" characters in the URL must be written as "%%" so that Python will convert them to a single percent sign on output.
https://docs.djangoproject.com/en/dev/ref/class-based-views/#django.views.generic.base.RedirectView

As for the use-case of the OP, I think that a .replace('%', '%%') should be appended to the urlencode() call.

Note: See TracTickets for help on using tickets.
Back to Top