Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix spooled temporary file exceptions on file upload - 2 #1409

Merged
merged 2 commits into from
Dec 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,11 @@ Unreleased
``/jump/<int(signed=True):count>``. (`#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
Expand Down Expand Up @@ -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


Expand Down
22 changes: 20 additions & 2 deletions tests/test_datastructures.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@

from __future__ import with_statement

import io
import pytest
import tempfile

from tests import strict_eq


Expand Down Expand Up @@ -885,8 +888,8 @@ def make_call_asserter(func=None):

>>> assert_calls, func = make_call_asserter()
>>> with assert_calls(2):
func()
func()
... func()
... func()
"""

calls = [0]
Expand Down Expand Up @@ -1068,6 +1071,21 @@ def test_bytes_proper_sentinel(self):
assert idx < 2
assert idx == 1

@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(
"ranges", ([(0, 1), (-5, None)], [(5, None)])
Expand Down
39 changes: 29 additions & 10 deletions tests/test_formparser.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
"""
from __future__ import with_statement

import csv
import io
import pytest

from os.path import join, dirname
Expand All @@ -22,7 +24,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
Expand Down Expand Up @@ -134,15 +136,32 @@ def test_parse_form_data_get_without_content(self):
strict_eq(len(form), 0)
strict_eq(len(files), 0)

def test_large_file(self):
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')
# close file to prevent fds from leaking
req.files['foo'].close()
@pytest.mark.parametrize(
("no_spooled", "size"),
((False, 100), (False, 3000), (True, 100), (True, 3000)),
)
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"
)
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)
Expand Down
10 changes: 9 additions & 1 deletion werkzeug/datastructures.py
Original file line number Diff line number Diff line change
Expand Up @@ -2755,7 +2755,15 @@ def __nonzero__(self):
__bool__ = __nonzero__

def __getattr__(self, name):
return getattr(self.stream, 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

def __iter__(self):
return iter(self.stream)
Expand Down