From 53f818e176398804a0d91a3c83c35b064f5c2598 Mon Sep 17 00:00:00 2001 From: Calum Lind Date: Sun, 29 Oct 2017 12:27:26 +0000 Subject: [PATCH] [#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. --- deluge/httpdownloader.py | 28 ++++++++++++++++------------ deluge/tests/test_httpdownloader.py | 16 ++++++++++++++++ 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/deluge/httpdownloader.py b/deluge/httpdownloader.py index 75000ced3..d42d27570 100644 --- a/deluge/httpdownloader.py +++ b/deluge/httpdownloader.py @@ -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] diff --git a/deluge/tests/test_httpdownloader.py b/deluge/tests/test_httpdownloader.py index 08bbd295a..6c410ecd2 100644 --- a/deluge/tests/test_httpdownloader.py +++ b/deluge/tests/test_httpdownloader.py @@ -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)