Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#30485 closed Bug (fixed)

Unexpected behavior for django.utils.http.urlencode

Reported by: Johan Lübcke Owned by: nobody
Component: Core (Other) Version: 2.2
Severity: Normal Keywords:
Cc: François Freitag Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The function django.utils.http.urlencode has been changed to give unexpected result for tuple values (and other iterable objects) in the case when no iterations is expected:

>>> django.utils.http.urlencode(dict(a=('a','b')), doseq=False)
'a=%5B%27a%27%2C+%27b%27%5D'

One would expect the same as the standard library version (Note the first and last characters has been replaced by square brackets):

>>> urllib.parse.urlencode(dict(a=('a', 'b')), doseq=False)
'a=%28%27a%27%2C+%27b%27%29'

If the value is a list, the result if what one would expect:

>>> django.utils.http.urlencode(dict(a=['a','b']), doseq=False)
'a=%5B%27a%27%2C+%27b%27%5D'
>>> urllib.parse.urlencode(dict(a=['a', 'b']), doseq=False)
'a=%5B%27a%27%2C+%27b%27%5D'

Note: This is a problem when one has objects that has a __str__ method defined, returning the value one would want to be in the urlencode result, but the object by coincidence is also iterable.

Change History (7)

comment:1 by Johan Lübcke, 5 years ago

Has patch: set

comment:2 by Carlton Gibson, 5 years ago

Cc: François Freitag added

Asking François to comment. This is related to #28679.

comment:3 by Carlton Gibson, 5 years ago

Triage Stage: UnreviewedAccepted

OK, having conferred with Mariusz, I'm going to Accept this as a bug on 2.2, to be fixed in master, rather than backported to 2.2. (We'll look at the final patch before deciding finally there.)

comment:4 by Carlton Gibson, 5 years ago

Triage Stage: AcceptedReady for checkin

comment:5 by Carlton Gibson <carlton.gibson@…>, 5 years ago

Resolution: fixed
Status: newclosed

In 0670b1b4:

Fixed #30485 -- Adjusted django.utils.http.urlencode for doseq=False case.

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In df46b329:

Refs #30485 -- Avoided unnecessary instance checks in urlencode.

Given doseq defaults to False it should avoid an unnecessary instance
check in most cases.

comment:7 by Carlton Gibson <carlton.gibson@…>, 5 years ago

In b903bb43:

Refs #30485 -- Removed non-representative test that emitted a warning.

Previously, when running the Django test suite with warnings enabled,
the following was emitted:

/usr/lib64/python3.7/urllib/parse.py:915: BytesWarning: str() on a bytearray instance

v = quote_via(str(v), safe, encoding, errors)

This occurred due to the bytearray() being passed to
urllib.parse.urlencode() which eventually calls str() on it. The test
does not represent desired real world behavior. Rather than test for and
assert strange unspecified behavior that emits a warning, remove it.

This was also discussed in PR #11374.

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