[#3070] Fix httpdownloader error with missing content-disposition filename

The parsing of the content-disposition in httpdownloader was not able to
handle missing parameters e.g. "Content-Disposition: attachment" and would
result in an IndexError. Added a test for this use-case.

Fixed the issue using the cgi.parse_header to extract the parameters.
This commit is contained in:
Calum Lind 2017-10-29 12:27:26 +00:00
parent 00dcd60d56
commit 53f818e176
2 changed files with 32 additions and 12 deletions

View File

@ -9,6 +9,7 @@
from __future__ import unicode_literals
import cgi
import logging
import os.path
import zlib
@ -79,20 +80,23 @@ class HTTPDownloader(client.HTTPDownloader):
self.decoder = zlib.decompressobj(zlib.MAX_WBITS + 32)
if 'content-disposition' in headers and not self.force_filename:
new_file_name = str(headers['content-disposition'][0]).split(';')[1].split('=')[1]
new_file_name = sanitise_filename(new_file_name)
new_file_name = os.path.join(os.path.split(self.value)[0], new_file_name)
content_disp = str(headers['content-disposition'][0])
content_disp_params = cgi.parse_header(content_disp)[1]
if 'filename' in content_disp_params:
new_file_name = content_disp_params['filename']
new_file_name = sanitise_filename(new_file_name)
new_file_name = os.path.join(os.path.split(self.value)[0], new_file_name)
count = 1
fileroot = os.path.splitext(new_file_name)[0]
fileext = os.path.splitext(new_file_name)[1]
while os.path.isfile(new_file_name):
# Increment filename if already exists
new_file_name = '%s-%s%s' % (fileroot, count, fileext)
count += 1
count = 1
fileroot = os.path.splitext(new_file_name)[0]
fileext = os.path.splitext(new_file_name)[1]
while os.path.isfile(new_file_name):
# Increment filename if already exists
new_file_name = '%s-%s%s' % (fileroot, count, fileext)
count += 1
self.fileName = new_file_name
self.value = new_file_name
self.fileName = new_file_name
self.value = new_file_name
elif self.code in (http.MOVED_PERMANENTLY, http.FOUND, http.SEE_OTHER, http.TEMPORARY_REDIRECT):
location = headers['location'][0]

View File

@ -48,6 +48,14 @@ class RenameResource(Resource):
return b'This file should be called ' + filename
class AttachmentResource(Resource):
def render(self, request):
request.setHeader(b'Content-Type', b'text/plain')
request.setHeader(b'Content-Disposition', b'attachment')
return b'Attachement with no filename set'
class CookieResource(Resource):
def render(self, request):
@ -99,6 +107,7 @@ class TopLevelResource(Resource):
self.redirect_rsrc = RedirectResource()
self.putChild('redirect', self.redirect_rsrc)
self.putChild('rename', RenameResource())
self.putChild('attachment', AttachmentResource())
self.putChild('partial', PartialDownloadResource())
def getChild(self, path, request): # NOQA: N802
@ -195,6 +204,13 @@ class DownloadFileTestCase(unittest.TestCase):
d.addCallback(self.assertContains, 'This file should be called /etc/passwd')
return d
def test_download_with_attachment_no_filename(self):
url = self.get_url('attachment')
d = download_file(url, fname('original'))
d.addCallback(self.assertEqual, fname('original'))
d.addCallback(self.assertContains, 'Attachement with no filename set')
return d
def test_download_with_rename_prevented(self):
url = self.get_url('rename?filename=spam')
d = download_file(url, fname('forced'), force_filename=True)