Added support for non-ascii filenames in content-disposition. #17
Conversation
Using the details outlined at: http://kbyanc.blogspot.hk/2010/07/serving-file-downloads-with-non-ascii.html To support older browsers, an ascii only filename is added to the content-dispostion. The unicode version is properly encoded and added to the header. The argument attachment_filename may now be False to send no filename. Added and fixed unit tests to verify the new behavior.
|
The linked page says that this doesn't work in "WebKit-based browsers", i.e., that it saves with a percent-encoded filename. Omitting the filename would appear to be a better solution, as this works regardless of browser. |
According to the latest tests, this now works in Safari 6. Are you able to test with Safari? From what I understand, Safari 5 will continue to show the percents but Safari 6 was released over a year ago, July 25, 2012. The original article describing the issue and solution was written July 1, 2010.
My patch supports this by passing the argument If you find any bugs with the patch let me know and I'll try to address them. Thanks. |
Tested on older versions of Django, realized the try/except is unnecassary. Should probably use force_text() instead of smart_text() as the unidecode() functions requires an actual unicode string.
To help some older browsers, such as Safari 5, the url quoted filename is only added if the filename contains non-ascii characters. In the ascii case, use the old style filename attachment. Fixed unittests.
|
@johnsensible Any chance on getting the code reviewed and possibly merged for a release? If you have any reservations or require more testing, please let me know. I'd be happy to look into it. |
|
Sorry, it's been a bit crazy here at work. Will try and look at this tomorrow. |
|
@jdufresne ok just had a quick look.
Otherwise I think it looks ok so far. cheers, John |
|
Oh and probably also best to move all of the django related imports inside of the functions as well (for same reason as moving the unidecode import). |
Will do.
I believe so. That is my intent. I will do some testing to try to verify this.
According to setup.py, Django >= 1.4.2 is a requirement of django-sendfile. Is this not correct? This is why I removed the try/except. Should I change the requirement to >=1.3 as well as falling back to force_unicode? |
|
I did the first two tests and changes requested. Please review. I'll wait to do the last fix when I get confirmation on the Django requirement. |
Fallback from force_text to force_unicode as it is named in Django 1.3. Fixed the requirements in setup.py to reflect that Django 1.3 is supported.
|
I went ahead and ran several tests with Django 1.3. The test suite passes no problem so I added the fallback back in. Django 1.3 is now listed as the requirement (not 1.4). |
|
Hmmm. That Django 1.4.2 requirement got added in by a contributor a while ago. Think I must have just overlooked it. Ok, well lets just stick with 1.4.2 then. I'll give your code another look now. |
|
Ok all merged in. I see you already change the requirements to Django 1.3. I've tested against 1.3 and it all seems to be ok. Should be appearing in PyPi shortly (version 0.3.3). Cheers, John |
|
Thanks! |
Using the details outlined at:
http://kbyanc.blogspot.hk/2010/07/serving-file-downloads-with-non-ascii.html
To support older browsers, an ascii only filename is added to the
content-dispostion. The unicode version is properly encoded and added
to the header. The argument attachment_filename may now be False to
send no filename. Added and fixed unit tests to verify the new
behavior.
This change introduces a new dependency on Unidecode https://pypi.python.org/pypi/Unidecode. I have tested this change with Python 2.7.5 and Django 1.6. All unit tests and manual test cases worked for me. Please review the changes. If any other configurations do not work please let me know with version numbers.