Skip to content

Commit 7730c4f

Browse files
Fix JS asset proxy review findings
1 parent e9a6705 commit 7730c4f

9 files changed

Lines changed: 393 additions & 25 deletions

File tree

crates/trusted-server-adapter-fastly/src/main.rs

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,10 @@ enum HandlerOutcome {
100100
response: HttpResponse,
101101
body: EdgeBody,
102102
},
103+
RawStreaming {
104+
response: HttpResponse,
105+
body: EdgeBody,
106+
},
103107
}
104108

105109
impl HandlerOutcome {
@@ -108,11 +112,23 @@ impl HandlerOutcome {
108112
match self {
109113
HandlerOutcome::Buffered(resp) | HandlerOutcome::AuthChallenge(resp) => resp.status(),
110114
HandlerOutcome::Streaming { response, .. }
111-
| HandlerOutcome::AssetStreaming { response, .. } => response.status(),
115+
| HandlerOutcome::AssetStreaming { response, .. }
116+
| HandlerOutcome::RawStreaming { response, .. } => response.status(),
112117
}
113118
}
114119
}
115120

121+
fn response_to_legacy_outcome(response: HttpResponse) -> HandlerOutcome {
122+
let (parts, body) = response.into_parts();
123+
match body {
124+
EdgeBody::Stream(_) => HandlerOutcome::RawStreaming {
125+
response: HttpResponse::from_parts(parts, EdgeBody::empty()),
126+
body,
127+
},
128+
body => HandlerOutcome::Buffered(HttpResponse::from_parts(parts, body)),
129+
}
130+
}
131+
116132
/// Returns `true` if the raw config-store value represents an enabled flag.
117133
///
118134
/// Accepted values (after whitespace trimming): `"1"` or `"true"` in any ASCII case.
@@ -912,6 +928,46 @@ fn legacy_main(mut req: FastlyRequest) {
912928
log::error!("failed to finish asset streaming body: {e}");
913929
}
914930
}
931+
HandlerOutcome::RawStreaming { mut response, body } => {
932+
finalize_response(&state.settings, geo_info.as_ref(), &mut response);
933+
asset_cache_policy.apply_after_route_finalization(&mut response);
934+
if should_finalize_ec {
935+
ec_finalize_response(
936+
&state.settings,
937+
&ec_context,
938+
finalize_kv_graph.as_ref(),
939+
&partner_registry,
940+
eids_cookie.as_deref(),
941+
sharedid_cookie.as_deref(),
942+
&mut response,
943+
);
944+
}
945+
request_filter_effects.apply_to_response(&mut response);
946+
let fastly_resp = compat::to_fastly_response_skeleton(response);
947+
let mut streaming_body = fastly_resp.stream_to_client();
948+
let mut stream_succeeded = false;
949+
if let Err(e) =
950+
futures::executor::block_on(stream_asset_body(body, &mut streaming_body))
951+
{
952+
log::error!("raw response streaming failed: {e:?}");
953+
drop(streaming_body);
954+
} else if let Err(e) = streaming_body.finish() {
955+
log::error!("failed to finish raw streaming body: {e}");
956+
} else {
957+
stream_succeeded = true;
958+
}
959+
960+
if is_real_browser && stream_succeeded {
961+
if let Some(context) = build_pull_sync_context(&ec_context) {
962+
run_pull_sync_after_send(
963+
&state.settings,
964+
&partner_registry,
965+
&context,
966+
&runtime_services,
967+
);
968+
}
969+
}
970+
}
915971
}
916972
}
917973

