diff --git a/CHANGELOG.md b/CHANGELOG.md index 74314ceec3..21155a1603 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ AppEngine version, listed here to ease deployment and troubleshooting. * Bump runtimeVersion to `2025.10.22`. * Upgraded stable Flutter analysis SDK to `3.35.6`. * Upgraded pana to `0.23.0`. + * Added more explicitly public `cache-control` to content pages. ## `20251017t101000-all` * Bump runtimeVersion to `2025.10.17`. diff --git a/app/lib/frontend/handlers/cache_control.dart b/app/lib/frontend/handlers/cache_control.dart index 0efd1cf610..966dd6a14f 100644 --- a/app/lib/frontend/handlers/cache_control.dart +++ b/app/lib/frontend/handlers/cache_control.dart @@ -62,6 +62,20 @@ final class CacheControl { public: true, ); + /// `Cache-Control` headers for package content pages, returning content that + /// is not updated frequently. + static const packageContentPage = CacheControl( + maxAge: Duration(minutes: 30), + public: true, + ); + + /// `Cache-Control` headers for package listing pages, returning content that + /// is may be updated frequently. + static const packageListingPage = CacheControl( + maxAge: Duration(minutes: 5), + public: true, + ); + /// `Cache-Control` headers for API end-points returning completion data for /// use in IDE integrations. static const completionData = CacheControl( diff --git a/app/lib/frontend/handlers/documentation.dart b/app/lib/frontend/handlers/documentation.dart index 828c762062..214cbfc071 100644 --- a/app/lib/frontend/handlers/documentation.dart +++ b/app/lib/frontend/handlers/documentation.dart @@ -23,6 +23,11 @@ import '../../shared/urls.dart'; /// /// - `/documentation//` Future documentationHandler(shelf.Request request) async { + final requestMethod = request.method.toUpperCase(); + if (requestMethod != 'HEAD' && requestMethod != 'GET') { + return methodNotAllowedHandler(request); + } + final docFilePath = parseRequestUri(request.requestedUri); if (docFilePath == null) { return notFoundHandler(request); @@ -58,12 +63,6 @@ Future documentationHandler(shelf.Request request) async { ), ); } - final String requestMethod = request.method.toUpperCase(); - - if (requestMethod != 'HEAD' && requestMethod != 'GET') { - // TODO: Should probably be "method not supported"! - return notFoundHandler(request); - } final package = docFilePath.package; final version = docFilePath.version!; diff --git a/app/lib/frontend/handlers/landing.dart b/app/lib/frontend/handlers/landing.dart index 4b0da1e9d5..c2265e98e3 100644 --- a/app/lib/frontend/handlers/landing.dart +++ b/app/lib/frontend/handlers/landing.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'package:_pub_shared/search/tags.dart'; +import 'package:pub_dev/frontend/handlers/cache_control.dart'; import 'package:pub_dev/search/top_packages.dart'; import 'package:shelf/shelf.dart' as shelf; @@ -49,7 +50,10 @@ Future indexLandingHandler(shelf.Request request) async { } if (requestContext.uiCacheEnabled) { - return htmlResponse((await cache.uiIndexPage().get(_render))!); + return htmlResponse( + (await cache.uiIndexPage().get(_render))!, + headers: CacheControl.packageListingPage.headers, + ); } return htmlResponse(await _render()); } diff --git a/app/lib/frontend/handlers/listing.dart b/app/lib/frontend/handlers/listing.dart index 51a45c74e0..f387d864c0 100644 --- a/app/lib/frontend/handlers/listing.dart +++ b/app/lib/frontend/handlers/listing.dart @@ -7,6 +7,8 @@ import 'dart:async'; import 'package:_pub_shared/search/search_form.dart'; import 'package:_pub_shared/search/tags.dart'; import 'package:logging/logging.dart'; +import 'package:pub_dev/frontend/handlers/cache_control.dart'; +import 'package:pub_dev/frontend/request_context.dart'; import 'package:shelf/shelf.dart' as shelf; import '../../package/name_tracker.dart'; @@ -94,6 +96,9 @@ Future _packagesHandlerHtmlCore(shelf.Request request) async { openSections: openSections, ), status: statusCode, + headers: statusCode == 200 && requestContext.uiCacheEnabled + ? CacheControl.packageListingPage.headers + : null, ); _searchOverallLatencyTracker.add(sw.elapsed); return result; diff --git a/app/lib/frontend/handlers/package.dart b/app/lib/frontend/handlers/package.dart index ef9b2d0a8b..dc13e8615d 100644 --- a/app/lib/frontend/handlers/package.dart +++ b/app/lib/frontend/handlers/package.dart @@ -301,6 +301,7 @@ Future _handlePackagePage({ Entry? cacheEntry, }) async { checkPackageVersionParams(packageName, versionName); + final cacheEnabled = requestContext.uiCacheEnabled && cacheEntry != null; final canonicalUrl = canonicalUrlFn( await _canonicalPackageName(packageName), @@ -314,7 +315,7 @@ Future _handlePackagePage({ } final Stopwatch sw = Stopwatch()..start(); String? cachedPage; - if (requestContext.uiCacheEnabled && cacheEntry != null) { + if (cacheEnabled) { cachedPage = await cacheEntry.get(); } @@ -350,12 +351,15 @@ Future _handlePackagePage({ } else { throw StateError('Unknown result type: ${renderedResult.runtimeType}'); } - if (requestContext.uiCacheEnabled && cacheEntry != null) { + if (cacheEnabled) { await cacheEntry.set(cachedPage); } _packageDoneLatencyTracker.add(sw.elapsed); } - return htmlResponse(cachedPage); + return htmlResponse( + cachedPage, + headers: cacheEnabled ? CacheControl.packageContentPage.headers : null, + ); } /// Returns the optionally lowercased version of [name], but only if there diff --git a/app/lib/frontend/request_context.dart b/app/lib/frontend/request_context.dart index 4fdc90bd9b..077e7b9bd8 100644 --- a/app/lib/frontend/request_context.dart +++ b/app/lib/frontend/request_context.dart @@ -110,7 +110,7 @@ Future buildRequestContext({ // don't cache if client session is active !clientSessionCookieStatus.isPresent && // sanity check, this should be covered by client session cookie - (csrfToken?.isNotEmpty ?? false); + !(csrfToken?.isNotEmpty ?? false); return RequestContext( indentJson: indentJson, blockRobots: !enableRobots, diff --git a/app/lib/shared/handlers.dart b/app/lib/shared/handlers.dart index d610d61ccf..65bec23433 100644 --- a/app/lib/shared/handlers.dart +++ b/app/lib/shared/handlers.dart @@ -101,6 +101,13 @@ shelf.Response notFoundHandler( return htmlResponse(body, status: 404, headers: headers); } +shelf.Response methodNotAllowedHandler( + shelf.Request request, { + Map? headers, +}) { + return shelf.Response(405, body: 'Method Not Allowed', headers: headers); +} + shelf.Response rejectRobotsHandler(shelf.Request request) => shelf.Response.ok('User-agent: *\nDisallow: /\n'); diff --git a/app/lib/task/handlers.dart b/app/lib/task/handlers.dart index 5dbc70fa47..4252744bf3 100644 --- a/app/lib/task/handlers.dart +++ b/app/lib/task/handlers.dart @@ -6,6 +6,7 @@ import 'package:path/path.dart' as p; import 'package:pub_dev/dartdoc/dartdoc_page.dart'; import 'package:pub_dev/dartdoc/models.dart'; +import 'package:pub_dev/frontend/handlers/cache_control.dart'; import 'package:pub_dev/shared/exceptions.dart'; import 'package:pub_dev/shared/handlers.dart'; import 'package:pub_dev/shared/redis_cache.dart'; @@ -90,7 +91,10 @@ Future handleDartDoc( ); final htmlBytes = await htmlBytesCacheEntry.get(); if (htmlBytes != null) { - return htmlBytesResponse(htmlBytes); + return htmlBytesResponse( + htmlBytes, + headers: CacheControl.packageContentPage.headers, + ); } // check cached status for redirect or missing pages @@ -211,7 +215,10 @@ Future handleDartDoc( switch (status.code) { case DocPageStatusCode.ok: await htmlBytesCacheEntry.set(bytes!); - return htmlBytesResponse(bytes); + return htmlBytesResponse( + bytes, + headers: CacheControl.packageContentPage.headers, + ); case DocPageStatusCode.redirect: return redirectPathResponse(status.redirectPath!); case DocPageStatusCode.missing: @@ -236,13 +243,14 @@ Future handleDartDoc( } if (request.method.toUpperCase() == 'HEAD') { - return htmlResponse(''); + return htmlResponse('', headers: CacheControl.packageContentPage.headers); } final acceptsGzip = request.acceptsGzipEncoding(); return shelf.Response.ok( acceptsGzip ? dataGz : gzip.decode(dataGz), headers: { + ...CacheControl.packageContentPage.headers, 'Content-Type': mime, 'Vary': 'Accept-Encoding', // body depends on accept-encoding! if (acceptsGzip) 'Content-Encoding': 'gzip', diff --git a/pkg/pub_integration/lib/src/fake_test_context_provider.dart b/pkg/pub_integration/lib/src/fake_test_context_provider.dart index b3a8f49c63..806738f35c 100644 --- a/pkg/pub_integration/lib/src/fake_test_context_provider.dart +++ b/pkg/pub_integration/lib/src/fake_test_context_provider.dart @@ -58,14 +58,20 @@ class TestContextProvider { await _fakePubServerProcess.kill(); } - Future createAnonymousTestUser() async { + Future createAnonymousTestUser({ + bool expectAllResponsesToBeCacheControlPublic = true, + }) async { final session = await _testBrowser.createSession(); return TestUser( email: '', browserApi: PubApiClient(pubHostedUrl), serverApi: PubApiClient(pubHostedUrl), withBrowserPage: (Future Function(Page) fn) async { - return await session.withPage(fn: fn); + return await session.withPage( + fn: fn, + expectAllResponsesToBeCacheControlPublic: + expectAllResponsesToBeCacheControlPublic, + ); }, readLatestEmail: () async => throw UnimplementedError(), createCredentials: () async => throw UnimplementedError(), diff --git a/pkg/pub_integration/lib/src/test_browser.dart b/pkg/pub_integration/lib/src/test_browser.dart index f19829d230..ef73c5c37a 100644 --- a/pkg/pub_integration/lib/src/test_browser.dart +++ b/pkg/pub_integration/lib/src/test_browser.dart @@ -185,7 +185,10 @@ class TestBrowserSession { TestBrowserSession(this._browser, this._context); /// Creates a new page and setup overrides and tracking. - Future withPage({required Future Function(Page page) fn}) async { + Future withPage({ + required Future Function(Page page) fn, + bool expectAllResponsesToBeCacheControlPublic = false, + }) async { final clientErrors = []; final serverErrors = []; final page = await _context.newPage(); @@ -261,20 +264,31 @@ class TestBrowserSession { } } - if (!rs.url.startsWith('data:') && - // exempt the image URL from markdown_samples.md - rs.url != 'https://pub.dev/static/img/pub-dev-logo.svg') { + if (rs.status == 200 && + rs.request.method.toUpperCase() == 'GET' && + // filters out data: and other-domain URLs + rs.url.startsWith(_browser._origin)) { final uri = Uri.parse(rs.url); - if (uri.pathSegments.length > 1 && uri.pathSegments.first == 'static') { + final firstPathSegment = uri.pathSegments.firstOrNull; + + if (firstPathSegment == 'static') { if (!uri.pathSegments[1].startsWith('hash-')) { serverErrors.add('Static URL ${rs.url} is without hash segment.'); } + } + final shouldBePublic = + firstPathSegment == 'static' || + firstPathSegment == 'documentation' || + expectAllResponsesToBeCacheControlPublic; + final knownExemption = + firstPathSegment == 'experimental' || firstPathSegment == 'report'; + if (shouldBePublic && !knownExemption) { final cacheHeader = rs.headers[HttpHeaders.cacheControlHeader]; if (cacheHeader == null || !cacheHeader.contains('public') || !cacheHeader.contains('max-age')) { - serverErrors.add('Static ${rs.url} is without public caching.'); + serverErrors.add('${rs.url} is without public caching.'); } } } diff --git a/pkg/pub_integration/test/fake_sign_in_test.dart b/pkg/pub_integration/test/fake_sign_in_test.dart index a994705bae..529b00f004 100644 --- a/pkg/pub_integration/test/fake_sign_in_test.dart +++ b/pkg/pub_integration/test/fake_sign_in_test.dart @@ -43,7 +43,9 @@ void main() { // This should normally be used as a test user in higher-level tests. // However, this integration test is to verify the lower-level details // of the fake sign-in, and the relation cookie and redirect handling. - final browserSession = await fakeTestScenario.createAnonymousTestUser(); + final browserSession = await fakeTestScenario.createAnonymousTestUser( + expectAllResponsesToBeCacheControlPublic: false, + ); String? firstSessionId; // sign-in page await browserSession.withBrowserPage((page) async {