jazzband / django-downloadview

Serve files with Django.
https://django-downloadview.readthedocs.io
Other
380 stars 58 forks source link

87 double quoted filename #88

Closed cjdreiss closed 9 years ago

cjdreiss commented 10 years ago

For issue #87. Test ASCII and UTF-8 files with and without commas. Works in Chrome, Safari, Firefox on OS X.

Natim commented 10 years ago

Please add a test showing the coma problem.

cjdreiss commented 10 years ago

Not sure how to add a test for a browser interaction. There was a filename on my production app that had a comma, and Chrome refused to download it giving an error about multiple headers. Other browsers handle it fine. Doing a search for Chrome Duplicate Header led me to find that Chrome expects filename parameter to be a quoted string, per the w3c 19.5.1 Content-Disposition specification

This issue was also in web2py, reported here https://groups.google.com/forum/#!topic/web2py/paweofnBz-g and fixed here https://github.com/web2py/web2py/commit/5e7de996e023a21247d4a9b05a55755d42478895

Natim commented 10 years ago

oh yes you are right the "" in the test should be enough

Natim commented 10 years ago

it looks like there is other tests to fix. see travis

benoitbryon commented 10 years ago

Travis reports failure on a test in demo project: https://travis-ci.org/benoitbryon/django-downloadview/jobs/26963046#L201 for both py27 and py33 environments. When I add a pdb.set_trace() in django_downloadview.test.DownloadResponseValidator.assert_content() I see this:

>>> [s for s in response.streaming_content]
['\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\x03\xf3H\xcd\xc9\xc9W(\xcf/\xcaIQ\xe4\x02\x00A\xe4\xa9\xb2\r\x00\x00\x00']

Whereas "Hello world!\n" is expected. I haven't figured out what's going wrong there yet...

benoitbryon commented 10 years ago

Looking at django_downloadview.files.HTTPFile.file(), we can see that self.request.content is Hello world!\n, whereas self.request.raw.read() is the strange \x1f\x8b\x08\x00\x00\x00\x00\x00\x00\x03\xf3H\xcd\xc9\xc9W(\xcf/\xcaIQ\xe4\x02\x00A\xe4\xa9\xb2\r\x00\x00\x00. (if you try it, notice you cannot read both request.content and request.raw consecutively, once you read one, you would have to seek(0) on request.raw, but "seek" operation is not allowed).

I checked versions of requests:

So I tried using requests 2.2.1 on the 87-double-quoted-filename branch => fails! I suppose the problem is not related to version of requests.

benoitbryon commented 10 years ago

Perhaps raw.github.com's response differs from raw.githubusercontent.com's one. I haven't tried to compare them yet.

If this is the origin of the issue, perhaps, we could (as another ticket/feature) replace downloads from Github by downloads from a Python server (SimpleHTTPServer?) we run during the tests. So that we do not depend on Github's API while running the tests.

cjdreiss commented 10 years ago

Could be the case that the response is different. Not sure how you would check that now, since going to raw.github redirects to raw.githubusercontent

They don't mention needing to do anything but allow your code to follow the redirect or change the URL yourself on the Github dev blog: https://developer.github.com/changes/2014-04-25-user-content-security/

benoitbryon commented 9 years ago

Hi! Looks like recent changes in master also unblock this pull-request: see #102. I guess we will be able to merge and release soon! Sorry for being that late on this fix. I've been quite inactive for a while on this project :(