Adding HTTP browser caching of UI elements (#4450)

changedetectionio/flask_app.py

@@ -19,6 +19,7 @@ from flask import (
     Flask,
     abort,
     flash,
+    g,
     redirect,
     render_template,
     request,
@@ -26,6 +27,7 @@ from flask import (
     session,
     url_for,
 )
+from flask.sessions import SecureCookieSessionInterface
 from flask_cors import CORS
 from flask_restful import Api, abort
 
@@ -139,7 +141,8 @@ if strtobool(os.getenv("FLASK_ENABLE_COMPRESSION")):
 app.config['TEMPLATES_AUTO_RELOAD'] = False
 
 
-# Stop browser caching of assets
+# Default to revalidate-always for anything served with send_file(); static_content() then
+# opts the fingerprinted asset URLs into real caching (see _fingerprint_static_urls).
 app.config['SEND_FILE_MAX_AGE_DEFAULT'] = 0
 app.config.exit = Event()
 
@@ -291,6 +294,103 @@ def get_css_version():
     return hashlib.sha256(f"{salt}{__version__}".encode()).hexdigest()[:10]
 
 
+# Static groups that are plain files on disk under changedetectionio/static/<group>/ - the
+# same bytes for every visitor, so they can be fingerprinted and cached hard. Deliberately
+# excludes the dynamic groups handled inside static_content() ('screenshot', 'favicon',
+# 'visual_selector_data', 'plugin'), which are per-watch and/or password protected.
+STATIC_CACHEABLE_GROUPS = frozenset(['favicons', 'images', 'js', 'styles'])
+
+# How long a fingerprint is trusted before the file is stat()ed again. The lookup sits in the
+# hot path (one watch-list render emits hundreds of asset url_for() calls) so it can't stat
+# per URL, but the URLs we hand out are served `immutable` - an edited .js/.css that never
+# re-fingerprinted would be pinned in the browser for a year. A few seconds of staleness is
+# the compromise: invisible in production (files only change on upgrade, and the container
+# restarts) and self-correcting while developing.
+STATIC_FINGERPRINT_TTL = 10.0
+_static_fingerprints = {}
+_static_fingerprints_expires = 0.0
+
+
+def get_static_fingerprint(group, filename):
+    """Short token identifying this exact revision of a static file, for `?v=` cache busting.
+
+    Built from the file's own mtime+size rather than get_css_version()'s app-version token:
+    a version-wide token silently serves a stale asset whenever content changes without a
+    release (local dev, a patched image, a rebuilt styles.css), which is not survivable once
+    the response says `immutable`. Returns '' when the file can't be stat()ed, so the request
+    stays unversioned (and revalidating) rather than being pinned under a made-up token.
+    """
+    global _static_fingerprints_expires
+
+    now = time.monotonic()
+    if now > _static_fingerprints_expires:
+        _static_fingerprints.clear()
+        _static_fingerprints_expires = now + STATIC_FINGERPRINT_TTL
+
+    key = (group, filename)
+    token = _static_fingerprints.get(key)
+    if token is None:
+        try:
+            st = os.stat(os.path.join(app.static_folder, group, filename))
+            token = f"{int(st.st_mtime)}-{st.st_size}"
+        except OSError:
+            token = ''
+        _static_fingerprints[key] = token
+
+    return token
+
+
+@app.url_defaults
+def _fingerprint_static_urls(endpoint, values):
+    """Pin every static asset URL to the revision of the file it resolves to.
+
+    Doing it here rather than in the templates means an asset can't be added without its
+    cache-buster - the `?v=` is what lets static_content() answer with a year-long
+    `immutable` instead of making the browser revalidate on every page load.
+    """
+    if endpoint != 'static_content' or 'v' in values:
+        return
+
+    if values.get('group') in STATIC_CACHEABLE_GROUPS:
+        token = get_static_fingerprint(values['group'], values.get('filename', ''))
+        if token:
+            values['v'] = token
+
+
+class PublicStaticAssetSessionInterface(SecureCookieSessionInterface):
+    """Keeps "Vary: Cookie" and the session cookie refresh off public static asset responses.
+
+    Flask tags any response whose session was touched with "Vary: Cookie", and flask_login's
+    auth check touches it on every single request. Since Flask 3.1.3 the request context sets
+    `session.accessed` itself, so a view or an after_request hook can't opt out - the header is
+    added in save_session(), which runs last. It has to go for the files marked by
+    static_content(): our session cookie is permanent and re-signed (fresh timestamp) on every
+    response, so the Cookie request header keeps changing, and a browser honouring
+    "Vary: Cookie" would then miss its cache on every asset of every page load - the immutable
+    caching would never be used at all. Nothing in those groups depends on the session.
+    """
+
+    def save_session(self, app, session, response):
+        public_asset = g.get('public_static_asset', False)
+
+        if public_asset and not session.modified:
+            # Same bytes for every visitor and nothing to persist: skip the cookie refresh
+            # and the Vary entirely.
+            return
+
+        super().save_session(app, session, response)
+
+        if public_asset and 'Set-Cookie' in response.headers:
+            # Shouldn't happen (these requests don't write to the session), but if something
+            # ever does, the response now carries one visitor's cookie - it must not be stored
+            # by a shared cache under the long-lived header static_content() just set.
+            response.cache_control.public = False
+            response.cache_control.private = True
+
+
+app.session_interface = PublicStaticAssetSessionInterface()
+
+
 @app.template_global('filtered_action_url')
 def _filtered_action_url(endpoint, **overrides):
     """Build a URL to `endpoint` carrying the CURRENT watch-list filters (query args)
@@ -1022,6 +1122,9 @@ def changedetection_app(config=None, datastore_o=None):
                 response = make_response(send_from_directory(f"static/flags/{subdir}", svg_file))
                 response.headers['Content-type'] = 'image/svg+xml'
                 response.headers['Cache-Control'] = 'max-age=86400, public'  # Cache for 24 hours
+                # Same for everyone, and the language modal pulls a few hundred of them - see
+                # PublicStaticAssetSessionInterface for why the "Vary: Cookie" has to go.
+                g.public_static_asset = True
                 return response
             except FileNotFoundError:
                 abort(404)
@@ -1171,10 +1274,35 @@ def changedetection_app(config=None, datastore_o=None):
 
         # These files should be in our subdirectory
         try:
-            return send_from_directory(f"static/{group}", path=filename)
+            response = make_response(send_from_directory(f"static/{group}", path=filename))
         except FileNotFoundError:
             abort(404)
 
+        # SEND_FILE_MAX_AGE_DEFAULT=0 means werkzeug hands these out as "no-cache, max-age=0",
+        # so every asset on every page load costs a request - a 304, but still a round trip.
+        # A `?v=` matching the file's current fingerprint (added by _fingerprint_static_urls)
+        # means the caller asked for this exact revision and can keep it for good; the next
+        # upgrade changes the URL, not the cache entry. Anything unversioned - older cached
+        # HTML, a hand-typed or third-party URL - keeps revalidating, where the ETag werkzeug
+        # already set turns the round trip into a 304 rather than a re-download.
+        if group in STATIC_CACHEABLE_GROUPS and request.args.get('v') == get_static_fingerprint(
+            group, filename
+        ):
+            response.headers['Cache-Control'] = 'public, max-age=31536000, immutable'
+            # werkzeug derived an "Expires: <now>" from SEND_FILE_MAX_AGE_DEFAULT=0. Cache-Control
+            # wins over it for anything HTTP/1.1, but leaving the two contradicting each other
+            # means an HTTP/1.0-era cache treats the file as already stale.
+            response.expires = int(time.time()) + 31536000
+        else:
+            response.headers['Cache-Control'] = 'public, max-age=0, must-revalidate'
+
+        # Identical for every visitor (the password-protected groups returned further up), so
+        # let PublicStaticAssetSessionInterface strip the "Vary: Cookie" that would otherwise
+        # key each of these on the caller's cookies and defeat the caching above.
+        g.public_static_asset = True
+
+        return response
+
     import changedetectionio.blueprint.browser_steps as browser_steps
 
     app.register_blueprint(

changedetectionio/templates/base.html

@@ -16,8 +16,9 @@
         {%- endif -%}
     {%- endif -%}
     <link rel="stylesheet" href="{{url_for('static_content', group='styles', filename='pure-min.css')}}" >
-    <link rel="stylesheet" href="{{url_for('static_content', group='styles', filename='flag-icons.min.css')}}" >
-    <link rel="stylesheet" href="{{url_for('static_content', group='styles', filename='styles.css')}}?v={{ get_css_version() }}" >
+    <link rel="stylesheet" href="{{url_for('static_content', group='styles', filename='flag-icons.min.css')}}"  >
+    {# ?v= is appended automatically for static_content (see _fingerprint_static_urls) #}
+    <link rel="stylesheet" href="{{url_for('static_content', group='styles', filename='styles.css')}}" >
     {% if extra_stylesheets %}
       {% for m in extra_stylesheets %}
         <link rel="stylesheet" href="{{ m }}?ver={{ get_css_version() }}" >
@@ -87,8 +88,10 @@
         };
     </script>
     <script src="{{url_for('static_content', group='js', filename='jquery-3.6.0.min.js')}}"></script>
+    <script src="{{url_for('static_content', group='js', filename='feather-icons.min.js')}}"></script>
+    <script src="{{url_for('static_content', group='js', filename='sidebar.js')}}"></script>
+
     <script src="{{url_for('static_content', group='js', filename='csrf.js')}}" defer></script>
-    <script src="{{url_for('static_content', group='js', filename='feather-icons.min.js')}}" defer></script>
     {# Render all <i data-feather="..."> icons app-wide (jQuery ready), so individual pages don't each need their own feather.replace() call. #}
     <script>$(function () { if (window.feather) feather.replace(); });</script>
     {% if socket_io_enabled %}
@@ -256,7 +259,7 @@
     </div><!-- /.app -->
     <script src="{{url_for('static_content', group='js', filename='toggle-theme.js')}}" defer></script>
     <script src="{{url_for('static_content', group='js', filename='hamburger-menu.js')}}" defer></script>
-    <script src="{{url_for('static_content', group='js', filename='sidebar.js')}}" defer></script>
+
     <script src="{{url_for('static_content', group='js', filename='menu-pop.js')}}" defer></script>
     <script src="{{url_for('static_content', group='js', filename='scrollable-title.js')}}" defer></script>
 

changedetectionio/tests/test_cache_control_headers.py

@@ -13,6 +13,8 @@ An app-wide after_request fills in Cache-Control when the route didn't set one,
 so the explicit headers on assets/screenshots/favicons still win.
 """
 
+import re
+
 
 def test_dynamic_pages_are_not_storable(client, live_server):
     # Any page carrying session state or a CSRF token - the cached copy is what
@@ -33,7 +35,7 @@ def test_routes_keep_their_own_cache_control(client, live_server):
     # The hook only fills in a missing header, so routes that deliberately
     # allow caching must come through untouched.
 
-    # send_from_directory() always sets Cache-Control itself, so static files
+    # static_content() sets its own header on assets (see the two tests below), so they
     # never reach the hook at all.
     response = client.get('/static/styles/styles.css')
     assert response.status_code == 200
@@ -49,3 +51,47 @@ def test_routes_keep_their_own_cache_control(client, live_server):
         f"flag SVGs set their own 24h public cache header, got "
         f"{response.headers.get('Cache-Control')!r}"
     )
+
+
+def test_static_assets_are_cacheable(client, live_server):
+    # Assets used to go out as "no-cache, max-age=0", so every CSS/JS/image on every page
+    # load cost a request (a 304, but still a round trip). url_for() now fingerprints the
+    # URL with the file's mtime+size, and a matching ?v= is what buys the long cache.
+    res = client.get('/')
+    assert res.status_code == 200
+
+    versioned = re.findall(r'(/static/(?:js|styles|images|favicons)/[^"?]+\?v=[0-9-]+)', res.data.decode())
+    assert versioned, "no fingerprinted asset URLs in the rendered page - ?v= is what unlocks caching"
+
+    for url in set(versioned):
+        response = client.get(url)
+        assert response.status_code == 200, url
+        assert response.headers.get('Cache-Control') == 'public, max-age=31536000, immutable', (
+            f"{url} is pinned to one file revision so it should be cacheable forever, got "
+            f"{response.headers.get('Cache-Control')!r}"
+        )
+        # The session cookie is permanent and re-signed on every response, so its value keeps
+        # changing - a "Vary: Cookie" here would miss the cache on every asset of every page load.
+        assert 'cookie' not in response.headers.get('Vary', '').lower(), (
+            f"{url} must not vary by cookie, got {response.headers.get('Vary')!r}"
+        )
+        assert 'Set-Cookie' not in response.headers, (
+            f"{url} is publicly cacheable, it must not carry a session cookie"
+        )
+        assert response.headers.get('ETag'), f"{url} sent no ETag"
+
+
+def test_unversioned_static_assets_still_revalidate(client, live_server):
+    # Older cached HTML (or a hand-typed URL) has no ?v=, and a stale one must not be trusted
+    # either - both have to keep revalidating so an upgrade can't serve the wrong file forever.
+    for url in ['/static/js/toggle-theme.js', '/static/js/toggle-theme.js?v=1-1']:
+        response = client.get(url)
+        assert response.status_code == 200, url
+        assert response.headers.get('Cache-Control') == 'public, max-age=0, must-revalidate', (
+            f"{url} isn't pinned to a known file revision so it must be revalidated, got "
+            f"{response.headers.get('Cache-Control')!r}"
+        )
+        etag = response.headers.get('ETag')
+        assert etag, f"{url} sent no ETag - revalidation would re-download the whole file"
+        # ETag is werkzeug's own mtime-size-path token, so revalidation is a cheap 304.
+        assert client.get(url, headers={'If-None-Match': etag}).status_code == 304