Code

Opened 7 years ago

Closed 6 years ago

#4074 closed (fixed)

admin interface filter.html does not encode url attributes properly

Reported by: tony.perkins@… Owned by: nobody
Component: contrib.admin Version: newforms-admin
Severity: Keywords: ampersand filter
Cc: robert@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

If you filter on a field that has a value that includes an ampersand when selecting that value to filter on it does not work.

The output of filter.html for an example


<li>
    <a href="?title=Escapade%20-%20Adult%20Kayak%20&amp;%20Snorkel,%20Northwest%20Maui">Escapade - Adult Kayak &amp; Snorkel, Northwest Maui</a></li>

I tried using the urlencode filter, but it encodes the ? as well which causes it to fail also.

<a href="{{ choice.query_string|urlencode }}">{{ choice.display|escape }}</a></li>

If I change the following it works. I know this is not the right place.

In filter.html
change

<a href="{{ choice.query_string }}">{{ choice.display|escape }}</a></li>

to

<a href="{{ choice.query_string|fix_ampersands }}">{{ choice.display|escape }}</a></li>

in html.py
change

def fix_ampersands(value):
    "Returns the given HTML with all unencoded ampersands encoded correctly"
    return unencoded_ampersands_re.sub('&amp;', value)

to

  def fix_ampersands(value):
    "Returns the given HTML with all unencoded ampersands encoded correctly"
    return unencoded_ampersands_re.sub('%26', value)

Attachments (4)

django-4074-admin-querystring-quote.patch (768 bytes) - added by Robert Bunting 7 years ago.
One way to patch it - saves lots of potential problems with the query string
django-4074-admin-querystring-quote2.patch (776 bytes) - added by Robert Bunting 7 years ago.
better unicode version
4074.diff (1.2 KB) - added by SmileyChris 7 years ago.
4074-nfa.diff (1.1 KB) - added by Karen Tracey <kmtracey@…> 6 years ago.
Same patch, only against newforms-admin

Download all attachments as: .zip

Change History (12)

comment:1 Changed 7 years ago by adrian

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Version changed from new-admin to SVN

Changed 7 years ago by Robert Bunting

One way to patch it - saves lots of potential problems with the query string

Changed 7 years ago by Robert Bunting

better unicode version

comment:2 Changed 7 years ago by Robert Bunting

  • Has patch set

Not sure if this would be acceptable, but it's a patch which will make sure the querystring is more useable everywhere. It solves '&', and also a problem I have been having with '>' (which when used in another form confuses the CSRF substitution regexp!)

comment:3 Changed 7 years ago by anonymous

  • Cc robert@… added

comment:4 Changed 7 years ago by SmileyChris

  • Patch needs improvement set
  • Summary changed from admin interface filter.html does not encode & properly to admin interface filter.html does not encode url attributes properly
  • Triage Stage changed from Unreviewed to Accepted

Changed 7 years ago by SmileyChris

comment:5 Changed 7 years ago by SmileyChris

  • Patch needs improvement unset
  • Triage Stage changed from Accepted to Ready for checkin

New patch (against SVN trunk, not newforms-admin branch) which fixes the issue at the core. I also added a small optimization.

The code in question hasn't changed in newforms, it may as well be fixed on trunk, yes?

comment:6 Changed 6 years ago by mtredinnick

  • Triage Stage changed from Ready for checkin to Accepted
  • Version changed from SVN to newforms-admin

Not worth fixing on trunk, since newforms-admin is so close. Pushing to the newforms-admin branch, though, so they can check it's been fixed over there.

Changed 6 years ago by Karen Tracey <kmtracey@…>

Same patch, only against newforms-admin

comment:7 Changed 6 years ago by Karen Tracey <kmtracey@…>

  • Triage Stage changed from Accepted to Ready for checkin

No, it hasn't been fixed in newforms-admin. Verified the problem and that the patch (rebased since the trunk version would not apply) fixes it.

comment:8 Changed 6 years ago by brosner

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

(In [7810]) newforms-admin: Fixed #4074 -- Properly urlencode the ChangeList query string when the value has an ampersand. Thanks Tony Perkins and SmileyChris.

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.