Skip to content

Commit 5b5f20b

Browse files
swernerclaude
andcommitted
server: reuse rust-embed's compile-time SHA-256 for asset ETags; cleanups
Mutable-asset ETags were rehashing the full body on every request, and embedded assets were copied out of the binary per request (including on the 304 path, where the body is discarded). fabro-spa now exposes the SHA-256 that rust-embed precomputes at compile time, and the server serves embedded bytes via Bytes::from_static instead of copying. Disk-served assets (dev/watch mode, explicit asset roots) still hash on read for freshness. Also: derive the ETag decision from is_content_hashed() instead of string-comparing Cache-Control values, dedupe the compression tests' fixture/server scaffolding into local helpers, and drop a redundant String() coercion in the web build script. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent e72199b commit 5b5f20b

4 files changed

Lines changed: 154 additions & 93 deletions

File tree

apps/fabro-web/scripts/build.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ async function buildOnce() {
8080
await writeIndexHtml(
8181
buildDir,
8282
result.outputs.map((output: any) => ({
83-
kind: String(output.kind),
83+
kind: output.kind,
8484
path: relative(buildDir, output.path),
8585
})),
8686
);
@@ -103,7 +103,12 @@ async function copyPierreWorkerAssets(targetDir: string) {
103103
}
104104
}
105105

106-
type IndexHtmlOutput = { kind: string; path: string };
106+
// `kind` mirrors Bun's `BuildArtifact.kind`; the union keeps the
107+
// "entry-point" comparison below typo-safe.
108+
type IndexHtmlOutput = {
109+
kind: "entry-point" | "chunk" | "asset" | "sourcemap" | "bytecode";
110+
path: string;
111+
};
107112

108113
async function writeIndexHtml(buildDir: string, outputs: IndexHtmlOutput[]) {
109114
const template = await readFile(templatePath, "utf8");

lib/crates/fabro-server/src/static_files.rs

Lines changed: 83 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1+
use std::borrow::Cow;
12
use std::path::{Path, PathBuf};
23
use std::sync::OnceLock;
34

4-
use axum::body::Body;
5+
use axum::body::{Body, Bytes};
56
use axum::http::{HeaderMap, HeaderValue, StatusCode, header};
67
use axum::response::{IntoResponse, Response};
78
use fabro_static::EnvVars;
@@ -101,9 +102,8 @@ async fn load_injected_install_shell(
101102
asset_root: Option<&Path>,
102103
dev_disk_only: bool,
103104
) -> Option<Vec<u8>> {
104-
Some(inject_install_mode(
105-
load_asset("index.html", asset_root, dev_disk_only).await?,
106-
))
105+
let shell = load_asset("index.html", asset_root, dev_disk_only).await?;
106+
Some(inject_install_mode(shell.bytes.into()))
107107
}
108108

109109
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
@@ -188,7 +188,39 @@ fn normalize(path: &str) -> String {
188188
}
189189
}
190190

191-
async fn load_asset(path: &str, asset_root: Option<&Path>, dev_disk_only: bool) -> Option<Vec<u8>> {
191+
/// An asset body plus, when the source precomputed it (the embedded SPA
192+
/// snapshot), its SHA-256. Carrying the hash lets mutable-asset ETags reuse
193+
/// rust-embed's compile-time digest instead of rehashing process-lifetime
194+
/// bytes on every revalidation.
195+
struct Asset {
196+
bytes: Bytes,
197+
sha256: Option<[u8; 32]>,
198+
}
199+
200+
impl Asset {
201+
fn from_vec(bytes: Vec<u8>) -> Self {
202+
Self {
203+
bytes: bytes.into(),
204+
sha256: None,
205+
}
206+
}
207+
208+
fn from_embedded(asset: fabro_spa::AssetBytes) -> Self {
209+
let sha256 = asset.sha256();
210+
let bytes = match asset.into_cow() {
211+
// Release builds embed assets as statics; serve them without
212+
// copying the (potentially multi-megabyte) body per request.
213+
Cow::Borrowed(bytes) => Bytes::from_static(bytes),
214+
Cow::Owned(bytes) => Bytes::from(bytes),
215+
};
216+
Self {
217+
bytes,
218+
sha256: Some(sha256),
219+
}
220+
}
221+
}
222+
223+
async fn load_asset(path: &str, asset_root: Option<&Path>, dev_disk_only: bool) -> Option<Asset> {
192224
if spa_assets_disabled_for_test() {
193225
return None;
194226
}
@@ -197,11 +229,11 @@ async fn load_asset(path: &str, asset_root: Option<&Path>, dev_disk_only: bool)
197229
// workspace's live `dist/` fallback or test isolation breaks.
198230
if let Some(root) = asset_root {
199231
if let Some(bytes) = read_disk_asset_from_root(root, path).await {
200-
return Some(bytes);
232+
return Some(Asset::from_vec(bytes));
201233
}
202234
} else if cfg!(debug_assertions) {
203235
if let Some(bytes) = read_disk_asset(path).await {
204-
return Some(bytes);
236+
return Some(Asset::from_vec(bytes));
205237
}
206238
}
207239

@@ -212,17 +244,19 @@ async fn load_asset(path: &str, asset_root: Option<&Path>, dev_disk_only: bool)
212244
return None;
213245
}
214246

215-
fabro_spa::get(path).map(fabro_spa::AssetBytes::into_vec)
247+
fabro_spa::get(path).map(Asset::from_embedded)
216248
}
217249

