Opened 8 years ago

Closed 3 years ago

Last modified 3 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: master
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 8 years ago.
Escape spaces in generated URL using urllib.quote
base.2.py.diff (664 bytes) - added by Esaj 8 years ago.
Better patch; the previous one erroneously escapes MEDIA_URL
base.3.py.diff (910 bytes) - added by Esaj 8 years ago.
Use django.util.http.urlquote for unicode safety
5160-test.diff (867 bytes) - added by claudep 3 years ago.
Test that issue is solved

Download all attachments as: .zip

Change History (15)

Changed 8 years ago by Esaj

Escape spaces in generated URL using urllib.quote

Changed 8 years ago by Esaj

Better patch; the previous one erroneously escapes MEDIA_URL

comment:1 Changed 8 years ago by mtredinnick

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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

comment:2 Changed 8 years ago by Simon G. <dev@…>

  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

Changed 8 years ago by Esaj

Use django.util.http.urlquote for unicode safety

comment:3 Changed 8 years ago by SmileyChris

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

comment:4 Changed 8 years ago by mtredinnick

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

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 Changed 8 years ago by esaj

  • Owner changed from nobody to esaj

comment:6 Changed 8 years ago by esaj

  • Status changed from new to assigned

comment:7 Changed 7 years ago by esaj

  • Owner esaj deleted
  • Status changed from assigned to new

comment:8 Changed 4 years ago by gabrielhurley

  • Severity set to Normal
  • Type set to New feature

Changed 3 years ago by claudep

Test that issue is solved

comment:9 Changed 3 years ago by claudep

  • 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 Changed 3 years ago by aaugustin

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

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 Changed 3 years ago by aaugustin

In [17361]:

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

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