util: use the uploaded werkzeug.FileStorage class directly

In newer versions of Werkzeug the uploaded file's stream is a
SpooledTemporaryFile. That class doesn't implement the .seekable method,
which is called by the Python zipfile library, and everything falls
over.

This patch avoids the issue by using the FileStorage class directly; as
of Werkzeug 0.15 it proxies this method correctly to whatever
object is backing the SpooledTemporaryFile. (At least according to the
relevant GH issue: pallets/werkzeug#1344 ).

hostthedocs/__init__.py

@@ -18,7 +18,7 @@ def hmfd():
     if request.method == 'POST':
         if not request.files:
             return abort(400, 'Request is missing a zip/tar file.')
-        uploaded_file = util.UploadedFile.from_request(request)
+        uploaded_file = util.file_from_request(request)
         unpack_project(
             uploaded_file,
             request.form,

hostthedocs/util.py

@@ -10,56 +10,24 @@
 
 logger = logging.getLogger()
 
-
-class UploadedFile(object):
-    """
-    UploadedFile represents a file uploaded during a POST request.
+def file_from_request(request):
     """
+    Get the uploaded file from a POST request, which should contain exactly one file.
 
-    def __init__(self, filename, stream):
-        """Instantiate an UploadedFile
-
-        :param str filename: The name of the file.
-        :param stream: The open file stream.
-        """
-        self._filename = filename
-        self._stream = stream
-
-    @classmethod
-    def from_request(cls, request):
-        """
-        Instantiate an UploadedFile from the first file in a request.
-
-        :param werkzeug.wrappers.BaseRequest request: The POST request.
-        :return: The instantiated UploadedFile.
-        :raises ValueError: if no files exist within the request.
-        """
-        uploaded_files = list(request.files.values())
-        if len(uploaded_files) > 1:
-            logger.warning(
-                'Only one file can be uploaded for each request. '
-                'Only the first file will be used.'
-            )
-        elif len(uploaded_files) == 0:
-            raise ValueError('Request does not contain uploaded file')
-
-        current_file = uploaded_files[0]
-        return cls(current_file.filename, current_file.stream)
-
-    def get_filename(self):
-        return self._filename
-
-    def get_stream(self):
-        return self._stream
-
-    def close(self):
-        """close the file stream
-        """
-        try:
-            self._stream.close()
-        except:
-            pass
+    :param werkzeug.wrappers.BaseRequest request: The POST request.
+    :return: The instantiated UploadedFile.
+    :raises ValueError: if no files exist within the request.
+    """
+    uploaded_files = list(request.files.values())
+    if len(uploaded_files) > 1:
+        logger.warning(
+            'Only one file can be uploaded for each request. '
+            'Only the first file will be used.'
+        )
+    elif len(uploaded_files) == 0:
+        raise ValueError('Request does not contain uploaded file')
 
+    return uploaded_files[0]
 
 class FileExpander(object):
     """
@@ -97,11 +65,11 @@ def detect_compression_method(cls, filename):
         raise ValueError('Unknown compression method for %s' % filename)
 
     def __enter__(self):
-        method = self.detect_compression_method(self._file.get_filename())
+        method = self.detect_compression_method(self._file.filename)
         if method == 'zip':
-            self._handle = zipfile.ZipFile(self._file.get_stream())
+            self._handle = zipfile.ZipFile(self._file)
         elif method == 'tar':
-            self._handle = tarfile.open(fileobj=self._file.get_stream(), mode='r:*')
+            self._handle = tarfile.open(fileobj=self._file, mode='r:*')
         else:
             raise ValueError('Unsupported method %s' % method)
 

tests/test_filekeeper.py

@@ -5,6 +5,7 @@
 import tempfile
 import unittest
 import natsort
+import werkzeug.datastructures
 
 from hostthedocs import filekeeper as fk
 from hostthedocs import util
@@ -15,6 +16,10 @@
 TARGZFILE = os.path.join(THISDIR, 'project.tar.gz')
 TARBZ2FILE = os.path.join(THISDIR, 'project.tar.bz2')
 
+def make_uploaded_file(filename=ZIPFILE):
+    stream = open(filename, 'rb')
+    basename = os.path.basename(filename)
+    return werkzeug.datastructures.FileStorage(stream=stream, filename=basename)
 
 class TestParseDocfiles(unittest.TestCase):
     def test_parses(self):
@@ -109,36 +114,33 @@ def do_unpacks(self, uploaded_file):
         self.assert_exists('proj/description.txt', exists=os.path.isfile)
 
     def test_unpack_zip(self):
-        uploaded_file = util.UploadedFile(ZIPFILE, open(ZIPFILE, mode='rb'))
+        uploaded_file = make_uploaded_file(ZIPFILE)
         self.do_unpacks(uploaded_file)
 
     def test_unpack_tar(self):
-        uploaded_file = util.UploadedFile(TARFILE, open(TARFILE, mode='rb'))
+        uploaded_file = make_uploaded_file(TARFILE)
         self.do_unpacks(uploaded_file)
 
     def test_unpack_tar_gz(self):
-        uploaded_file = util.UploadedFile(TARGZFILE, open(TARGZFILE, mode='rb'))
+        uploaded_file = make_uploaded_file(TARGZFILE)
         self.do_unpacks(uploaded_file)
 
     def test_unpack_tar_bz(self):
-        uploaded_file = util.UploadedFile(TARBZ2FILE, open(TARBZ2FILE, mode='rb'))
+        uploaded_file = make_uploaded_file(TARBZ2FILE)
         self.do_unpacks(uploaded_file)
 
 
 class TestUnpackProjectDescr(BaseTestUnpackProject):
-    def make_uploaded_file(self):
-        return util.UploadedFile(ZIPFILE, open(ZIPFILE, mode='rb'))
-
     def test_ensure_description_file(self):
-        uploaded_file = self.make_uploaded_file()
+        uploaded_file = make_uploaded_file()
         tempd = self.make_temp_dir()
         metad = {'name': 'proj', 'version': '1.1', 'description': 'descr'}
         fk.unpack_project(uploaded_file, metad, tempd)
 
         self.assert_contains('proj/description.txt', metad['description'])
 
     def test_update_description_file(self):
-        uploaded_file = self.make_uploaded_file()
+        uploaded_file = make_uploaded_file()
         tempd = self.make_temp_dir()
         metad = {'name': 'proj', 'version': '1.1', 'description': 'descr_old'}
         fk.unpack_project(uploaded_file, metad, tempd)
@@ -148,7 +150,7 @@ def test_update_description_file(self):
         self.assert_contains('proj/description.txt', metad['description'])
 
     def test_no_update_description_file(self):
-        uploaded_file = self.make_uploaded_file()
+        uploaded_file = make_uploaded_file()
         tempd = self.make_temp_dir()
         metad = {'name': 'proj', 'version': '1.1', 'description': 'descr_old'}
         fk.unpack_project(uploaded_file, metad, tempd)
@@ -158,7 +160,7 @@ def test_no_update_description_file(self):
         self.assert_contains('proj/description.txt', 'descr_old')
 
     def test_no_description_file(self):
-        uploaded_file = self.make_uploaded_file()
+        uploaded_file = make_uploaded_file()
         tempd = self.make_temp_dir()
         metad = {'name': 'proj', 'version': '1.1'}
         fk.unpack_project(uploaded_file, metad, tempd)

tests/test_util.py

@@ -22,10 +22,10 @@ def test_extracting_filestream_from_request(self):
             request = Request(builder.get_environ())
 
             # WHEN we process the request.
-            filestream = util.UploadedFile.from_request(request)
+            filestream = util.file_from_request(request)
 
             # THEN the file-stream should correspond to the provided file.
-            self.assertEqual(content, filestream.get_stream().read())
+            self.assertEqual(content, filestream.read())
 
     @raises(ValueError)
     def test_exception_thrown_if_request_contains_no_file(self):
@@ -34,4 +34,4 @@ def test_exception_thrown_if_request_contains_no_file(self):
         request = Request(builder.get_environ())
 
         # EXPECT an exception is thrown when we process the request.
-        util.UploadedFile.from_request(request)
+        util.file_from_request(request)