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.

Author: rduplain

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

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

flask/app.py

@@ -1041,12 +1041,6 @@ def _register_error_handler(self, key, code_or_exception, f):
             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.

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 @@ def jinja_loader(self):
             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 @@ def send_static_file(self, filename):
         """
         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

flask/testsuite/blueprints.py

@@ -386,6 +386,29 @@ def test_templates_and_static(self):
         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())

flask/testsuite/helpers.py

@@ -237,28 +237,37 @@ def test_static_file(self):
         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):