From 7dbbae4d7c5bba46c5239f811c1ab2a1346882f1 Mon Sep 17 00:00:00 2001 From: Graham Knop Date: Mon, 27 May 2024 00:40:31 +0200 Subject: [PATCH] rewrite static middleware to correctly set headers Surrogate-Control headers were being set for 404 responses for static files. This would cause Fastly to cache them indefinitely. During a deploy, there would be a period of time where new servers would link to js and css assets, which old servers would respond with 404s and get cached. Fix the static file serving code to only set the Surrogate-Control headers for 2xx responses to prevent this. This ended up being a significant rewrite of the middleware. --- lib/MetaCPAN/Middleware/Static.pm | 161 +++++++++++++++++++----------- lib/MetaCPAN/Web.pm | 5 +- t/static-mounts.t | 106 ++++++++++++++++++++ 3 files changed, 209 insertions(+), 63 deletions(-) create mode 100644 t/static-mounts.t diff --git a/lib/MetaCPAN/Middleware/Static.pm b/lib/MetaCPAN/Middleware/Static.pm index 2246687905..08947d6174 100644 --- a/lib/MetaCPAN/Middleware/Static.pm +++ b/lib/MetaCPAN/Middleware/Static.pm @@ -1,17 +1,18 @@ package MetaCPAN::Middleware::Static; use strict; use warnings; -use Plack::Builder qw( builder enable mount ); -use Plack::App::File (); +use Cpanel::JSON::XS (); use Cwd qw( cwd ); +use Plack::App::File (); +use Plack::Builder qw( builder enable mount ); use Plack::MIME (); -use Cpanel::JSON::XS (); +use Plack::Util (); Plack::MIME->add_type( '.eot' => 'application/vnd.ms-fontobject', + '.map' => 'application/json', + '.mjs' => 'application/javascript', '.otf' => 'font/otf', - '.ttf' => 'font/ttf', - '.woff' => 'application/font-woff', '.woff2' => 'application/font-woff2', ); @@ -21,26 +22,77 @@ my $hour_ttl = 60 * 60; my $day_ttl = $hour_ttl * 24; my $year_ttl = $day_ttl * 365; +sub _response_mw { + my ( $app, $cb ) = @_; + sub { Plack::Util::response_cb( $app->(@_), $cb ) }; +} + +sub _add_headers { + my ( $app, $add_headers ) = @_; + _response_mw( + $app, + sub { + my $res = shift; + my ( $status, $headers ) = @$res; + if ( $status >= 200 && $status < 300 ) { + push @$headers, @$add_headers; + } + return $res; + } + ); +} + +sub _add_surrogate_keys { + my ($app) = @_; + _response_mw( + $app, + sub { + my $res = shift; + my $headers = $res->[1]; + if ( my $content_type + = Plack::Util::header_get( $headers, 'Content-Type' ) ) + { + $content_type =~ s/;.*//; + my $media_type = $content_type =~ s{/.*}{}r; + push @$headers, + 'Surrogate-Key' => join( ', ', + map "content_type=$_", + $content_type, $media_type ); + } + return $res; + } + ); +} + +sub _file_app { + my ( $type, $path, $headers ) = @_; + _add_surrogate_keys( _add_headers( + Plack::App::File->new( $type => $path )->to_app, $headers, + ) ); +} + +sub _get_assets { + my ($root) = @_; + open my $fh, '<', "$root/assets/assets.json" + or die "can't find asset map"; + my $json = do { local $/; <$fh> }; + close $fh; + my $files = Cpanel::JSON::XS->new->decode($json); + return [ map "/assets/$_", @$files ]; +} + sub wrap { my ( $self, $app, %args ) = @_; my $root_dir = $args{root} || cwd; + my $root = "$root_dir/root"; my $dev_mode = exists $args{dev_mode} ? $args{dev_mode} : ( $ENV{PLACK_ENV} && $ENV{PLACK_ENV} eq 'development' ); - my $get_assets = sub { - open my $fh, '<', "$root_dir/root/assets/assets.json" - or die "can't find asset map"; - my $json = do { local $/; <$fh> }; - close $fh; - my $files = Cpanel::JSON::XS->new->decode($json); - return [ map "/assets/$_", @$files ]; - }; - my $assets; if ( !$dev_mode ) { - $assets = $get_assets->(); + $assets = _get_assets($root); } builder { @@ -49,60 +101,49 @@ sub wrap { sub { my ($env) = @_; if ($dev_mode) { - $assets = $get_assets->(); + $assets = _get_assets($root); } push @{ $env->{'metacpan.assets'} ||= [] }, @$assets; $app->($env); }; }; - my $favicon_app - = Plack::App::File->new( file => 'root/static/icons/favicon.ico' ) - ->to_app; - mount '/favicon.ico' => sub { - my $res = $favicon_app->(@_); - push @{ $res->[1] }, - ( - 'Cache-Control' => "max-age=${day_ttl}", + mount '/favicon.ico' => _file_app( + file => "$root/static/icons/favicon.ico", + [ + 'Cache-Control' => "public, max-age=${day_ttl}", 'Surrogate-Control' => "max-age=${year_ttl}", 'Surrogate-Key' => 'assets', - ); - $res; - }; - my $static_app - = Plack::App::File->new( root => 'root/static' )->to_app; - mount '/static' => sub { - my $env = shift; - my $res = $static_app->($env); - if ( $env->{PATH_INFO} =~ m{^/(?:images|icons|fonts|modules)/} ) { - push @{ $res->[1] }, - ( 'Cache-Control' => - "public, max-age=${year_ttl}, immutable", ); - } - else { - push @{ $res->[1] }, - ( 'Cache-Control' => "public, max-age=${day_ttl}", ); - } - push @{ $res->[1] }, - ( - 'Surrogate-Key' => 'assets', - 'Surrogate-Control' => "max-age=${year_ttl}", - ); - $res; - }; - my $assets_app - = Plack::App::File->new( root => 'root/assets' )->to_app; - mount '/assets' => sub { - my $env = shift; - my $res = $assets_app->($env); - push @{ $res->[1] }, - ( - 'Cache-Control' => "public, max-age=${year_ttl}, immutable", - 'Surrogate-Key' => 'assets', + ], + ); + + for my $static_dir ( qw( + assets + static/icons + static/images + ) ) + { + mount "/$static_dir" => _file_app( + root => "$root/$static_dir", + [ + 'Cache-Control' => + "public, max-age=${year_ttl}, immutable", + 'Surrogate-Control' => "max-age=${year_ttl}", + 'Surrogate-Key' => 'assets', + ], + ); + } + + mount "/static" => _file_app( + root => "$root/static", + [ + $dev_mode + ? ( 'Cache-Control' => "public, max-age=${day_ttl}", ) + : ( 'Cache-Control' => "public", ), 'Surrogate-Control' => "max-age=${year_ttl}", - ); - return $res; - }; + 'Surrogate-Key' => 'assets', + ], + ); mount '/' => $app; }; diff --git a/lib/MetaCPAN/Web.pm b/lib/MetaCPAN/Web.pm index 3c342d8b44..1e58e93ae7 100644 --- a/lib/MetaCPAN/Web.pm +++ b/lib/MetaCPAN/Web.pm @@ -6,13 +6,12 @@ use Catalyst::Runtime 5.90042; use Catalyst qw/ ConfigLoader - Static::Simple Authentication - +MetaCPAN::Role::Fastly::Catalyst /, '-Log=warn,error,fatal'; use Log::Log4perl::Catalyst (); -extends 'Catalyst'; +with 'MetaCPAN::Role::Fastly'; +with 'MetaCPAN::Role::Fastly::Catalyst'; __PACKAGE__->request_class_traits( [ qw( MetaCPAN::Web::Role::Request diff --git a/t/static-mounts.t b/t/static-mounts.t new file mode 100644 index 0000000000..46db073d49 --- /dev/null +++ b/t/static-mounts.t @@ -0,0 +1,106 @@ +use strict; +use warnings; +use lib 't/lib'; + +use Test::More; +use MetaCPAN::Web::Test qw( app GET test_psgi ); + +test_psgi app, sub { + my $cb = shift; + { + ok( my $res = $cb->( GET '/favicon.ico' ), 'GET /favicon.ico' ); + is( $res->code, 200, 'code 200' ); + unlike $res->header('Cache-Control'), qr/immutable/, "not immutable"; + is_deeply [ sort split /, /, $res->header('Surrogate-Key') ], [ qw( + assets + content_type=image + content_type=image/vnd.microsoft.icon + ) ], + 'correct Surrogate-Key'; + } + { + ok( my $res = $cb->( GET '/static/opensearch.xml' ), + 'GET /static/opensearch.xml' ); + is( $res->code, 200, 'code 200' ); + unlike $res->header('Cache-Control'), qr/immutable/, "not immutable"; + is_deeply [ sort split /, /, $res->header('Surrogate-Key') ], [ qw( + assets + content_type=application + content_type=application/xml + ) ], + 'correct Surrogate-Key'; + } + { + ok( my $res = $cb->( GET '/static/fastly_do_not_delete.gif' ), + 'GET /static/fastly_do_not_delete.gif' ); + is( $res->code, 200, 'code 200' ); + unlike $res->header('Cache-Control'), qr/immutable/, "not immutable"; + is_deeply [ sort split /, /, $res->header('Surrogate-Key') ], [ qw( + assets + content_type=image + content_type=image/gif + ) ], + 'correct Surrogate-Key'; + } + { + ok( my $res = $cb->( GET '/static/icons/grid.svg' ), + 'GET /static/icons/grid.svg' ); + is( $res->code, 200, 'code 200' ); + like $res->header('Cache-Control'), qr/immutable/, "immutable"; + is_deeply [ sort split /, /, $res->header('Surrogate-Key') ], [ qw( + assets + content_type=image + content_type=image/svg+xml + ) ], + 'correct Surrogate-Key'; + } + { + ok( my $res = $cb->( GET '/static/images/dots.svg' ), + 'GET /static/images/dots.svg' ); + is( $res->code, 200, 'code 200' ); + like $res->header('Cache-Control'), qr/immutable/, "immutable"; + is_deeply [ sort split /, /, $res->header('Surrogate-Key') ], [ qw( + assets + content_type=image + content_type=image/svg+xml + ) ], + 'correct Surrogate-Key'; + } + { + ok( my $res = $cb->( GET '/static/js/main.mjs' ), + 'GET /static/js/main.mjs' ); + is( $res->code, 200, 'code 200' ); + unlike $res->header('Cache-Control'), qr/immutable/, "not immutable"; + is_deeply [ sort split /,? /, $res->header('Surrogate-Key') ], [ qw( + assets + content_type=application + content_type=application/javascript + ) ], + 'correct Surrogate-Key'; + } + { + ok( my $res = $cb->( GET '/assets/assets.json' ), + 'GET /assets/assets.json' ); + is( $res->code, 200, 'code 200' ); + like $res->header('Cache-Control'), qr/immutable/, "immutable"; + is_deeply [ sort split /, /, $res->header('Surrogate-Key') ], [ qw( + assets + content_type=application + content_type=application/json + ) ], + 'correct Surrogate-Key'; + } + { + ok( my $res = $cb->( GET '/assets/this-file-does-not-exist.js' ), + 'GET /assets/this-file-does-not-exist.js' ); + is( $res->code, 404, 'code 404' ); + unlike $res->header('Cache-Control'), qr/immutable/, "not immutable"; + is_deeply [ sort split /, /, $res->header('Surrogate-Key') ], [ qw( + content_type=text + content_type=text/plain + ) ], + 'correct Surrogate-Key'; + } +}; + +done_testing;