Opened 6 years ago

Closed 6 years ago

#13765 closed (fixed)

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

Reported by: KyleMac Owned by: Chris Beaven
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:

Description

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 Chris Beaven 6 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 6 years ago by anonymous

Component: SerializationTemplate system
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

comment:2 Changed 6 years ago by Russell Keith-Magee

Triage Stage: UnreviewedAccepted

Changed 6 years ago by Chris Beaven

Attachment: 13765.diff added

comment:3 Changed 6 years ago by Chris Beaven

Has patch: set
Owner: changed from nobody to Chris Beaven
Status: newassigned
Triage Stage: AcceptedReady 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 6 years ago by Luke Plant

Fixed in [13849] and [13850].

comment:5 Changed 6 years ago by Luke Plant

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.
Back to Top