218250
async fn load_asset_for_mode(
219251
path: &str,
220252
mode: SpaMode,
221253
asset_root: Option<&Path>,
222254
dev_disk_only: bool,
223-
) -> Option<Vec<u8>> {
255+
) -> Option<Asset> {
224256
if mode == SpaMode::Install && path == "index.html" {
225-
return cached_install_mode_shell(asset_root, dev_disk_only).await;
257+
return cached_install_mode_shell(asset_root, dev_disk_only)
258+
.await
259+
.map(Asset::from_vec);
226260
}
227261
load_asset(path, asset_root, dev_disk_only).await
228262
}
@@ -280,13 +314,14 @@ fn disk_asset_root() -> PathBuf {
280314
const IMMUTABLE_CACHE_CONTROL: &str = "public, max-age=31536000, immutable";
281315
const REVALIDATE_CACHE_CONTROL: &str = "no-cache";
282316

283-
fn asset_response(path: &str, bytes: Vec<u8>, request_headers: &HeaderMap) -> Response {
284-
let cache_control = cache_control(path);
317+
fn asset_response(path: &str, asset: Asset, request_headers: &HeaderMap) -> Response {
318+
let content_hashed = is_content_hashed(path);
319+
let cache_control = cache_control(content_hashed);
285320
// Mutable assets keep stable names across deploys, so their `no-cache`
286321
// policy needs a validator to revalidate as a cheap 304 instead of a full
287322
// body download on every use. Hashed immutable assets never revalidate,
288-
// so hashing their (multi-megabyte) bytes per request would be waste.
289-
let etag = (cache_control == REVALIDATE_CACHE_CONTROL).then(|| asset_etag(&bytes));
323+
// so an ETag would be dead weight.
324+
let etag = (!content_hashed).then(|| asset_etag(&asset));
290325

291326
if let Some(etag) = &etag {
292327
if if_none_match_matches(request_headers, etag) {
@@ -298,7 +333,7 @@ fn asset_response(path: &str, bytes: Vec<u8>, request_headers: &HeaderMap) -> Re
298333
}
299334

300335
let mime = mime_guess::from_path(path).first_or_octet_stream();
301-
let mut response = Response::new(Body::from(bytes));
336+
let mut response = Response::new(Body::from(asset.bytes));
302337
*response.status_mut() = StatusCode::OK;
303338
response.headers_mut().insert(
304339
header::CONTENT_TYPE,
@@ -321,8 +356,11 @@ fn apply_cache_headers(headers: &mut HeaderMap, cache_control: &'static str, eta
321356
}
322357
}
323358

324-
fn asset_etag(bytes: &[u8]) -> String {
325-
format!("\"{}\"", hex::encode(Sha256::digest(bytes)))
359+
fn asset_etag(asset: &Asset) -> String {
360+
let digest = asset
361+
.sha256
362+
.unwrap_or_else(|| Sha256::digest(&asset.bytes).into());
363+
format!("\"{}\"", hex::encode(digest))
326364
}
327365

328366
fn if_none_match_matches(headers: &HeaderMap, etag: &str) -> bool {
@@ -336,8 +374,8 @@ fn if_none_match_matches(headers: &HeaderMap, etag: &str) -> bool {
336374
})
337375
}
338376

339-
fn cache_control(path: &str) -> &'static str {
340-
if is_content_hashed(path) {
377+
fn cache_control(content_hashed: bool) -> &'static str {
378+
if content_hashed {
341379
IMMUTABLE_CACHE_CONTROL
342380
} else {
343381
REVALIDATE_CACHE_CONTROL
@@ -395,8 +433,8 @@ mod tests {
395433
use axum::http::{HeaderMap, HeaderValue, StatusCode, header};
396434

397435
use super::{
398-
accepts_html, cache_control, inject_install_mode, is_source_map, read_disk_asset_from_root,
399-
serve_with_asset_root,
436+
accepts_html, cache_control, inject_install_mode, is_content_hashed, is_source_map,
437+
read_disk_asset_from_root, serve_with_asset_root,
400438
};
401439

402440
fn headers_with_accept(value: &str) -> HeaderMap {
@@ -431,35 +469,36 @@ mod tests {
431469

432470
#[test]
433471
fn hashed_assets_are_cached_immutably() {
434-
assert_eq!(
435-
cache_control("assets/entry-0sv53bs3.js"),
436-
"public, max-age=31536000, immutable"
437-
);
438-
assert_eq!(
439-
cache_control("assets/chunk-4tr91ktd.js"),
440-
"public, max-age=31536000, immutable"
441-
);
442-
assert_eq!(
443-
cache_control("assets/chunk-x912wb67.css"),
444-
"public, max-age=31536000, immutable"
445-
);
472+
for path in [
473+
"assets/entry-0sv53bs3.js",
474+
"assets/chunk-4tr91ktd.js",
475+
"assets/chunk-x912wb67.css",
476+
] {
477+
assert!(is_content_hashed(path), "{path} should be content-hashed");
478+
}
479+
assert_eq!(cache_control(true), "public, max-age=31536000, immutable");
446480
}
447481

448482
#[test]
449483
fn stable_named_assets_must_revalidate() {
450484
// Files whose names do NOT change when their bytes change would be
451485
// pinned stale in browsers for a year if marked immutable.
452-
assert_eq!(cache_control("index.html"), "no-cache");
453-
assert_eq!(cache_control("assets/app.css"), "no-cache");
454-
assert_eq!(
455-
cache_control("assets/pierre-diffs-worker/worker-portable.js"),
456-
"no-cache"
457-
);
458-
assert_eq!(cache_control("images/apple-touch-icon.png"), "no-cache");
459-
// Dash segment that isn't an 8-char lowercase base-36 hash.
460-
assert_eq!(cache_control("assets/entry-abc123.js"), "no-cache");
461-
// Right hash shape, but not a bundler output extension.
462-
assert_eq!(cache_control("assets/photo-a1b2c3d4.png"), "no-cache");
486+
for path in [
487+
"index.html",
488+
"assets/app.css",
489+
"assets/pierre-diffs-worker/worker-portable.js",
490+
"images/apple-touch-icon.png",
491+
// Dash segment that isn't an 8-char lowercase base-36 hash.
492+
"assets/entry-abc123.js",
493+
// Right hash shape, but not a bundler output extension.
494+
"assets/photo-a1b2c3d4.png",
495+
] {
496+
assert!(
497+
!is_content_hashed(path),
498+
"{path} should not be content-hashed"
499+
);
500+
}
501+
assert_eq!(cache_control(false), "no-cache");
463502
}
464503

465504
#[tokio::test]

lib/crates/fabro-server/tests/it/api/compression.rs

Lines changed: 41 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,50 @@
1010
reason = "integration tests stage fixtures with sync std::fs; test infrastructure, not Tokio-hot path"
1111
)]
1212

13+
use std::net::SocketAddr;
14+
15+
use axum::Router;
1316
use axum::body::Body;
1417
use axum::http::{Request, StatusCode, header};
1518
use fabro_server::server::RouterOptions;
19+
use tempfile::TempDir;
1620
use tower::ServiceExt;
1721

1822
use crate::helpers::{api, test_app_state};
1923

24+
/// Router serving an SPA shell comfortably above the compression size floor,
25+
/// through the same fallback service production uses for static assets.
26+
fn spa_router_with_big_index() -> (Router, TempDir) {
27+
let temp_dir = tempfile::tempdir().unwrap();
28+
std::fs::write(
29+
temp_dir.path().join("index.html"),
30+
format!("<!doctype html><title>spa</title>{}", "x".repeat(8192)),
31+
)
32+
.unwrap();
33+
let app = fabro_server::test_support::build_test_router_with_options(
34+
test_app_state(),
35+
RouterOptions {
36+
web_enabled: true,
37+
static_asset_root: Some(temp_dir.path().to_path_buf()),
38+
..RouterOptions::default()
39+
},
40+
);
41+
(app, temp_dir)
42+
}
43+
44+
async fn serve_on_ephemeral_port(app: Router) -> SocketAddr {
45+
let listener = tokio::net::TcpListener::bind("127.0.0.1:0")
46+
.await
47+
.expect("test TCP listener should bind");
48+
let addr = listener
49+
.local_addr()
50+
.expect("test TCP listener should have a local address");
51+
tokio::spawn(async move {
52+
let _ = axum::serve(listener, app).await;
53+
});
54+
addr
55+
}
56+
2057
fn openapi_request(accept_encoding: Option<&str>) -> Request<Body> {
2158
let mut builder = Request::builder().method("GET").uri(api("/openapi.json"));
2259
if let Some(encoding) = accept_encoding {
@@ -67,21 +104,7 @@ async fn spa_assets_are_compressed() {
67104
// SPA assets are served by the router's fallback service, not a regular
68105
// route — this test pins that compression covers that path too, since the
69106
// multi-megabyte JS bundle is the single largest thing the server sends.
70-
let temp_dir = tempfile::tempdir().unwrap();
71-
std::fs::write(
72-
temp_dir.path().join("index.html"),
73-
format!("<!doctype html><title>spa</title>{}", "x".repeat(8192)),
74-
)
75-
.unwrap();
76-
77-
let app = fabro_server::test_support::build_test_router_with_options(
78-
test_app_state(),
79-
RouterOptions {
80-
web_enabled: true,
81-
static_asset_root: Some(temp_dir.path().to_path_buf()),
82-
..RouterOptions::default()
83-
},
84-
);
107+
let (app, _temp_dir) = spa_router_with_big_index();
85108
let request = Request::builder()
86109
.method("GET")
87110
.uri("/")
@@ -107,16 +130,8 @@ async fn compression_applies_over_a_real_tcp_connection() {
107130
// `oneshot` exercises the tower stack directly; this pins the same
108131
// behavior through hyper's real connection handling, matching how the
109132
// production server actually serves (`axum::serve`).
110-
let listener = tokio::net::TcpListener::bind("127.0.0.1:0")
111-
.await
112-
.expect("test TCP listener should bind");
113-
let addr = listener
114-
.local_addr()
115-
.expect("test TCP listener should have a local address");
116133
let app = fabro_server::test_support::build_test_router(test_app_state());
117-
tokio::spawn(async move {
118-
let _ = axum::serve(listener, app).await;
119-
});
134+
let addr = serve_on_ephemeral_port(app).await;
120135

121136
let response = fabro_test::test_http_client()
122137
.get(format!("http://{addr}/api/v1/openapi.json"))
@@ -142,25 +157,8 @@ async fn compression_applies_over_a_real_tcp_connection() {
142157
async fn spa_assets_compress_over_a_real_tcp_connection() {
143158
use tokio::io::{AsyncReadExt, AsyncWriteExt};
144159

145-
let temp_dir = tempfile::tempdir().unwrap();
146-
std::fs::write(
147-
temp_dir.path().join("index.html"),
148-
format!("<!doctype html><title>spa</title>{}", "x".repeat(8192)),
149-
)
150-
.unwrap();
151-
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
152-
let addr = listener.local_addr().unwrap();
153-
let app = fabro_server::test_support::build_test_router_with_options(
154-
test_app_state(),
155-
RouterOptions {
156-
web_enabled: true,
157-
static_asset_root: Some(temp_dir.path().to_path_buf()),
158-
..RouterOptions::default()
159-
},
160-
);
161-
tokio::spawn(async move {
162-
let _ = axum::serve(listener, app).await;
163-
});
160+
let (app, _temp_dir) = spa_router_with_big_index();
161+
let addr = serve_on_ephemeral_port(app).await;
164162

165163
// Raw HTTP/1.1 over the socket: no client-side redirect following or
166164
// transparent decompression can distort what the server actually sent.

0 commit comments

Comments
 (0)