From 980168d08498c00a14ab0f687ffac8cc50edb326 Mon Sep 17 00:00:00 2001 From: David Lord Date: Mon, 18 Nov 2019 23:34:45 -0800 Subject: [PATCH] send_file doesn't allow StringIO --- CHANGES.rst | 3 ++ src/flask/helpers.py | 88 +++++++++++++++++++--------------- tests/test_helpers.py | 109 ++++++++++++++++++++---------------------- 3 files changed, 104 insertions(+), 96 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 859aaf35..0683bd98 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -13,6 +13,9 @@ Unreleased - The ``flask run`` command will only defer errors on reload. Errors present during the initial call will cause the server to exit with the traceback immediately. :issue:`3431` +- :func:`send_file` raises a :exc:`ValueError` when passed an + :mod:`io` object in text mode. Previously, it would respond with + 200 OK and an empty file. :issue:`3358` Version 1.1.2 diff --git a/src/flask/helpers.py b/src/flask/helpers.py index 3f401a5b..e3e59248 100644 --- a/src/flask/helpers.py +++ b/src/flask/helpers.py @@ -489,6 +489,11 @@ def send_file( guessing requires a `filename` or an `attachment_filename` to be provided. + When passing a file-like object instead of a filename, only binary + mode is supported (``open(filename, "rb")``, :class:`~io.BytesIO`, + etc.). Text mode files and :class:`~io.StringIO` will raise a + :exc:`ValueError`. + ETags will also be attached automatically if a `filename` is provided. You can turn this off by setting `add_etags=False`. @@ -499,53 +504,56 @@ def send_file( Please never pass filenames to this function from user sources; you should use :func:`send_from_directory` instead. - .. versionadded:: 0.2 + .. versionchanged:: 2.0 + Passing a file-like object that inherits from + :class:`~io.TextIOBase` will raise a :exc:`ValueError` rather + than sending an empty file. - .. versionadded:: 0.5 - The `add_etags`, `cache_timeout` and `conditional` parameters were - added. The default behavior is now to attach etags. + .. versionchanged:: 1.1 + ``filename`` may be a :class:`~os.PathLike` object. - .. versionchanged:: 0.7 - mimetype guessing and etag support for file objects was - deprecated because it was unreliable. Pass a filename if you are - able to, otherwise attach an etag yourself. This functionality - will be removed in Flask 1.0 + .. versionchanged:: 1.1 + Passing a :class:`~io.BytesIO` object supports range requests. - .. versionchanged:: 0.9 - cache_timeout pulls its default from application config, when None. - - .. versionchanged:: 0.12 - The filename is no longer automatically inferred from file objects. If - you want to use automatic mimetype and etag support, pass a filepath via - `filename_or_fp` or `attachment_filename`. - - .. versionchanged:: 0.12 - The `attachment_filename` is preferred over `filename` for MIME-type - detection. + .. versionchanged:: 1.0.3 + Filenames are encoded with ASCII instead of Latin-1 for broader + compatibility with WSGI servers. .. versionchanged:: 1.0 UTF-8 filenames, as specified in `RFC 2231`_, are supported. .. _RFC 2231: https://tools.ietf.org/html/rfc2231#section-4 - .. versionchanged:: 1.0.3 - Filenames are encoded with ASCII instead of Latin-1 for broader - compatibility with WSGI servers. + .. versionchanged:: 0.12 + The filename is no longer automatically inferred from file + objects. If you want to use automatic MIME and etag support, pass + a filename via ``filename_or_fp`` or ``attachment_filename``. - .. versionchanged:: 1.1 - Filename may be a :class:`~os.PathLike` object. + .. versionchanged:: 0.12 + ``attachment_filename`` is preferred over ``filename`` for MIME + detection. - .. versionadded:: 1.1 - Partial content supports :class:`~io.BytesIO`. + .. versionchanged:: 0.9 + ``cache_timeout`` defaults to + :meth:`Flask.get_send_file_max_age`. - :param filename_or_fp: the filename of the file to send. - This is relative to the :attr:`~Flask.root_path` - if a relative path is specified. - Alternatively a file object might be provided in - which case ``X-Sendfile`` might not work and fall - back to the traditional method. Make sure that the - file pointer is positioned at the start of data to - send before calling :func:`send_file`. + .. versionchanged:: 0.7 + MIME guessing and etag support for file-like objects was + deprecated because it was unreliable. Pass a filename if you are + able to, otherwise attach an etag yourself. This functionality + will be removed in Flask 1.0. + + .. versionadded:: 0.5 + The ``add_etags``, ``cache_timeout`` and ``conditional`` + parameters were added. The default behavior is to add etags. + + .. versionadded:: 0.2 + + :param filename_or_fp: The filename of the file to send, relative to + :attr:`~Flask.root_path` if a relative path is specified. + Alternatively, a file-like object opened in binary mode. Make + sure the file pointer is seeked to the start of the data. + ``X-Sendfile`` will only be used with filenames. :param mimetype: the mimetype of the file if provided. If a file path is given, auto detection happens as fallback, otherwise an error will be raised. @@ -620,25 +628,29 @@ def send_file( if current_app.use_x_sendfile and filename: if file is not None: file.close() + headers["X-Sendfile"] = filename fsize = os.path.getsize(filename) - headers["Content-Length"] = fsize data = None else: if file is None: file = open(filename, "rb") mtime = os.path.getmtime(filename) fsize = os.path.getsize(filename) - headers["Content-Length"] = fsize elif isinstance(file, io.BytesIO): try: fsize = file.getbuffer().nbytes except AttributeError: # Python 2 doesn't have getbuffer fsize = len(file.getvalue()) - headers["Content-Length"] = fsize + elif isinstance(file, io.TextIOBase): + raise ValueError("Files must be opened in binary mode or use BytesIO.") + data = wrap_file(request.environ, file) + if fsize is not None: + headers["Content-Length"] = fsize + rv = current_app.response_class( data, mimetype=mimetype, headers=headers, direct_passthrough=True ) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 21735af1..c0138a3c 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -24,6 +24,7 @@ from werkzeug.http import parse_options_header import flask from flask import json +from flask._compat import PY2 from flask._compat import StringIO from flask._compat import text_type from flask.helpers import get_debug_flag @@ -446,6 +447,14 @@ class TestJSON(object): assert lines == sorted_by_str +class PyStringIO(object): + def __init__(self, *args, **kwargs): + self._io = io.BytesIO(*args, **kwargs) + + def __getattr__(self, name): + return getattr(self._io, name) + + class TestSendfile(object): def test_send_file_regular(self, app, req_ctx): rv = flask.send_file("static/index.html") @@ -473,7 +482,7 @@ class TestSendfile(object): @app.route("/") def index(): return flask.send_file( - StringIO("party like it's"), + io.BytesIO(b"party like it's"), last_modified=last_modified, mimetype="text/plain", ) @@ -483,66 +492,54 @@ class TestSendfile(object): def test_send_file_object_without_mimetype(self, app, req_ctx): with pytest.raises(ValueError) as excinfo: - flask.send_file(StringIO("LOL")) + flask.send_file(io.BytesIO(b"LOL")) assert "Unable to infer MIME-type" in str(excinfo.value) assert "no filename is available" in str(excinfo.value) - flask.send_file(StringIO("LOL"), attachment_filename="filename") - - def test_send_file_object(self, app, req_ctx): - with open(os.path.join(app.root_path, "static/index.html"), mode="rb") as f: - rv = flask.send_file(f, mimetype="text/html") - rv.direct_passthrough = False - with app.open_resource("static/index.html") as f: - assert rv.data == f.read() - assert rv.mimetype == "text/html" - rv.close() + flask.send_file(io.BytesIO(b"LOL"), attachment_filename="filename") + @pytest.mark.parametrize( + "opener", + [ + lambda app: open(os.path.join(app.static_folder, "index.html"), "rb"), + lambda app: io.BytesIO(b"Test"), + pytest.param( + lambda app: StringIO("Test"), + marks=pytest.mark.skipif(not PY2, reason="Python 2 only"), + ), + lambda app: PyStringIO(b"Test"), + ], + ) + @pytest.mark.usefixtures("req_ctx") + def test_send_file_object(self, app, opener): + file = opener(app) app.use_x_sendfile = True - - with open(os.path.join(app.root_path, "static/index.html")) as f: - rv = flask.send_file(f, mimetype="text/html") - assert rv.mimetype == "text/html" - assert "x-sendfile" not in rv.headers - rv.close() - - app.use_x_sendfile = False - f = StringIO("Test") - rv = flask.send_file(f, mimetype="application/octet-stream") + rv = flask.send_file(file, mimetype="text/plain") rv.direct_passthrough = False - assert rv.data == b"Test" - assert rv.mimetype == "application/octet-stream" - rv.close() - - class PyStringIO(object): - def __init__(self, *args, **kwargs): - self._io = StringIO(*args, **kwargs) - - def __getattr__(self, name): - return getattr(self._io, name) - - f = PyStringIO("Test") - f.name = "test.txt" - rv = flask.send_file(f, attachment_filename=f.name) - rv.direct_passthrough = False - assert rv.data == b"Test" + assert rv.data assert rv.mimetype == "text/plain" - rv.close() - - f = StringIO("Test") - rv = flask.send_file(f, mimetype="text/plain") - rv.direct_passthrough = False - assert rv.data == b"Test" - assert rv.mimetype == "text/plain" - rv.close() - - app.use_x_sendfile = True - - f = StringIO("Test") - rv = flask.send_file(f, mimetype="text/html") assert "x-sendfile" not in rv.headers rv.close() + @pytest.mark.parametrize( + "opener", + [ + lambda app: io.StringIO(u"Test"), + pytest.param( + lambda app: open(os.path.join(app.static_folder, "index.html")), + marks=pytest.mark.skipif(PY2, reason="Python 3 only"), + ), + ], + ) + @pytest.mark.usefixtures("req_ctx") + def test_send_file_text_fails(self, app, opener): + file = opener(app) + + with pytest.raises(ValueError): + flask.send_file(file, mimetype="text/plain") + + file.close() + def test_send_file_pathlike(self, app, req_ctx): rv = flask.send_file(FakePath("static/index.html")) assert rv.direct_passthrough @@ -630,10 +627,6 @@ class TestSendfile(object): assert rv.data == b"somethingsomething"[4:16] rv.close() - @pytest.mark.skipif( - not callable(getattr(Range, "to_content_range_header", None)), - reason="not implemented within werkzeug", - ) def test_send_file_range_request_xsendfile_invalid(self, app, client): # https://github.com/pallets/flask/issues/2526 app.use_x_sendfile = True @@ -649,7 +642,7 @@ class TestSendfile(object): def test_attachment(self, app, req_ctx): app = flask.Flask(__name__) with app.test_request_context(): - with open(os.path.join(app.root_path, "static/index.html")) as f: + with open(os.path.join(app.root_path, "static/index.html"), "rb") as f: rv = flask.send_file( f, as_attachment=True, attachment_filename="index.html" ) @@ -657,7 +650,7 @@ class TestSendfile(object): assert value == "attachment" rv.close() - with open(os.path.join(app.root_path, "static/index.html")) as f: + with open(os.path.join(app.root_path, "static/index.html"), "rb") as f: rv = flask.send_file( f, as_attachment=True, attachment_filename="index.html" ) @@ -674,7 +667,7 @@ class TestSendfile(object): rv.close() rv = flask.send_file( - StringIO("Test"), + io.BytesIO(b"Test"), as_attachment=True, attachment_filename="index.txt", add_etags=False,