Opened 17 years ago

Closed 12 years ago

Last modified 12 years ago

#5160 closed New feature (fixed)

get_FIELD_url should return a valid URL with spaces escaped

Reported by: Esaj Owned by:
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: get_FIELD_url escape urllib.quote
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Some of my RSS feeds aren't validating because calling get_FIELD_url for an ImageField isn't escaping spaces in the filename.

A simple patch is attached which uses urllib.quote to fix this problem.

Attachments (4)

base.py.diff (664 bytes ) - added by Esaj 17 years ago.
Escape spaces in generated URL using urllib.quote
base.2.py.diff (664 bytes ) - added by Esaj 17 years ago.
Better patch; the previous one erroneously escapes MEDIA_URL
base.3.py.diff (910 bytes ) - added by Esaj 17 years ago.
Use django.util.http.urlquote for unicode safety
5160-test.diff (867 bytes ) - added by Claude Paroz 12 years ago.
Test that issue is solved

Download all attachments as: .zip

Change History (15)

by Esaj, 17 years ago

Attachment: base.py.diff added

Escape spaces in generated URL using urllib.quote

by Esaj, 17 years ago

Attachment: base.2.py.diff added

Better patch; the previous one erroneously escapes MEDIA_URL

comment:1 by Malcolm Tredinnick, 17 years ago

Using urllib.urlquote is incorrect here because the value could contain non-ASCII characters. Should be using django.utils.http.urlquote instead.

comment:2 by Simon G. <dev@…>, 17 years ago

Patch needs improvement: set
Triage Stage: UnreviewedAccepted

by Esaj, 17 years ago

Attachment: base.3.py.diff added

Use django.util.http.urlquote for unicode safety

comment:3 by Chris Beaven, 17 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:4 by Malcolm Tredinnick, 17 years ago

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

This isn't ready to commit yet. Firstly, because the existing tests fail, which is a bit of a showstopper (in this case, the failing test is probably wrong, but it still needs to be fixed at the same time). Secondly because it doesn't have any tests of its own. And, most importantly, because it causes some problems with serialisation -- things don't round-trip cleanly.

The last point is almost certainly a problem in the serialization framework, but it needs to be fixed first or at the same time. Moving back to accepted until I (or somebody else, preferably) can look at the issues.

comment:5 by Jason Davies, 17 years ago

Owner: changed from nobody to Jason Davies

comment:6 by Jason Davies, 17 years ago

Status: newassigned

comment:7 by Jason Davies, 16 years ago

Owner: Jason Davies removed
Status: assignednew

comment:8 by Gabriel Hurley, 13 years ago

Severity: Normal
Type: New feature

by Claude Paroz, 12 years ago

Attachment: 5160-test.diff added

Test that issue is solved

comment:9 by Claude Paroz, 12 years ago

Easy pickings: unset
Patch needs improvement: unset
UI/UX: unset

I guess this issue is solved nowadays. I've just added the test for the space char in the corresponding file_storage test.

comment:10 by Aymeric Augustin, 12 years ago

Resolution: fixed
Status: newclosed

get_FIELD_url doesn't exist anymore. Now it's just an url attribute on the field and the problem appears to be fixed. I'll commit the test.

comment:11 by Aymeric Augustin, 12 years ago

In [17361]:

Tested that spaces are properly escaped in files URLs. Refs #5160.

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