From 72a401e4671cce8df3a57ce9747bb3bb70a54cd6 Mon Sep 17 00:00:00 2001 From: James Price Date: Tue, 8 Sep 2026 11:40:07 +0100 Subject: [PATCH] fix(cloudfront): serve DefaultRootObject at the distribution root `DefaultRootObject` was parsed onto the model and never read, so a viewer request for the distribution root reached the origin as `/`. Against an S3 origin that returns the bucket's `ListBucketResult` XML instead of the SPA shell. Resolve it alongside the cache behavior and fetch it in place of the root path, preserving the query string. AWS applies the default root object to the distribution root ONLY, so a subdirectory request is never rewritten to `/`; a blank config value is ignored and a stray leading slash is normalized rather than proxied as `//index.html`. --- crates/fakecloud-cloudfront/src/dataplane.rs | 93 +++++++++++++++++-- .../tests/cloudfront_dataplane.rs | 65 +++++++++++++ 2 files changed, 151 insertions(+), 7 deletions(-) diff --git a/crates/fakecloud-cloudfront/src/dataplane.rs b/crates/fakecloud-cloudfront/src/dataplane.rs index fa191d47a..622158464 100644 --- a/crates/fakecloud-cloudfront/src/dataplane.rs +++ b/crates/fakecloud-cloudfront/src/dataplane.rs @@ -162,12 +162,20 @@ impl CloudFrontDataPlane { )); }; - let path_and_query = parts - .uri - .path_and_query() - .map(|p| p.as_str()) - .unwrap_or("/") - .to_string(); + // A root request is fetched as the distribution's DefaultRootObject; the + // query string is preserved, as CloudFront does. + let path_and_query = match &route.root_object { + Some(object) => match parts.uri.query() { + Some(q) => format!("{object}?{q}"), + None => object.clone(), + }, + None => parts + .uri + .path_and_query() + .map(|p| p.as_str()) + .unwrap_or("/") + .to_string(), + }; let url = format!("{}{path_and_query}", route.upstream.url_base); trace!(%host, path = %parts.uri.path(), origin = %route.upstream.host_header, "CloudFront data plane: proxying"); let resp = self @@ -293,6 +301,9 @@ struct RouteResolution { default_upstream: UpstreamTarget, /// CustomErrorResponses that have a response page path. error_rules: Vec, + /// `DefaultRootObject` as an origin path (`/index.html`), set only when this + /// request is for the distribution root and the distribution configures one. + root_object: Option, } /// A resolved origin address: the scheme+authority to connect to and the `Host` @@ -350,10 +361,21 @@ fn resolve_route( .collect() }) .unwrap_or_default(); + // DefaultRootObject: a request for the distribution ROOT is served the named + // object from the origin. AWS applies this to the root only -- a request for a + // subdirectory is never rewritten to `/` -- so the rewrite is + // decided here, from the original viewer path, and the cache behavior is still + // selected on that original path. + let root_object = matches!(path, "" | "/") + .then(|| cfg.default_root_object.as_deref().map(str::trim)) + .flatten() + .filter(|o| !o.is_empty()) + .map(|o| format!("/{}", o.trim_start_matches('/'))); Some(RouteResolution { upstream, default_upstream, error_rules, + root_object, }) } @@ -497,7 +519,9 @@ fn is_hop_by_hop(name: &str) -> bool { #[cfg(test)] mod tests { use super::*; - use crate::model::{AliasItems, Aliases, CustomOriginConfig, Origin}; + use crate::model::{ + AliasItems, Aliases, CustomOriginConfig, DefaultCacheBehavior, Origin, OriginItems, Origins, + }; use crate::state::StoredDistribution; use chrono::Utc; @@ -608,6 +632,61 @@ mod tests { assert_eq!(up.host_header, "b.s3-website-us-east-1.amazonaws.com"); } + fn cfg_with_root(root: Option<&str>) -> DistributionConfig { + DistributionConfig { + default_root_object: root.map(Into::into), + origins: Origins { + quantity: 1, + items: Some(OriginItems { + origin: vec![origin("b.s3.us-east-1.amazonaws.com", None)], + }), + }, + default_cache_behavior: DefaultCacheBehavior { + target_origin_id: "o".into(), + ..Default::default() + }, + ..Default::default() + } + } + + fn root_object_for(cfg: &DistributionConfig, path: &str) -> Option { + resolve_route(cfg, path, "127.0.0.1:4566") + .expect("route resolves") + .root_object + } + + #[test] + fn default_root_object_applies_to_the_distribution_root() { + let cfg = cfg_with_root(Some("index.html")); + assert_eq!(root_object_for(&cfg, "/").as_deref(), Some("/index.html")); + assert_eq!(root_object_for(&cfg, "").as_deref(), Some("/index.html")); + } + + #[test] + fn default_root_object_does_not_apply_below_the_root() { + // AWS serves the default root object for the distribution root ONLY; a + // subdirectory request is never rewritten to `/`. + let cfg = cfg_with_root(Some("index.html")); + assert_eq!(root_object_for(&cfg, "/about/"), None); + assert_eq!(root_object_for(&cfg, "/about"), None); + assert_eq!(root_object_for(&cfg, "/index.html"), None); + } + + #[test] + fn default_root_object_unset_or_blank_leaves_the_root_alone() { + for root in [None, Some(""), Some(" ")] { + assert_eq!(root_object_for(&cfg_with_root(root), "/"), None, "{root:?}"); + } + } + + #[test] + fn default_root_object_is_normalized_to_a_single_leading_slash() { + // AWS rejects a leading slash in the config, but accept one defensively + // rather than proxying a `//index.html` path to the origin. + let cfg = cfg_with_root(Some("/index.html")); + assert_eq!(root_object_for(&cfg, "/").as_deref(), Some("/index.html")); + } + #[test] fn https_only_custom_origin_uses_https_and_port() { let up = upstream_for( diff --git a/crates/fakecloud-e2e/tests/cloudfront_dataplane.rs b/crates/fakecloud-e2e/tests/cloudfront_dataplane.rs index d65d40c1f..1f89afb65 100644 --- a/crates/fakecloud-e2e/tests/cloudfront_dataplane.rs +++ b/crates/fakecloud-e2e/tests/cloudfront_dataplane.rs @@ -69,6 +69,33 @@ pub async fn make_spa_distribution( .clone() } +/// Create a SPA distribution that also sets `DefaultRootObject`. +pub async fn make_spa_distribution_with_root_object( + cf: &aws_sdk_cloudfront::Client, + default_origin_domain: &str, + root_object: &str, +) -> aws_sdk_cloudfront::types::Distribution { + let mut config = spa_config( + default_origin_domain, + None, + &format!("spa-{}", uuid_like()), + true, + true, + &[], + ); + config.default_root_object = Some(root_object.to_string()); + let create = cf + .create_distribution() + .distribution_config(config) + .send() + .await + .expect("create_distribution"); + create + .distribution() + .expect("distribution returned") + .clone() +} + /// Build the SPA distribution config. Shared by create (enabled=true) and the /// disable-via-update path (enabled=false) so both use an identical shape; the /// `caller_reference` must be preserved across an UpdateDistribution. @@ -644,3 +671,41 @@ async fn api_traffic_is_not_intercepted() { .await .expect("s3 list_buckets must pass through the viewer middleware"); } + +/// Regression: `DefaultRootObject` was stored on the model but never applied, so a +/// viewer request for the distribution root reached the bucket root and returned +/// S3's `ListBucketResult` XML instead of the SPA shell. +#[tokio::test] +async fn serves_default_root_object_at_the_distribution_root() { + let server = TestServer::start().await; + let s3 = server.s3_client().await; + s3.create_bucket() + .bucket("rootsite") + .send() + .await + .expect("create_bucket"); + put_object(&s3, "rootsite", "index.html", "text/html", b"SHELL").await; + put_object(&s3, "rootsite", "nested/index.html", "text/html", b"NESTED").await; + + let cf = server.cloudfront_client().await; + let dist = make_spa_distribution_with_root_object( + &cf, + "rootsite.s3-website-us-east-1.amazonaws.com", + "index.html", + ) + .await; + assert!(wait_for_served(&server, dist.id(), Duration::from_secs(10)).await); + let host = dist.domain_name(); + + // The root serves the default root object, not a bucket listing. + let r = viewer_get(&server, host, "/").await; + assert_eq!(r.status(), 200); + assert_eq!(r.text().await.unwrap(), "SHELL"); + + // A subdirectory is NOT rewritten to `nested/index.html`: AWS applies the + // default root object to the distribution root only. Here the miss falls + // through to the SPA CustomErrorResponse rule instead. + let r = viewer_get(&server, host, "/nested/").await; + assert_eq!(r.status(), 200); + assert_eq!(r.text().await.unwrap(), "SHELL"); +}