Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: set cache headers for static assets #999

Merged
merged 2 commits into from
Apr 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from functools import partial
from numbers import Number
from time import monotonic
from urllib.parse import urljoin

import timeago
from flask import (
Expand Down Expand Up @@ -134,7 +135,7 @@ def create_app(application):

application.config.from_object(configs[notify_environment])
asset_fingerprinter._cdn_domain = application.config['ASSET_DOMAIN']
asset_fingerprinter._asset_root = application.config['ASSET_PATH']
asset_fingerprinter._asset_root = urljoin(application.config['ADMIN_BASE_URL'], application.config['ASSET_PATH'])

application.config["BABEL_DEFAULT_LOCALE"] = "en"
babel = Babel(application)
Expand Down Expand Up @@ -257,7 +258,6 @@ def record_start_time():
@application.context_processor
def inject_global_template_variables():
return {
'asset_path': application.config['ASSET_PATH'],
'header_colour': application.config['HEADER_COLOUR'],
'asset_url': asset_fingerprinter.get_url,
'asset_s3_url': asset_fingerprinter.get_s3_url,
Expand Down Expand Up @@ -648,8 +648,13 @@ def useful_headers_after_request(response):
))
if 'Cache-Control' in response.headers:
del response.headers['Cache-Control']
response.headers.add(
'Cache-Control', 'no-store, no-cache, private, must-revalidate')
# Cache static assets (CSS, JS, images) for a long time
# as they have unique hashes thanks to the asset
# fingerprinter
if asset_fingerprinter.is_static_asset(request.url):
response.headers.add('Cache-Control', 'public, max-age=31536000, immutable')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just so you are aware: immutable is only support on Firefox, Safari

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else:
response.headers.add('Cache-Control', 'no-store, no-cache, private, must-revalidate')
for key, value in response.headers:
response.headers[key] = SanitiseASCII.encode(value)
return response
Expand Down
3 changes: 3 additions & 0 deletions app/asset_fingerprinter.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,8 @@ def get_asset_file_contents(self, asset_file_path):
contents = asset_file.read()
return contents

def is_static_asset(self, url):
return url and url.startswith(self._asset_root)


asset_fingerprinter = AssetFingerprinter()
1 change: 0 additions & 1 deletion app/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@ class Test(Development):
ANTIVIRUS_API_HOST = 'https://test-antivirus'
ANTIVIRUS_API_KEY = 'test-antivirus-secret'
ASSET_DOMAIN = 'static.example.com'
ASSET_PATH = 'https://static.example.com/'


class Production(Config):
Expand Down
6 changes: 3 additions & 3 deletions app/templates/main_template.html
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@
<link
rel="stylesheet"
media="screen"
href="{{ asset_path }}stylesheets/index.css"
href="{{ asset_url('stylesheets/index.css') }}"
/>


<link
rel="shortcut icon"
href="{{ asset_path }}images/favicon.ico?0.24.2"
href="{{ asset_url('images/favicon.ico') }}"
type="image/x-icon"
/>

Expand All @@ -34,7 +34,7 @@
<meta name="viewport" content="width=device-width, initial-scale=1"/>
<meta
property="og:image"
content="{{ admin_base_url }}/static/images/product/notify-main.jpg"
content="{{ asset_url('images/product/notify-main.jpg') }}"
/>
<script>
window.APP_LANG = "{{ current_lang }}"
Expand Down
2 changes: 1 addition & 1 deletion app/templates/notify_template.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<title>{% block page_title %}Notify{% endblock %}</title>
<link
rel="shortcut icon"
href="{{ asset_path }}images/favicon.ico?0.24.2"
href="{{ asset_url('images/favicon.ico') }}"
type="image/x-icon"
/>
<link
Expand Down
4 changes: 2 additions & 2 deletions app/templates/views/signedout.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@
<meta property="og:description" content="{{ description }}"/>
<meta
property="og:image"
content="{{ admin_base_url }}/static/images/product/notify-main.jpg"
content="{{ asset_url('images/product/notify-main.jpg') }}"
/>
<meta property="og:url" content="{{ admin_base_url }}"/>

<link
rel="stylesheet"
media="screen"
href="{{ asset_path }}stylesheets/index.css"
href="{{ asset_url('stylesheets/index.css') }}"
/>

{% endblock %}
Expand Down
10 changes: 10 additions & 0 deletions tests/app/main/test_asset_fingerprinter.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,16 @@ def test_s3_url(self):

assert fingerprinter.get_s3_url('foo.png') == 'https://assets.example.com/static/foo.png'

def test_is_static_asset(self):
fingerprinter = AssetFingerprinter(
asset_root='https://example.com/static/',
cdn_domain='assets.example.com',
)

assert fingerprinter.is_static_asset('https://example.com/static/image.png')
assert not fingerprinter.is_static_asset('https://assets.example.com/image.png')
assert not fingerprinter.is_static_asset('https://example.com/robots.txt')


class TestAssetFingerprintWithUnicode(object):
def test_can_read_self(self):
Expand Down
17 changes: 17 additions & 0 deletions tests/app/main/views/test_headers.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import pytest

from app.asset_fingerprinter import asset_fingerprinter

service = [{'service_id': 1, 'service_name': 'jessie the oak tree',
'organisation_name': 'Forest', 'consent_to_research': True,
'contact_name': 'Forest fairy', 'organisation_type': 'Ecosystem',
Expand Down Expand Up @@ -40,6 +44,19 @@ def test_owasp_useful_headers_set(
)


@pytest.mark.parametrize('url, cache_headers', [
(asset_fingerprinter.get_url('stylesheets/index.css'), 'public, max-age=31536000, immutable'),
(asset_fingerprinter.get_url('images/favicon.ico'), 'public, max-age=31536000, immutable'),
(asset_fingerprinter.get_url('javascripts/main.min.js'), 'public, max-age=31536000, immutable'),
('/robots.txt', 'no-store, no-cache, private, must-revalidate'),
], ids=["CSS file", "image", "JS file", "static page"])
def test_headers_cache_static_assets(client, url, cache_headers):
response = client.get(url)

assert response.status_code == 200
assert response.headers['Cache-Control'] == cache_headers


def test_headers_non_ascii_characters_are_replaced(
client,
mocker,
Expand Down
5 changes: 2 additions & 3 deletions tests/app/main/views/test_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,12 +189,11 @@ def test_css_is_served_from_correct_path(client_request):
for index, link in enumerate(
page.select('link[rel=stylesheet]')
):

assert link['href'].startswith([
'https://static.example.com/stylesheets/index.css',
'http://localhost:6012/static/stylesheets/index.css',
'https://fonts.googleapis.com/css?family=Lato:400,700,900&display=swap',
'https://fonts.googleapis.com/css?',
'https://static.example.com/stylesheets/main.css?',
'http://localhost:6012/static/stylesheets/main.css?',
][index])


Expand Down