From 26da6a5365e1fd229932f167a299128f30fae154 Mon Sep 17 00:00:00 2001 From: Ron DuPlain Date: Tue, 24 Apr 2012 01:48:05 -0400 Subject: [PATCH] Use default send_file max-age consistently. Prior to this commit, the send_file max-age hook and config were only used for the static file handler. Now they are used when calling helpers.send_file directly. --- CHANGES | 17 ++++++------ docs/config.rst | 11 +++++--- flask/app.py | 6 ----- flask/helpers.py | 51 ++++++++++++++++++++++------------- flask/testsuite/blueprints.py | 23 ++++++++++++++++ flask/testsuite/helpers.py | 25 +++++++++++------ 6 files changed, 88 insertions(+), 45 deletions(-) diff --git a/CHANGES b/CHANGES index 83c1f4fe..537f6544 100644 --- a/CHANGES +++ b/CHANGES @@ -53,14 +53,15 @@ Relase date to be decided, codename to be chosen. - View functions can now return a tuple with the first instance being an instance of :class:`flask.Response`. This allows for returning ``jsonify(error="error msg"), 400`` from a view function. -- :class:`flask.Flask` now provides a `get_send_file_options` hook for - subclasses to override behavior of serving static files from Flask when using - :meth:`flask.Flask.send_static_file` based on keywords in - :func:`flask.helpers.send_file`. This hook is provided a filename, which for - example allows changing cache controls by file extension. The default - max-age for `send_static_file` can be configured through a new - ``SEND_FILE_MAX_AGE_DEFAULT`` configuration variable, regardless of whether - the `get_send_file_options` hook is used. +- :class:`~flask.Flask` and :class:`~flask.Blueprint` now provide a + :meth:`~flask.Flask.get_send_file_max_age` hook for subclasses to override + behavior of serving static files from Flask when using + :meth:`flask.Flask.send_static_file` (used for the default static file + handler) and :func:`~flask.helpers.send_file`. This hook is provided a + filename, which for example allows changing cache controls by file extension. + The default max-age for `send_file` and static files can be configured + through a new ``SEND_FILE_MAX_AGE_DEFAULT`` configuration variable, which is + used in the default `get_send_file_max_age` implementation. - Fixed an assumption in sessions implementation which could break message flashing on sessions implementations which use external storage. - Changed the behavior of tuple return values from functions. They are no diff --git a/docs/config.rst b/docs/config.rst index 86bfb0d1..7a32fb84 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -111,12 +111,15 @@ The following configuration values are used internally by Flask: content length greater than this by returning a 413 status code. ``SEND_FILE_MAX_AGE_DEFAULT``: Default cache control max age to use with - :meth:`flask.Flask.send_static_file`, in + :meth:`~flask.Flask.send_static_file` (the + default static file handler) and + :func:`~flask.send_file`, in seconds. Override this value on a per-file basis using the - :meth:`flask.Flask.get_send_file_options` and - :meth:`flask.Blueprint.get_send_file_options` - hooks. Defaults to 43200 (12 hours). + :meth:`~flask.Flask.get_send_file_max_age` + hook on :class:`~flask.Flask` or + :class:`~flask.Blueprint`, + respectively. Defaults to 43200 (12 hours). ``TRAP_HTTP_EXCEPTIONS`` If this is set to ``True`` Flask will not execute the error handlers of HTTP exceptions but instead treat the diff --git a/flask/app.py b/flask/app.py index 67383bbe..5f809abb 100644 --- a/flask/app.py +++ b/flask/app.py @@ -1041,12 +1041,6 @@ class Flask(_PackageBoundObject): self.error_handler_spec.setdefault(key, {}).setdefault(None, []) \ .append((code_or_exception, f)) - def get_send_file_options(self, filename): - # Override: Hooks in SEND_FILE_MAX_AGE_DEFAULT config. - options = super(Flask, self).get_send_file_options(filename) - options['cache_timeout'] = self.config['SEND_FILE_MAX_AGE_DEFAULT'] - return options - @setupmethod def template_filter(self, name=None): """A decorator that is used to register custom template filter. diff --git a/flask/helpers.py b/flask/helpers.py index 5560d38c..05f84ef7 100644 --- a/flask/helpers.py +++ b/flask/helpers.py @@ -406,7 +406,7 @@ def get_flashed_messages(with_categories=False, category_filter=[]): def send_file(filename_or_fp, mimetype=None, as_attachment=False, attachment_filename=None, add_etags=True, - cache_timeout=60 * 60 * 12, conditional=False): + cache_timeout=None, conditional=False): """Sends the contents of a file to the client. This will use the most efficient method available and configured. By default it will try to use the WSGI server's file_wrapper support. Alternatively @@ -420,10 +420,6 @@ def send_file(filename_or_fp, mimetype=None, as_attachment=False, guessing requires a `filename` or an `attachment_filename` to be provided. - Note `get_send_file_options` in :class:`flask.Flask` hooks the - ``SEND_FILE_MAX_AGE_DEFAULT`` configuration variable to set the default - cache_timeout. - Please never pass filenames to this function from user sources without checking them first. Something like this is usually sufficient to avoid security problems:: @@ -443,6 +439,9 @@ def send_file(filename_or_fp, mimetype=None, as_attachment=False, able to, otherwise attach an etag yourself. This functionality will be removed in Flask 1.0 + .. versionchanged:: 0.9 + cache_timeout pulls its default from application config, when None. + :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. @@ -459,7 +458,11 @@ def send_file(filename_or_fp, mimetype=None, as_attachment=False, differs from the file's filename. :param add_etags: set to `False` to disable attaching of etags. :param conditional: set to `True` to enable conditional responses. - :param cache_timeout: the timeout in seconds for the headers. + + :param cache_timeout: the timeout in seconds for the headers. When `None` + (default), this value is set by + :meth:`~Flask.get_send_file_max_age` of + :data:`~flask.current_app`. """ mtime = None if isinstance(filename_or_fp, basestring): @@ -523,6 +526,8 @@ def send_file(filename_or_fp, mimetype=None, as_attachment=False, rv.last_modified = int(mtime) rv.cache_control.public = True + if cache_timeout is None: + cache_timeout = current_app.get_send_file_max_age(filename) if cache_timeout: rv.cache_control.max_age = cache_timeout rv.expires = int(time() + cache_timeout) @@ -757,26 +762,31 @@ class _PackageBoundObject(object): return FileSystemLoader(os.path.join(self.root_path, self.template_folder)) - def get_send_file_options(self, filename): - """Provides keyword arguments to send to :func:`send_from_directory`. + def get_send_file_max_age(self, filename): + """Provides default cache_timeout for the :func:`send_file` functions. + + By default, this function returns ``SEND_FILE_MAX_AGE_DEFAULT`` from + the configuration of :data:`~flask.current_app`. + + Static file functions such as :func:`send_from_directory` use this + function, and :func:`send_file` calls this function on + :data:`~flask.current_app` when the given cache_timeout is `None`. If a + cache_timeout is given in :func:`send_file`, that timeout is used; + otherwise, this method is called. This allows subclasses to change the behavior when sending files based on the filename. For example, to set the cache timeout for .js files - to 60 seconds (note the options are keywords for :func:`send_file`):: + to 60 seconds:: class MyFlask(flask.Flask): - def get_send_file_options(self, filename): - options = super(MyFlask, self).get_send_file_options(filename) - if filename.lower().endswith('.js'): - options['cache_timeout'] = 60 - options['conditional'] = True - return options + def get_send_file_max_age(self, name): + if name.lower().endswith('.js'): + return 60 + return flask.Flask.get_send_file_max_age(self, name) .. versionadded:: 0.9 """ - options = {} - options['cache_timeout'] = current_app.config['SEND_FILE_MAX_AGE_DEFAULT'] - return options + return current_app.config['SEND_FILE_MAX_AGE_DEFAULT'] def send_static_file(self, filename): """Function used internally to send static files from the static @@ -786,8 +796,11 @@ class _PackageBoundObject(object): """ if not self.has_static_folder: raise RuntimeError('No static folder for this object') + # Ensure get_send_file_max_age is called in all cases. + # Here, we ensure get_send_file_max_age is called for Blueprints. + cache_timeout = self.get_send_file_max_age(filename) return send_from_directory(self.static_folder, filename, - **self.get_send_file_options(filename)) + cache_timeout=cache_timeout) def open_resource(self, resource, mode='rb'): """Opens a resource from the application's resource folder. To see diff --git a/flask/testsuite/blueprints.py b/flask/testsuite/blueprints.py index 5f3d3ab3..c9622121 100644 --- a/flask/testsuite/blueprints.py +++ b/flask/testsuite/blueprints.py @@ -386,6 +386,29 @@ class BlueprintTestCase(FlaskTestCase): with flask.Flask(__name__).test_request_context(): self.assert_equal(flask.render_template('nested/nested.txt'), 'I\'m nested') + def test_default_static_cache_timeout(self): + app = flask.Flask(__name__) + class MyBlueprint(flask.Blueprint): + def get_send_file_max_age(self, filename): + return 100 + + blueprint = MyBlueprint('blueprint', __name__, static_folder='static') + app.register_blueprint(blueprint) + + # try/finally, in case other tests use this app for Blueprint tests. + max_age_default = app.config['SEND_FILE_MAX_AGE_DEFAULT'] + try: + with app.test_request_context(): + unexpected_max_age = 3600 + if app.config['SEND_FILE_MAX_AGE_DEFAULT'] == unexpected_max_age: + unexpected_max_age = 7200 + app.config['SEND_FILE_MAX_AGE_DEFAULT'] = unexpected_max_age + rv = blueprint.send_static_file('index.html') + cc = parse_cache_control_header(rv.headers['Cache-Control']) + self.assert_equal(cc.max_age, 100) + finally: + app.config['SEND_FILE_MAX_AGE_DEFAULT'] = max_age_default + def test_templates_list(self): from blueprintapp import app templates = sorted(app.jinja_env.list_templates()) diff --git a/flask/testsuite/helpers.py b/flask/testsuite/helpers.py index 4781d2d9..a0e60aac 100644 --- a/flask/testsuite/helpers.py +++ b/flask/testsuite/helpers.py @@ -237,28 +237,37 @@ class SendfileTestCase(FlaskTestCase): app = flask.Flask(__name__) # default cache timeout is 12 hours with app.test_request_context(): + # Test with static file handler. rv = app.send_static_file('index.html') cc = parse_cache_control_header(rv.headers['Cache-Control']) self.assert_equal(cc.max_age, 12 * 60 * 60) + # Test again with direct use of send_file utility. + rv = flask.send_file('static/index.html') + cc = parse_cache_control_header(rv.headers['Cache-Control']) + self.assert_equal(cc.max_age, 12 * 60 * 60) app.config['SEND_FILE_MAX_AGE_DEFAULT'] = 3600 with app.test_request_context(): + # Test with static file handler. rv = app.send_static_file('index.html') cc = parse_cache_control_header(rv.headers['Cache-Control']) self.assert_equal(cc.max_age, 3600) - # override get_send_file_options with some new values and check them + # Test again with direct use of send_file utility. + rv = flask.send_file('static/index.html') + cc = parse_cache_control_header(rv.headers['Cache-Control']) + self.assert_equal(cc.max_age, 3600) class StaticFileApp(flask.Flask): - def get_send_file_options(self, filename): - opts = super(StaticFileApp, self).get_send_file_options(filename) - opts['cache_timeout'] = 10 - # this test catches explicit inclusion of the conditional - # keyword arg in the guts - opts['conditional'] = True - return opts + def get_send_file_max_age(self, filename): + return 10 app = StaticFileApp(__name__) with app.test_request_context(): + # Test with static file handler. rv = app.send_static_file('index.html') cc = parse_cache_control_header(rv.headers['Cache-Control']) self.assert_equal(cc.max_age, 10) + # Test again with direct use of send_file utility. + rv = flask.send_file('static/index.html') + cc = parse_cache_control_header(rv.headers['Cache-Control']) + self.assert_equal(cc.max_age, 10) class LoggingTestCase(FlaskTestCase):