From a2285c0e91063cf0f07637664c6055acdcacd9a8 Mon Sep 17 00:00:00 2001 From: Owen Jacobson Date: Tue, 2 Dec 2025 02:35:39 -0500 Subject: Ask clients to avoid re-requesting immutable assets for 90 days. Immutable assets include compiled script chunks, as well as stylesheets, fonts, and images baked into the SvelteKit app bundle. They do _not_ include the service worker, the PWA manifest, or the version and environment JSON files that SvelteKit uses to configure the application on startup. This complements Pilcrow's `If-None-Modified` support: if a browser asks but has a cached copy locally, we use the `If-Not-Modified` header to detect that and send back a response instructing the client to use that copy, saving bandwidth but not round trips. This change instructs clients not to even try asking, if they're willing to cache the response, and to instead satisfy those assets entirely from cache, saving round trips. `Duration::from_hours` was added in Rust 1.91, so this change also includes a minimum Rust version bump. --- Cargo.toml | 2 +- src/ui/assets.rs | 87 ++++++++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 73 insertions(+), 16 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5a5513b..4085a19 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ name = "pilcrow" version = "0.1.0" edition = "2024" -rust-version = "1.90" +rust-version = "1.91" authors = [ "Owen Jacobson ", "Kit La Touche ", diff --git a/src/ui/assets.rs b/src/ui/assets.rs index b8ca00b..2fa703b 100644 --- a/src/ui/assets.rs +++ b/src/ui/assets.rs @@ -1,12 +1,12 @@ -use std::str::FromStr; +use std::{str::FromStr, time::Duration}; use ::mime::Mime; use axum::{ http::StatusCode, - response::{self, IntoResponse}, + response::{self, IntoResponse, IntoResponseParts, ResponseParts}, }; use axum_extra::TypedHeader; -use headers::{ContentType, ETag, IfNoneMatch}; +use headers::{CacheControl, ContentType, ETag, IfNoneMatch}; use rust_embed::EmbeddedFile; use super::{error::NotFound, mime}; @@ -19,11 +19,12 @@ use crate::error::{ #[folder = "$OUT_DIR/ui"] pub struct Assets; +// Prefer the corresponding methods in `Response` unless you specifically do not want HTTP caching +// behaviours. impl Assets { - pub fn load(path: impl AsRef) -> Result { - let path = path.as_ref(); - let mime = mime::from_path(path).fail("Failed to determine MIME type from asset path")?; - + pub fn load(path: &str) -> Result { + let mime = mime::from_path(path) + .fail_with(|| format!("Failed to determine MIME type from asset path: {path}"))?; let file = Self::get(path).ok_or(Error::NotFound(path.into()))?; let asset = Asset::new(mime, file)?; @@ -60,7 +61,7 @@ impl Asset { Ok(tag) } - pub fn differs_from(&self, if_none_match: &IfNoneMatch) -> bool { + fn differs_from(&self, if_none_match: &IfNoneMatch) -> bool { if_none_match.precondition_passes(&self.etag) } } @@ -84,25 +85,37 @@ impl IntoResponse for Asset { #[expect(clippy::large_enum_variant)] pub enum Response { NotModified, - Asset(Asset), + // I've opted to make Mutability part of the conditional-HTTP-response logic here and not part + // of the underlying Asset struct, even though it would be pretty reasonable to deem that it is + // the Asset that is immutable rather than its specific binding to an HTTP result. Assets don't + // know their own path, and there isn't a tidy way to tag that information onto Asset at + // construction without refactoring a _ton_ of things that are fine the way they are, though, + // and since mutability only affects an HTTP header and nothing else, the resulting structs + // work well enough. + Asset(Asset, Mutability), } +// This interface parallels the `Assets` interface, above. However, unlike `Assets`, this supports +// browser and intermediate server cache mechanisms, both by considering the provided +// `If-None-Match` header against each asset's `ETag`, and by tacking on cache control headers for +// static assets that we commit to never changing. impl Response { - pub fn load(path: impl AsRef, if_none_match: &IfNoneMatch) -> Result { + pub fn load(path: &str, if_none_match: &IfNoneMatch) -> Result { + let mutability = Mutability::from_path(path); let asset = Assets::load(path)?; - let response = Self::respond_with(asset, if_none_match); + let response = Self::respond_with(asset, if_none_match, mutability); Ok(response) } pub fn index(if_none_match: &IfNoneMatch) -> Result { let asset = Assets::index()?; - let response = Self::respond_with(asset, if_none_match); + let response = Self::respond_with(asset, if_none_match, Mutability::Mutable); Ok(response) } - fn respond_with(asset: Asset, if_none_match: &IfNoneMatch) -> Self { + fn respond_with(asset: Asset, if_none_match: &IfNoneMatch, mutability: Mutability) -> Self { if asset.differs_from(if_none_match) { - Self::Asset(asset) + Self::Asset(asset, mutability) } else { Self::NotModified } @@ -113,7 +126,51 @@ impl IntoResponse for Response { fn into_response(self) -> response::Response { match self { Self::NotModified => StatusCode::NOT_MODIFIED.into_response(), - Self::Asset(asset) => asset.into_response(), + Self::Asset(asset, mutability) => (mutability, asset).into_response(), + } + } +} + +pub enum Mutability { + Immutable, + Mutable, +} + +impl Mutability { + fn from_path(path: &str) -> Self { + // This is a complete hack and depends intimately on the output of SvelteKit's static site + // adapter in ways that are not documented anywhere I can find. On the other hand, I also + // can't find any sign of a better way to do this, and I think the odds of us (or SvelteKit) + // messing with these paths in a way that breaks this cacheability check are pretty low. + if path.starts_with("_app/immutable/") { + Self::Immutable + } else { + Self::Mutable + } + } +} + +impl IntoResponseParts for Mutability { + type Error = as IntoResponseParts>::Error; + + fn into_response_parts(self, res: ResponseParts) -> Result { + // 90 days is pretty arbitrary. Everything _works_ if immutable assets get + // re-requested more frequently than this (or even on every page view), but it eats + // up bandwidth and battery to transfer them, and they only ever change to and from + // "404 Not Found" (which we do not instruct browsers to cache). If it was free to + // cache them forever, we'd ask for that. + // + // If a user doesn't visit the app for 90 days, they're probably better off re-downloading + // the whole thing if they ever come back, and better off getting that cache space back for + // something else, until then. + match self { + Self::Immutable => TypedHeader( + CacheControl::new() + .with_immutable() + .with_max_age(Duration::from_hours(90 * 24)), + ) + .into_response_parts(res), + Self::Mutable => ().into_response_parts(res), } } } -- cgit v1.2.3