Skip to content

Commit

Permalink
Remove deprecation warnings for add_etags & mimetype guessing for sen…
Browse files Browse the repository at this point in the history
…d_file()

Fix #1849
  • Loading branch information
dsully authored and untitaker committed Jun 3, 2016
1 parent 3d72099 commit 8458cc5
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 50 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Patches and Suggestions
- Chris Grindstaff
- Christopher Grebs
- Daniel Neuhäuser
- Dan Sully
- David Lord @davidism
- Edmond Burnett
- Florent Xicluna
Expand Down
2 changes: 2 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ 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``.

Version 0.11
------------
Expand Down
31 changes: 9 additions & 22 deletions flask/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -437,11 +437,7 @@ 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``.
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.
You must explicitly provide the mimetype for the filename or file object.
Please never pass filenames to this function from user sources;
you should use :func:`send_from_directory` instead.
Expand All @@ -461,6 +457,11 @@ def send_file(filename_or_fp, mimetype=None, as_attachment=False,
.. versionchanged:: 0.9
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.
:param filename_or_fp: the filename of the file to send in `latin-1`.
This is relative to the :attr:`~Flask.root_path`
if a relative path is specified.
Expand Down Expand Up @@ -488,25 +489,9 @@ 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 behavior is now deprecated because it was unreliable.
# removed in Flask 1.0
if not attachment_filename and not mimetype \
and isinstance(filename, string_types):
warn(DeprecationWarning('The filename support for file objects '
'passed to send_file is now 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 behavior 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)
Expand Down Expand Up @@ -553,7 +538,9 @@ def send_file(filename_or_fp, mimetype=None, as_attachment=False,
rv.cache_control.max_age = cache_timeout
rv.expires = int(time() + cache_timeout)

if add_etags and filename is not None:
if add_etags and filename is not None and file is None:

This comment has been minimized.

Copy link
@magne4000

magne4000 Aug 2, 2016

Contributor

Here, file can only be None if

  • filename was provided
  • current_app.use_x_sendfile is True (see line 513)

I don't think that was the point of this patch, unless there's something I'm missing

from warnings import warn

try:
rv.set_etag('%s-%s-%s' % (
os.path.getmtime(filename),
Expand Down
30 changes: 2 additions & 28 deletions tests/test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ def test_send_file_xsendfile(self, catch_deprecation_warnings):
assert rv.mimetype == 'text/html'
rv.close()

def test_send_file_object(self, recwarn):
def test_send_file_object(self):
app = flask.Flask(__name__)

with app.test_request_context():
Expand All @@ -361,10 +361,6 @@ def test_send_file_object(self, recwarn):
assert rv.mimetype == 'text/html'
rv.close()

# mimetypes + etag
recwarn.pop(DeprecationWarning)
recwarn.pop(DeprecationWarning)

app.use_x_sendfile = True

with app.test_request_context():
Expand All @@ -376,10 +372,6 @@ def test_send_file_object(self, recwarn):
os.path.join(app.root_path, 'static/index.html')
rv.close()

# mimetypes + etag
recwarn.pop(DeprecationWarning)
recwarn.pop(DeprecationWarning)

app.use_x_sendfile = False
with app.test_request_context():
f = StringIO('Test')
Expand All @@ -389,9 +381,6 @@ def test_send_file_object(self, recwarn):
assert rv.mimetype == 'application/octet-stream'
rv.close()

# etags
recwarn.pop(DeprecationWarning)

class PyStringIO(object):
def __init__(self, *args, **kwargs):
self._io = StringIO(*args, **kwargs)
Expand All @@ -405,21 +394,13 @@ def __getattr__(self, name):
assert rv.mimetype == 'text/plain'
rv.close()

# attachment_filename and etags
a = recwarn.pop(DeprecationWarning)
b = recwarn.pop(DeprecationWarning)
c = recwarn.pop(UserWarning) # file not found

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()

# etags
recwarn.pop(DeprecationWarning)

app.use_x_sendfile = True

with app.test_request_context():
Expand All @@ -428,10 +409,7 @@ def __getattr__(self, name):
assert 'x-sendfile' not in rv.headers
rv.close()

# etags
recwarn.pop(DeprecationWarning)

def test_attachment(self, recwarn):
def test_attachment(self):
app = flask.Flask(__name__)
with app.test_request_context():
with open(os.path.join(app.root_path, 'static/index.html')) as f:
Expand All @@ -441,10 +419,6 @@ def test_attachment(self, recwarn):
assert value == 'attachment'
rv.close()

# mimetypes + etag
assert len(recwarn.list) == 2
recwarn.clear()

with app.test_request_context():
assert options['filename'] == 'index.html'
rv = flask.send_file('static/index.html', as_attachment=True)
Expand Down

0 comments on commit 8458cc5

Please sign in to comment.