From 11b966afbc2382a15bf71dd458f25ebb0dac3802 Mon Sep 17 00:00:00 2001 From: Johnny Cochrane Date: Fri, 30 Nov 2018 01:48:53 -0800 Subject: [PATCH 1/2] FileStorage looks up attributes on stream and stream._file --- tests/test_datastructures.py | 17 ++++++++++++++-- tests/test_formparser.py | 39 +++++++++++++++++++++++++++++++----- werkzeug/datastructures.py | 7 ++++++- 3 files changed, 55 insertions(+), 8 deletions(-) diff --git a/tests/test_datastructures.py b/tests/test_datastructures.py index 6f2621988..51fc2eaef 100644 --- a/tests/test_datastructures.py +++ b/tests/test_datastructures.py @@ -885,8 +885,8 @@ def make_call_asserter(func=None): >>> assert_calls, func = make_call_asserter() >>> with assert_calls(2): - func() - func() + ... func() + ... func() """ calls = [0] @@ -1068,6 +1068,19 @@ def test_bytes_proper_sentinel(self): assert idx < 2 assert idx == 1 + @pytest.mark.skipif(PY2, reason='io "File Objects" are only available in PY3.') + @pytest.mark.parametrize( + "attributes", ( + # make sure the file object has the bellow attributes as described + # in https://github.com/pallets/werkzeug/issues/1344 + "writable", "readable", "seekable" + ) + ) + def test_proxy_can_access_stream_attrs(self, attributes): + from tempfile import SpooledTemporaryFile + file_storage = self.storage_class(stream=SpooledTemporaryFile()) + assert hasattr(file_storage, attributes) + @pytest.mark.parametrize( "ranges", ([(0, 1), (-5, None)], [(5, None)]) diff --git a/tests/test_formparser.py b/tests/test_formparser.py index 9e226f2d5..961bc5a70 100644 --- a/tests/test_formparser.py +++ b/tests/test_formparser.py @@ -22,7 +22,7 @@ from werkzeug.exceptions import RequestEntityTooLarge from werkzeug.datastructures import MultiDict from werkzeug.formparser import parse_form_data, FormDataParser -from werkzeug._compat import BytesIO +from werkzeug._compat import BytesIO, PY2 @Request.application @@ -134,13 +134,42 @@ def test_parse_form_data_get_without_content(self): strict_eq(len(form), 0) strict_eq(len(files), 0) - def test_large_file(self): + @pytest.mark.skipif(PY2, reason='io "File Objects" are only available in PY3.') + @pytest.mark.parametrize( + "attributes", ( + # make sure we have a real file here, because we expect to be + # on the disk. > 1024 * 500 + "fileno", + # Make sure the file object has the bellow attributes as described + # in https://github.com/pallets/werkzeug/issues/1344 + "writable", "readable", "seekable" + ) + ) + def test_large_file(self, attributes): data = b'x' * (1024 * 600) req = Request.from_values(data={'foo': (BytesIO(data), 'test.txt')}, method='POST') - # make sure we have a real file here, because we expect to be - # on the disk. > 1024 * 500 - assert hasattr(req.files['foo'].stream, u'fileno') + file_storage = req.files['foo'] + assert hasattr(file_storage, attributes) + # close file to prevent fds from leaking + req.files['foo'].close() + + @pytest.mark.skipif(PY2, reason='io "File Objects" are only available in PY3.') + @pytest.mark.parametrize( + "attributes", ( + # Make sure the file object has the bellow attributes as described + # in https://github.com/pallets/werkzeug/issues/1344 + "writable", "readable", "seekable" + ) + ) + def test_small_file(self, attributes): + # when the data's length is below the "max_size", this will implement + # an in-memory buffer + data = b'x' * 256 + req = Request.from_values(data={'foo': (BytesIO(data), 'test.txt')}, + method='POST') + file_storage = req.files['foo'] + assert hasattr(file_storage, attributes) # close file to prevent fds from leaking req.files['foo'].close() diff --git a/werkzeug/datastructures.py b/werkzeug/datastructures.py index 444f9c558..cfe450e49 100644 --- a/werkzeug/datastructures.py +++ b/werkzeug/datastructures.py @@ -2755,7 +2755,12 @@ def __nonzero__(self): __bool__ = __nonzero__ def __getattr__(self, name): - return getattr(self.stream, name) + try: + return getattr(self.stream, name) + except AttributeError: + if hasattr(self.stream, "_file"): + return getattr(self.stream._file, name) + raise def __iter__(self): return iter(self.stream) From 3638d0ec59d14cdadea9a4da57b8634d579ca644 Mon Sep 17 00:00:00 2001 From: David Lord Date: Mon, 3 Dec 2018 07:20:46 -0800 Subject: [PATCH 2/2] refactor FileStorage tests --- CHANGES.rst | 6 ++++ tests/test_datastructures.py | 29 +++++++++-------- tests/test_formparser.py | 60 +++++++++++++++--------------------- werkzeug/datastructures.py | 3 ++ 4 files changed, 51 insertions(+), 47 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 92129aaf4..ff303db21 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -141,6 +141,11 @@ Unreleased ``/jump/``. (`#1355`_) - :class:`~datastructures.Range` validates that list of range tuples passed to it would produce a valid ``Range`` header. (`#1412`_) +- :class:`~datastructures.FileStorage` looks up attributes on + ``stream._file`` if they don't exist on ``stream``, working around + an issue where :func:`tempfile.SpooledTemporaryFile` didn't + implement all of :class:`io.IOBase`. See + https://github.com/python/cpython/pull/3249. (`#1409`_) .. _`#209`: https://github.com/pallets/werkzeug/pull/209 .. _`#609`: https://github.com/pallets/werkzeug/pull/609 @@ -193,6 +198,7 @@ Unreleased .. _`#1401`: https://github.com/pallets/werkzeug/pull/1401 .. _`#1402`: https://github.com/pallets/werkzeug/pull/1402 .. _#1404: https://github.com/pallets/werkzeug/pull/1404 +.. _#1409: https://github.com/pallets/werkzeug/pull/1409 .. _#1412: https://github.com/pallets/werkzeug/pull/1412 diff --git a/tests/test_datastructures.py b/tests/test_datastructures.py index 51fc2eaef..51a4170f6 100644 --- a/tests/test_datastructures.py +++ b/tests/test_datastructures.py @@ -21,7 +21,10 @@ from __future__ import with_statement +import io import pytest +import tempfile + from tests import strict_eq @@ -1068,18 +1071,20 @@ def test_bytes_proper_sentinel(self): assert idx < 2 assert idx == 1 - @pytest.mark.skipif(PY2, reason='io "File Objects" are only available in PY3.') - @pytest.mark.parametrize( - "attributes", ( - # make sure the file object has the bellow attributes as described - # in https://github.com/pallets/werkzeug/issues/1344 - "writable", "readable", "seekable" - ) - ) - def test_proxy_can_access_stream_attrs(self, attributes): - from tempfile import SpooledTemporaryFile - file_storage = self.storage_class(stream=SpooledTemporaryFile()) - assert hasattr(file_storage, attributes) + @pytest.mark.skipif(PY2, reason='io.IOBase is only needed in PY3.') + @pytest.mark.parametrize("stream", (tempfile.SpooledTemporaryFile, io.BytesIO)) + def test_proxy_can_access_stream_attrs(self, stream): + """``SpooledTemporaryFile`` doesn't implement some of + ``IOBase``. Ensure that ``FileStorage`` can still access the + attributes from the backing file object. + + https://github.com/pallets/werkzeug/issues/1344 + https://github.com/python/cpython/pull/3249 + """ + file_storage = self.storage_class(stream=stream()) + + for name in ("fileno", "writable", "readable", "seekable"): + assert hasattr(file_storage, name) @pytest.mark.parametrize( diff --git a/tests/test_formparser.py b/tests/test_formparser.py index 961bc5a70..b29eea597 100644 --- a/tests/test_formparser.py +++ b/tests/test_formparser.py @@ -10,6 +10,8 @@ """ from __future__ import with_statement +import csv +import io import pytest from os.path import join, dirname @@ -134,44 +136,32 @@ def test_parse_form_data_get_without_content(self): strict_eq(len(form), 0) strict_eq(len(files), 0) - @pytest.mark.skipif(PY2, reason='io "File Objects" are only available in PY3.') @pytest.mark.parametrize( - "attributes", ( - # make sure we have a real file here, because we expect to be - # on the disk. > 1024 * 500 - "fileno", - # Make sure the file object has the bellow attributes as described - # in https://github.com/pallets/werkzeug/issues/1344 - "writable", "readable", "seekable" - ) + ("no_spooled", "size"), + ((False, 100), (False, 3000), (True, 100), (True, 3000)), ) - def test_large_file(self, attributes): - data = b'x' * (1024 * 600) - req = Request.from_values(data={'foo': (BytesIO(data), 'test.txt')}, - method='POST') - file_storage = req.files['foo'] - assert hasattr(file_storage, attributes) - # close file to prevent fds from leaking - req.files['foo'].close() - - @pytest.mark.skipif(PY2, reason='io "File Objects" are only available in PY3.') - @pytest.mark.parametrize( - "attributes", ( - # Make sure the file object has the bellow attributes as described - # in https://github.com/pallets/werkzeug/issues/1344 - "writable", "readable", "seekable" + def test_default_stream_factory(self, no_spooled, size, monkeypatch): + if no_spooled: + monkeypatch.setattr("werkzeug.formparser.SpooledTemporaryFile", None) + + data = b"a,b,c\n" * size + req = Request.from_values( + data={"foo": (BytesIO(data), "test.txt")}, + method="POST" ) - ) - def test_small_file(self, attributes): - # when the data's length is below the "max_size", this will implement - # an in-memory buffer - data = b'x' * 256 - req = Request.from_values(data={'foo': (BytesIO(data), 'test.txt')}, - method='POST') - file_storage = req.files['foo'] - assert hasattr(file_storage, attributes) - # close file to prevent fds from leaking - req.files['foo'].close() + file_storage = req.files["foo"] + + try: + if PY2: + reader = csv.reader(file_storage) + else: + reader = csv.reader(io.TextIOWrapper(file_storage)) + # This fails if file_storage doesn't implement IOBase. + # https://github.com/pallets/werkzeug/issues/1344 + # https://github.com/python/cpython/pull/3249 + assert sum(1 for _ in reader) == size + finally: + file_storage.close() def test_streaming_parse(self): data = b'x' * (1024 * 600) diff --git a/werkzeug/datastructures.py b/werkzeug/datastructures.py index cfe450e49..35b45958d 100644 --- a/werkzeug/datastructures.py +++ b/werkzeug/datastructures.py @@ -2758,6 +2758,9 @@ def __getattr__(self, name): try: return getattr(self.stream, name) except AttributeError: + # SpooledTemporaryFile doesn't implement IOBase, get the + # attribute from its backing file instead. + # https://github.com/python/cpython/pull/3249 if hasattr(self.stream, "_file"): return getattr(self.stream._file, name) raise