From fda14678c07d036ef3a1984a4e346e793cc5a63c Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sat, 7 Aug 2010 13:36:39 +0200 Subject: [PATCH] Deprecated send_file etag support and mimetype guessing for file-like objects. This fixes #104 --- CHANGES | 4 ++ docs/upgrading.rst | 17 ++++++++ flask/helpers.py | 27 +++++++++++- tests/flask_tests.py | 100 +++++++++++++++++++++++++++++-------------- 4 files changed, 116 insertions(+), 32 deletions(-) diff --git a/CHANGES b/CHANGES index 6d0a0172..bb5b295d 100644 --- a/CHANGES +++ b/CHANGES @@ -13,6 +13,10 @@ Release date to be announced, codename to be selected behaviour for `OPTIONS` responses. - Unbound locals now raise a proper :exc:`RuntimeError` instead of an :exc:`AttributeError`. +- Mimetype guessing and etag support based on file objects is now + deprecated for :func:`flask.send_file` because it was unreliable. + Pass filenames instead or attach your own etags and provide a + proper mimetype by hand. Version 0.6.1 ------------- diff --git a/docs/upgrading.rst b/docs/upgrading.rst index ba8e9947..ac811e41 100644 --- a/docs/upgrading.rst +++ b/docs/upgrading.rst @@ -27,6 +27,23 @@ raise a :exc:`RuntimeError` instead of an :exc:`AttributeError` when they are unbound. If you cought these exceptions with :exc:`AttributeError` before, you should catch them with :exc:`RuntimeError` now. +Additionally the :func:`~flask.send_file` function is now issuing +deprecation warnings if you depend on functionality that will be removed +in Flask 1.0. Previously it was possible to use etags and mimetypes +when file objects were passed. This was unreliable and caused issues +for a few setups. If you get a deprecation warning, make sure to +update your application to work with either filenames there or disable +etag attaching and attach them yourself. + +Old code:: + + return send_file(my_file_object) + return send_file(my_file_object) + +New code:: + + return send_file(my_file_object, add_etags=False) + Version 0.6 ----------- diff --git a/flask/helpers.py b/flask/helpers.py index a6c18a33..18eb6d0e 100644 --- a/flask/helpers.py +++ b/flask/helpers.py @@ -259,7 +259,9 @@ def send_file(filename_or_fp, mimetype=None, as_attachment=False, 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 sent certain files as attachment (HTML for instance). + to sent certain files as attachment (HTML for instance). The mimetype + guessing requires a `filename` or an `attachment_filename` to be + provided. Please never pass filenames to this function from user sources without checking them first. Something like this is usually sufficient to @@ -274,6 +276,12 @@ def send_file(filename_or_fp, mimetype=None, as_attachment=False, The `add_etags`, `cache_timeout` and `conditional` parameters were added. The default behaviour is now to attach etags. + .. 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 + :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. @@ -295,8 +303,25 @@ def send_file(filename_or_fp, mimetype=None, as_attachment=False, filename = filename_or_fp file = None else: + from warnings import warn file = filename_or_fp filename = getattr(file, 'name', None) + + # XXX: this behaviour is now deprecated because it was unreliable. + # removed in Flask 1.0 + if not attachment_filename and not mimetype \ + and isinstance(filename, basestring): + warn(DeprecationWarning('The filename support for file objects ' + 'passed to send_file is not deprecated. Pass an ' + 'attach_filename if you want mimetypes to be guessed.'), + stacklevel=2) + if add_etags: + warn(DeprecationWarning('In future flask releases etags will no ' + 'longer be generated for file objects passed to the send_file ' + 'function because this behaviour was unreliable. Pass ' + 'filenames instead if possible, otherwise attach an etag ' + 'yourself based on another value'), stacklevel=2) + if filename is not None: if not os.path.isabs(filename): filename = os.path.join(current_app.root_path, filename) diff --git a/tests/flask_tests.py b/tests/flask_tests.py index 2f90a2f3..392368e7 100644 --- a/tests/flask_tests.py +++ b/tests/flask_tests.py @@ -33,8 +33,27 @@ TEST_KEY = 'foo' SECRET_KEY = 'devkey' +@contextmanager +def catch_warnings(): + """Catch warnings in a with block in a list""" + import warnings + filters = warnings.filters + warnings.filters = filters[:] + old_showwarning = warnings.showwarning + log = [] + def showwarning(message, category, filename, lineno, file=None, line=None): + log.append(locals()) + try: + warnings.showwarning = showwarning + yield log + finally: + warnings.filters = filters + warnings.showwarning = old_showwarning + + @contextmanager def catch_stderr(): + """Catch stderr in a StringIO""" old_stderr = sys.stderr sys.stderr = rv = StringIO() try: @@ -834,46 +853,64 @@ class SendfileTestCase(unittest.TestCase): def test_send_file_object(self): app = flask.Flask(__name__) - with app.test_request_context(): - f = open(os.path.join(app.root_path, 'static/index.html')) - rv = flask.send_file(f) - with app.open_resource('static/index.html') as f: - assert rv.data == f.read() - assert rv.mimetype == 'text/html' + with catch_warnings() as captured: + with app.test_request_context(): + f = open(os.path.join(app.root_path, 'static/index.html')) + rv = flask.send_file(f) + with app.open_resource('static/index.html') as f: + assert rv.data == f.read() + assert rv.mimetype == 'text/html' + # mimetypes + etag + assert len(captured) == 2 app.use_x_sendfile = True - with app.test_request_context(): - f = open(os.path.join(app.root_path, 'static/index.html')) - rv = flask.send_file(f) - 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') + with catch_warnings() as captured: + with app.test_request_context(): + f = open(os.path.join(app.root_path, 'static/index.html')) + rv = flask.send_file(f) + 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') + # mimetypes + etag + assert len(captured) == 2 app.use_x_sendfile = False with app.test_request_context(): - f = StringIO('Test') - rv = flask.send_file(f) - assert rv.data == 'Test' - assert rv.mimetype == 'application/octet-stream' - f = StringIO('Test') - rv = flask.send_file(f, mimetype='text/plain') - assert rv.data == 'Test' - assert rv.mimetype == 'text/plain' + with catch_warnings() as captured: + f = StringIO('Test') + rv = flask.send_file(f) + assert rv.data == 'Test' + assert rv.mimetype == 'application/octet-stream' + # etags + assert len(captured) == 1 + with catch_warnings() as captured: + f = StringIO('Test') + rv = flask.send_file(f, mimetype='text/plain') + assert rv.data == 'Test' + assert rv.mimetype == 'text/plain' + # etags + assert len(captured) == 1 app.use_x_sendfile = True - with app.test_request_context(): - f = StringIO('Test') - rv = flask.send_file(f) - assert 'x-sendfile' not in rv.headers + with catch_warnings() as captured: + with app.test_request_context(): + f = StringIO('Test') + rv = flask.send_file(f) + assert 'x-sendfile' not in rv.headers + # etags + assert len(captured) == 1 def test_attachment(self): app = flask.Flask(__name__) - with app.test_request_context(): - f = open(os.path.join(app.root_path, 'static/index.html')) - rv = flask.send_file(f, as_attachment=True) - value, options = parse_options_header(rv.headers['Content-Disposition']) - assert value == 'attachment' + with catch_warnings() as captured: + with app.test_request_context(): + f = open(os.path.join(app.root_path, 'static/index.html')) + rv = flask.send_file(f, as_attachment=True) + value, options = parse_options_header(rv.headers['Content-Disposition']) + assert value == 'attachment' + # mimetypes + etag + assert len(captured) == 2 with app.test_request_context(): assert options['filename'] == 'index.html' @@ -884,7 +921,8 @@ class SendfileTestCase(unittest.TestCase): with app.test_request_context(): rv = flask.send_file(StringIO('Test'), as_attachment=True, - attachment_filename='index.txt') + attachment_filename='index.txt', + add_etags=False) assert rv.mimetype == 'text/plain' value, options = parse_options_header(rv.headers['Content-Disposition']) assert value == 'attachment'