From 71e10be28665eee6a919045f9e5d8773508dc756 Mon Sep 17 00:00:00 2001 From: Markus Unterwaditzer Date: Fri, 26 Aug 2016 03:08:03 +0200 Subject: [PATCH] Properly remove f.name usage in send_file (#1988) * Properly remove f.name usage in send_file * Update changelogs * Fix tests --- CHANGES | 6 +++-- docs/upgrading.rst | 40 ++++++++++++++++++++++++++++++- flask/helpers.py | 56 ++++++++++++++++++++++++++++++------------- tests/test_helpers.py | 31 ++++++++++++++++-------- 4 files changed, 103 insertions(+), 30 deletions(-) diff --git a/CHANGES b/CHANGES index 9e13bd71..5273e09d 100644 --- a/CHANGES +++ b/CHANGES @@ -7,8 +7,10 @@ Version 0.12 ------------ - the cli command now responds to `--version`. -- Mimetype guessing for ``send_file`` has been removed, as per issue ``#104``. - See pull request ``#1849``. +- Mimetype guessing and ETag generation for file-like objects in ``send_file`` + has been removed, as per issue ``#104``. See pull request ``#1849``. +- Mimetype guessing in ``send_file`` now fails loudly and doesn't fall back to + ``application/octet-stream``. See pull request ``#1988``. - Make ``flask.safe_join`` able to join multiple paths like ``os.path.join`` (pull request ``#1730``). diff --git a/docs/upgrading.rst b/docs/upgrading.rst index a85fb0fa..6b933c59 100644 --- a/docs/upgrading.rst +++ b/docs/upgrading.rst @@ -19,7 +19,45 @@ providing the ``--upgrade`` parameter:: $ pip install --upgrade Flask -.. _upgrading-to-10: +.. _upgrading-to-012: + +Version 0.12 +------------ + +Changes to send_file +```````````````````` + +The ``filename`` is no longer automatically inferred from file-like objects. +This means that the following code will no longer automatically have +``X-Sendfile`` support, etag generation or MIME-type guessing:: + + response = send_file(open('/path/to/file.txt')) + +Any of the following is functionally equivalent:: + + fname = '/path/to/file.txt' + + # Just pass the filepath directly + response = send_file(fname) + + # Set the MIME-type and ETag explicitly + response = send_file(open(fname), mimetype='text/plain') + response.set_etag(...) + + # Set `attachment_filename` for MIME-type guessing + # ETag still needs to be manually set + response = send_file(open(fname), attachment_filename=fname) + response.set_etag(...) + +The reason for this is that some file-like objects have a invalid or even +misleading ``name`` attribute. Silently swallowing errors in such cases was not +a satisfying solution. + +Additionally the default of falling back to ``application/octet-stream`` has +been removed. If Flask can't guess one or the user didn't provide one, the +function fails. + +.. _upgrading-to-011: Version 0.11 ------------ diff --git a/flask/helpers.py b/flask/helpers.py index 4129ed30..e8422f7a 100644 --- a/flask/helpers.py +++ b/flask/helpers.py @@ -437,7 +437,14 @@ def send_file(filename_or_fp, mimetype=None, as_attachment=False, to ``True`` to directly emit an ``X-Sendfile`` header. This however requires support of the underlying webserver for ``X-Sendfile``. - You must explicitly provide the mimetype for the filename or file object. + By default it will try to guess the mimetype for you, but you can + also explicitly provide one. For extra security you probably want + to send certain files as attachment (HTML for instance). The mimetype + guessing requires a `filename` or an `attachment_filename` to be + provided. + + ETags will also be attached automatically if a `filename` is provided. You + can turn this off by setting `add_etags=False`. Please never pass filenames to this function from user sources; you should use :func:`send_from_directory` instead. @@ -458,9 +465,13 @@ def send_file(filename_or_fp, mimetype=None, as_attachment=False, cache_timeout pulls its default from application config, when None. .. versionchanged:: 0.12 - mimetype guessing and etag support removed for file objects. - If no mimetype or attachment_filename is provided, application/octet-stream - will be used. + 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. :param filename_or_fp: the filename of the file to send in `latin-1`. This is relative to the :attr:`~Flask.root_path` @@ -470,8 +481,9 @@ def send_file(filename_or_fp, mimetype=None, as_attachment=False, 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`. - :param mimetype: the mimetype of the file if provided, otherwise - auto detection happens. + :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. :param as_attachment: set to ``True`` if you want to send this file with a ``Content-Disposition: attachment`` header. :param attachment_filename: the filename for the attachment if it @@ -490,26 +502,36 @@ def send_file(filename_or_fp, mimetype=None, as_attachment=False, mtime = None if isinstance(filename_or_fp, string_types): filename = filename_or_fp + if not os.path.isabs(filename): + filename = os.path.join(current_app.root_path, filename) file = None + if attachment_filename is None: + attachment_filename = os.path.basename(filename) else: file = filename_or_fp - filename = getattr(file, 'name', None) + filename = None - if filename is not None: - if not os.path.isabs(filename): - filename = os.path.join(current_app.root_path, filename) - if mimetype is None and (filename or attachment_filename): - mimetype = mimetypes.guess_type(filename or attachment_filename)[0] if mimetype is None: - mimetype = 'application/octet-stream' + if attachment_filename is not None: + mimetype = mimetypes.guess_type(attachment_filename)[0] + + if mimetype is None: + if attachment_filename is not None: + raise ValueError( + 'Unable to infer MIME-type from filename {!r}, please ' + 'pass one explicitly.'.format(mimetype_filename) + ) + raise ValueError( + 'Unable to infer MIME-type because no filename is available. ' + 'Please set either `attachment_filename`, pass a filepath to ' + '`filename_or_fp` or set your own MIME-type via `mimetype`.' + ) headers = Headers() if as_attachment: if attachment_filename is None: - if filename is None: - raise TypeError('filename unavailable, required for ' - 'sending as attachment') - attachment_filename = os.path.basename(filename) + raise TypeError('filename unavailable, required for ' + 'sending as attachment') headers.add('Content-Disposition', 'attachment', filename=attachment_filename) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 610e18ea..0f9a8e27 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -384,18 +384,30 @@ class TestSendfile(object): @app.route('/') def index(): - return flask.send_file(StringIO("party like it's"), last_modified=last_modified) + return flask.send_file(StringIO("party like it's"), + last_modified=last_modified, + mimetype='text/plain') c = app.test_client() rv = c.get('/') assert rv.last_modified == last_modified + def test_send_file_object_without_mimetype(self): + app = flask.Flask(__name__) + + with app.test_request_context(): + with pytest.raises(ValueError) as excinfo: + flask.send_file(StringIO("LOL")) + + assert 'Unable to infer MIME-type' in str(excinfo) + assert 'no filename is available' in str(excinfo) + def test_send_file_object(self): app = flask.Flask(__name__) with app.test_request_context(): with open(os.path.join(app.root_path, 'static/index.html'), mode='rb') as f: - rv = flask.send_file(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() @@ -406,17 +418,15 @@ class TestSendfile(object): with app.test_request_context(): with open(os.path.join(app.root_path, 'static/index.html')) as f: - rv = flask.send_file(f) + rv = flask.send_file(f, mimetype='text/html') assert rv.mimetype == 'text/html' - assert 'x-sendfile' in rv.headers - assert rv.headers['x-sendfile'] == \ - os.path.join(app.root_path, 'static/index.html') + assert 'x-sendfile' not in rv.headers rv.close() app.use_x_sendfile = False with app.test_request_context(): f = StringIO('Test') - rv = flask.send_file(f) + rv = flask.send_file(f, mimetype='application/octet-stream') rv.direct_passthrough = False assert rv.data == b'Test' assert rv.mimetype == 'application/octet-stream' @@ -429,7 +439,7 @@ class TestSendfile(object): return getattr(self._io, name) f = PyStringIO('Test') f.name = 'test.txt' - rv = flask.send_file(f) + rv = flask.send_file(f, attachment_filename=f.name) rv.direct_passthrough = False assert rv.data == b'Test' assert rv.mimetype == 'text/plain' @@ -446,7 +456,7 @@ class TestSendfile(object): with app.test_request_context(): f = StringIO('Test') - rv = flask.send_file(f) + rv = flask.send_file(f, mimetype='text/html') assert 'x-sendfile' not in rv.headers rv.close() @@ -454,7 +464,8 @@ class TestSendfile(object): app = flask.Flask(__name__) with app.test_request_context(): with open(os.path.join(app.root_path, 'static/index.html')) as f: - rv = flask.send_file(f, as_attachment=True) + rv = flask.send_file(f, as_attachment=True, + attachment_filename='index.html') value, options = \ parse_options_header(rv.headers['Content-Disposition']) assert value == 'attachment'