Permalink
Browse files

Deprecated send_file etag support and mimetype guessing for file-like…

… objects. This fixes #104
  • Loading branch information...
mitsuhiko committed Aug 7, 2010
1 parent faa1c71 commit fda14678c07d036ef3a1984a4e346e793cc5a63c
Showing with 116 additions and 32 deletions.
  1. +4 −0 CHANGES
  2. +17 −0 docs/upgrading.rst
  3. +26 −1 flask/helpers.py
  4. +69 −31 tests/flask_tests.py
View
@@ -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
-------------
View
@@ -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
-----------
View
@@ -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)
View
@@ -33,8 +33,27 @@
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 @@ def test_send_file_xsendfile(self):
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 @@ def test_attachment(self):
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'

0 comments on commit fda1467

Please sign in to comment.