@@ -1359,7 +1415,7 @@ async fn route_request(
13591415
let _ = organic_route;
13601416

13611417
let outcome = result
1362-
.map(HandlerOutcome::Buffered)
1418+
.map(response_to_legacy_outcome)
13631419
.unwrap_or_else(|e| HandlerOutcome::Buffered(http_error_response(&e)));
13641420

13651421
Ok(RouteResult {

crates/trusted-server-adapter-fastly/src/route_tests.rs

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1551,6 +1551,84 @@ fn asset_routes_stream_asset_responses_directly() {
15511551
// take the `response: None` direct-send path with EC finalization skipped.
15521552
}
15531553

1554+
#[test]
1555+
fn integration_routes_preserve_streaming_proxy_responses() {
1556+
let mut settings = create_test_settings();
1557+
settings
1558+
.integrations
1559+
.insert_config(
1560+
"js_asset_proxy",
1561+
&json!({
1562+
"enabled": true,
1563+
"assets": [{
1564+
"path": "/assets/vendor.js",
1565+
"origin_url": "https://cdn.example.com/vendor.js",
1566+
"proxy": "enabled"
1567+
}]
1568+
}),
1569+
)
1570+
.expect("should insert JS asset proxy config");
1571+
let orchestrator = build_orchestrator(&settings).expect("should build auction orchestrator");
1572+
let integration_registry =
1573+
IntegrationRegistry::new(&settings).expect("should create integration registry");
1574+
let partner_registry = test_partner_registry(&settings);
1575+
1576+
let fastly_req = Request::get("https://test.com/assets/vendor.js");
1577+
let http_client = Arc::new(StreamingRecordingHttpClient::new());
1578+
let services = test_runtime_services_with_secret_http_client_and_geo(
1579+
&fastly_req,
1580+
Arc::new(FixedBackend),
1581+
Arc::new(NoopSecretStore),
1582+
Arc::clone(&http_client) as Arc<dyn PlatformHttpClient>,
1583+
Arc::new(FixedGeo(us_california_geo())),
1584+
);
1585+
let req = compat::from_fastly_request(fastly_req);
1586+
1587+
let outcome = futures::executor::block_on(route_request(
1588+
&settings,
1589+
&orchestrator,
1590+
&integration_registry,
1591+
&partner_registry,
1592+
&services,
1593+
req,
1594+
browser_device_signals(),
1595+
))
1596+
.expect("should route streaming integration request");
1597+
1598+
let (response, body) = match outcome.outcome {
1599+
HandlerOutcome::RawStreaming { response, body } => Some((response, body)),
1600+
_ => None,
1601+
}
1602+
.expect("should preserve streaming integration response");
1603+
assert_eq!(response.status(), StatusCode::OK);
1604+
assert!(
1605+
matches!(body, EdgeBody::Stream(_)),
1606+
"should preserve the integration response stream"
1607+
);
1608+
1609+
let calls = http_client
1610+
.calls
1611+
.lock()
1612+
.expect("should lock recorded calls");
1613+
assert_eq!(
1614+
calls.len(),
1615+
1,
1616+
"should send exactly one integration request"
1617+
);
1618+
assert!(
1619+
calls[0].stream_response,
1620+
"JS asset proxy should request a streaming origin response from the platform"
1621+
);
1622+
assert_eq!(
1623+
calls[0].backend_name, "https-cdn.example.com",
1624+
"should resolve the configured JS asset origin backend"
1625+
);
1626+
assert_eq!(
1627+
calls[0].uri, "https://cdn.example.com/vendor.js",
1628+
"should send the request to the configured JS asset origin URL"
1629+
);
1630+
}
1631+
15541632
#[test]
15551633
fn asset_origin_failure_does_not_fall_back_to_publisher_origin() {
15561634
let mut settings = create_test_settings();

crates/trusted-server-cli/src/audit.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ fn build_js_asset_proxy_section(
426426
"# Audit note: some discovered scripts may be runtime-injected and may not appear\n",
427427
);
428428
toml.push_str(
429-
"# in origin HTML. JS Asset Proxy rewrites only exact script src values present in\n",
429+
"# in origin HTML. JS Asset Proxy rewrites only matching script src URLs present in\n",
430430
);
431431
toml.push_str("# HTML processed by Trusted Server.\n");
432432

0 commit comments

Comments
 (0)