Merge pull request #3435 from pallets/send-file-text

send_file doesn't allow StringIO

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

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
+    .. versionchanged:: 1.1
-       The `add_etags`, `cache_timeout` and `conditional` parameters were
+        ``filename`` may be a :class:`~os.PathLike` object.
-       added.  The default behavior is now to attach etags.
 
-    .. versionchanged:: 0.7
+    .. versionchanged:: 1.1
-       mimetype guessing and etag support for file objects was
+        Passing a :class:`~io.BytesIO` object supports range requests.
-       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:: 0.9
+    .. versionchanged:: 1.0.3
-       cache_timeout pulls its default from application config, when None.
+        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 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
         UTF-8 filenames, as specified in `RFC 2231`_, are supported.
 
     .. _RFC 2231: https://tools.ietf.org/html/rfc2231#section-4
 
-    .. versionchanged:: 1.0.3
+    .. versionchanged:: 0.12
-        Filenames are encoded with ASCII instead of Latin-1 for broader
+       The filename is no longer automatically inferred from file
-        compatibility with WSGI servers.
+       objects. If you want to use automatic MIME and etag support, pass
+       a filename via ``filename_or_fp`` or ``attachment_filename``.
 
-    .. versionchanged:: 1.1
+    .. versionchanged:: 0.12
-        Filename may be a :class:`~os.PathLike` object.
+       ``attachment_filename`` is preferred over ``filename`` for MIME
+       detection.
 
-    .. versionadded:: 1.1
+    .. versionchanged:: 0.9
-        Partial content supports :class:`~io.BytesIO`.
+       ``cache_timeout`` defaults to
+       :meth:`Flask.get_send_file_max_age`.
 
-    :param filename_or_fp: the filename of the file to send.
+    .. versionchanged:: 0.7
-                           This is relative to the :attr:`~Flask.root_path`
+       MIME guessing and etag support for file-like objects was
-                           if a relative path is specified.
+       deprecated because it was unreliable. Pass a filename if you are
-                           Alternatively a file object might be provided in
+       able to, otherwise attach an etag yourself. This functionality
-                           which case ``X-Sendfile`` might not work and fall
+       will be removed in Flask 1.0.
-                           back to the traditional method.  Make sure that the
+
-                           file pointer is positioned at the start of data to
+    .. versionadded:: 0.5
-                           send before calling :func:`send_file`.
+       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
     )

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")
+        flask.send_file(io.BytesIO(b"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()
 
+    @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
-
+        rv = flask.send_file(file, mimetype="text/plain")
-        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.direct_passthrough = False
-        assert rv.data == b"Test"
+        assert rv.data
-        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.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,