From b895f986253e476d8ec5ca3a9762781a81df34c2 Mon Sep 17 00:00:00 2001 From: Jonas Finnemann Jensen Date: Mon, 25 Mar 2024 11:34:53 +0100 Subject: [PATCH] An attempt at conservative default cache-control headers (#7587) * An attempt at conservative default cache-control headers * Reduce default cache-control max-age --- app/lib/frontend/handlers.dart | 11 +++++++++++ app/lib/frontend/handlers/account.dart | 26 ++++++++++++++++++-------- app/lib/frontend/handlers/headers.dart | 26 +++++++++++++++++++++++--- app/lib/frontend/handlers/misc.dart | 13 +++++++++++-- app/lib/frontend/handlers/package.dart | 1 + app/lib/frontend/handlers/report.dart | 12 +++++++++--- app/lib/frontend/handlers/routes.dart | 6 +++++- app/lib/shared/handler_helpers.dart | 2 +- app/lib/task/handlers.dart | 7 ++++++- 9 files changed, 85 insertions(+), 19 deletions(-) diff --git a/app/lib/frontend/handlers.dart b/app/lib/frontend/handlers.dart index 5c839d2307..fc800cafa4 100644 --- a/app/lib/frontend/handlers.dart +++ b/app/lib/frontend/handlers.dart @@ -11,6 +11,7 @@ import 'package:shelf_router/shelf_router.dart'; import '../shared/handlers.dart'; import '../shared/urls.dart'; +import 'handlers/headers.dart'; import 'handlers/misc.dart'; import 'handlers/pubapi.dart'; import 'handlers/redirects.dart'; @@ -68,11 +69,21 @@ shelf.Handler createAppHandler() { final rs = await pubApiHandler(request); if (rs != Router.routeNotFound) { + if (rs.statusCode == 200 && + request.method == 'GET' && + !CacheHeaders.hasCacheHeader(rs.headers)) { + return rs.change(headers: CacheHeaders.defaultApi()); + } return rs; } final res = await pubSiteHandler(request); if (res != Router.routeNotFound) { + if (rs.statusCode == 200 && + request.method == 'GET' && + !CacheHeaders.hasCacheHeader(rs.headers)) { + return rs.change(headers: CacheHeaders.defaultUI()); + } return res; } diff --git a/app/lib/frontend/handlers/account.dart b/app/lib/frontend/handlers/account.dart index 20917f5ae6..55a1e3acf9 100644 --- a/app/lib/frontend/handlers/account.dart +++ b/app/lib/frontend/handlers/account.dart @@ -26,6 +26,7 @@ import '../../shared/urls.dart' as urls; import '../templates/admin.dart'; import '../templates/consent.dart'; import '../templates/misc.dart' show renderUnauthenticatedPage; +import 'headers.dart'; /// Handles requests for /authorized shelf.Response authorizedHandler(_) => htmlResponse(renderAuthorizedPage()); @@ -55,10 +56,13 @@ Future startSignInHandler(shelf.Request request) async { ); return redirectResponse( oauth2Url.toString(), - headers: session_cookie.createClientSessionCookie( - sessionId: session.sessionId, - maxAge: session.maxAge, - ), + headers: { + ...session_cookie.createClientSessionCookie( + sessionId: session.sessionId, + maxAge: session.maxAge, + ), + ...CacheHeaders.privateZero(), + }, ); } @@ -109,10 +113,13 @@ Future signInCompleteHandler(shelf.Request request) async { ); return redirectResponse( go, - headers: session_cookie.createClientSessionCookie( - sessionId: newSession.sessionId, - maxAge: newSession.maxAge, - ), + headers: { + ...session_cookie.createClientSessionCookie( + sessionId: newSession.sessionId, + maxAge: newSession.maxAge, + ), + ...CacheHeaders.privateZero(), + }, ); } @@ -174,6 +181,9 @@ Future consentPageHandler( // Consent pages have the consent ID in the URL. Browsers should not pass on // this ID to the pages that are linked from the consent page. noReferrer: true, + headers: { + ...CacheHeaders.privateZero(), + }, ); } diff --git a/app/lib/frontend/handlers/headers.dart b/app/lib/frontend/handlers/headers.dart index 722927e517..e4cefee2be 100644 --- a/app/lib/frontend/handlers/headers.dart +++ b/app/lib/frontend/handlers/headers.dart @@ -43,7 +43,9 @@ class CacheHeaders { 'private' else if (public) 'public' - else if (requestContext.isNotAuthenticated) + else if (requestContext.isNotAuthenticated && + requestContext.uiCacheEnabled && + requestContext.sessionData == null) 'public' else 'private', @@ -57,8 +59,26 @@ class CacheHeaders { return headers.containsKey(HttpHeaders.cacheControlHeader); } - /// Default private-only caching. - static final defaultPrivate = CacheHeaders._(Duration.zero, private: true); + /// Private-only max-age zero. + static final privateZero = CacheHeaders._(Duration.zero, private: true); + + /// Default cache-control for public user-interface + static final defaultPublicUI = CacheHeaders._( + Duration(minutes: 10), + public: true, + ); + + /// Default cache-control for user-interface + static final defaultUI = CacheHeaders._( + Duration(minutes: 10), + // public / private is inferred from authentication + session header + ); + + /// Default cache-control for API responses + static final defaultApi = CacheHeaders._( + Duration(minutes: 10), + // public / private is inferred from authentication + session header + ); /// Everything under the /documentation/ endpoint. static final dartdocAsset = CacheHeaders._(Duration(minutes: 15)); diff --git a/app/lib/frontend/handlers/misc.dart b/app/lib/frontend/handlers/misc.dart index 659dc0f290..18d4b0e5e2 100644 --- a/app/lib/frontend/handlers/misc.dart +++ b/app/lib/frontend/handlers/misc.dart @@ -68,9 +68,16 @@ Future securityPageHandler(shelf.Request request) async { /// Handles requests for /readiness_check Future readinessCheckHandler(shelf.Request request) async { if (nameTracker.isReady) { - return htmlResponse('OK'); + return htmlResponse( + 'OK', + headers: CacheHeaders.privateZero(), + ); } else { - return htmlResponse('Service Unavailable', status: 503); + return htmlResponse( + 'Service Unavailable', + status: 503, + headers: CacheHeaders.privateZero(), + ); } } @@ -182,6 +189,7 @@ Future staticsHandler(shelf.Request request) async { acceptsGzipEncoding ? staticFile.gzippedBytes : staticFile.bytes; final headers = { if (acceptsGzipEncoding) HttpHeaders.contentEncodingHeader: 'gzip', + 'Vary': 'Accept-Encoding', HttpHeaders.contentTypeHeader: staticFile.contentType, HttpHeaders.contentLengthHeader: bytes.length.toString(), HttpHeaders.lastModifiedHeader: formatHttpDate(staticFile.lastModified), @@ -232,6 +240,7 @@ Future experimentalHandler(shelf.Request request) async { value: flags.encodedAsCookie(), maxAge: experimentalCookieDuration, ), + ...CacheHeaders.privateZero(), }); } diff --git a/app/lib/frontend/handlers/package.dart b/app/lib/frontend/handlers/package.dart index 61b27943a2..52e4d4b18f 100644 --- a/app/lib/frontend/handlers/package.dart +++ b/app/lib/frontend/handlers/package.dart @@ -231,6 +231,7 @@ Future packageScoreLogTxtHandler( body: log ?? 'no log', headers: { 'content-type': 'text/plain;charset=UTF-8', + ...CacheHeaders.defaultPublicUI(), }, ); } diff --git a/app/lib/frontend/handlers/report.dart b/app/lib/frontend/handlers/report.dart index 25c1769539..cc09220595 100644 --- a/app/lib/frontend/handlers/report.dart +++ b/app/lib/frontend/handlers/report.dart @@ -17,6 +17,7 @@ import '../../shared/handlers.dart'; import '../../shared/utils.dart'; import '../request_context.dart'; import '../templates/report.dart'; +import 'headers.dart'; /// Handles GET /report Future reportPageHandler(shelf.Request request) async { @@ -29,9 +30,14 @@ Future reportPageHandler(shelf.Request request) async { return unauthenticatedRs; } - return htmlResponse(renderReportPage( - sessionData: requestContext.sessionData, - )); + return htmlResponse( + renderReportPage( + sessionData: requestContext.sessionData, + ), + headers: { + ...CacheHeaders.privateZero(), + }, + ); } /// Handles POST /api/report diff --git a/app/lib/frontend/handlers/routes.dart b/app/lib/frontend/handlers/routes.dart index 51b75866a7..3f773e0f11 100644 --- a/app/lib/frontend/handlers/routes.dart +++ b/app/lib/frontend/handlers/routes.dart @@ -17,6 +17,7 @@ import '../../shared/handlers.dart'; import 'account.dart'; import 'atom_feed.dart'; import 'documentation.dart'; +import 'headers.dart'; import 'landing.dart'; import 'listing.dart'; import 'misc.dart'; @@ -35,7 +36,10 @@ class PubSiteService { // **** @Route.get('/liveness_check') - Future livenessCheck(Request request) async => htmlResponse('OK'); + Future livenessCheck(Request request) async => htmlResponse( + 'OK', + headers: CacheHeaders.privateZero(), + ); @Route.get('/readiness_check') Future readinessCheck(Request request) async => diff --git a/app/lib/shared/handler_helpers.dart b/app/lib/shared/handler_helpers.dart index e436227b00..63658ea6fb 100644 --- a/app/lib/shared/handler_helpers.dart +++ b/app/lib/shared/handler_helpers.dart @@ -136,7 +136,7 @@ shelf.Handler _requestContextWrapper(shelf.Handler handler) { if (!context.uiCacheEnabled && !CacheHeaders.hasCacheHeader(rs.headers)) { // Indicates that the response is intended for a single user and must not // be stored by a shared cache. A private cache may store the response. - rs = rs.change(headers: CacheHeaders.defaultPrivate()); + rs = rs.change(headers: CacheHeaders.privateZero()); } return rs; }; diff --git a/app/lib/task/handlers.dart b/app/lib/task/handlers.dart index eb47395be7..10931c40c8 100644 --- a/app/lib/task/handlers.dart +++ b/app/lib/task/handlers.dart @@ -176,7 +176,10 @@ Future handleTaskResource( } if (request.method.toUpperCase() == 'HEAD') { - return htmlResponse(''); + return htmlResponse( + '', + headers: CacheHeaders.defaultPublicUI(), + ); } final acceptsGzip = request.acceptsGzipEncoding(); @@ -185,6 +188,8 @@ Future handleTaskResource( headers: { 'Content-Type': mime, if (acceptsGzip) 'Content-Encoding': 'gzip', + 'Vary': 'Accept-Encoding', + ...CacheHeaders.defaultPublicUI(), }, ); }