Opened 5 years ago

Closed 5 years ago

#13765 closed (fixed)

The urlencode filter is actually urlquote and it doesn't accept the safe parameter

Reported by: KyleMac Owned by: SmileyChris
Component: Template system Version: 1.2
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:


Isn't it a bit odd that the |urlencode filter is actually urllib.quote and not urllib.urlencode as the name would suggest? To make things worse it doesn't accept the safe parameter that urllib.quote takes.

I don't know whether it should deal with dictionaries or not like urllib.urlencode does, but I do believe that |urlencode should escape the same characters as urllib.urlencode.

Short story is that you can't escape a / for proper use in a query string with |urlencode because / is a default safe character in urllib.quote since / is acceptable in paths. The quickest fix would be to allow |urlencode to take the safe parameter and to have it default to an empty string.

Attachments (1)

13765.diff (3.7 KB) - added by SmileyChris 5 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 5 years ago by anonymous

  • Component changed from Serialization to Template system
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 5 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

Changed 5 years ago by SmileyChris

comment:3 Changed 5 years ago by SmileyChris

  • Has patch set
  • Owner changed from nobody to SmileyChris
  • Status changed from new to assigned
  • Triage Stage changed from Accepted to Ready for checkin

Patch added with tests & docs.

KyleMac: we can't make it default to an empty string, that would be backwards incompatible. But with this patch you can pass an empty string to the filter.

I'm guessing that russ' acceptance was primarily for the feature of adding the safe argument, not changing the filter name or anything.

comment:4 Changed 5 years ago by lukeplant

Fixed in [13849] and [13850].

comment:5 Changed 5 years ago by lukeplant

  • Resolution set to fixed
  • Status changed from assigned to closed
Note: See TracTickets for help on using tickets.
Back to Top