Skip to content

Commit 38a1cee

Browse files
committed
mirror_worker: tighten add-entries spec compliance
Enforce the spec's Content-Type MUST, advertise gzip support in responses, and use a fresh DO snapshot when a concurrent commit forces a final 409 so the mirror-info body is not stale.
1 parent b41d7e9 commit 38a1cee

2 files changed

Lines changed: 104 additions & 4 deletions

File tree

crates/mirror_worker/src/add_entries.rs

Lines changed: 70 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,16 @@ pub(crate) async fn add_entries(
6868
State(env): State<Env>,
6969
req: axum::extract::Request,
7070
) -> ApiResult<axum::response::Response> {
71+
// Spec: the add-entries request body MUST have Content-Type
72+
// application/octet-stream. Reject anything else up front (before
73+
// spending time reading or decoding the body).
74+
let (parts, body) = req.into_parts();
75+
if !content_type_is_octet_stream(&parts.headers) {
76+
return Err(AppError::UnsupportedMediaType(
77+
"add-entries requires Content-Type: application/octet-stream".to_owned(),
78+
));
79+
}
80+
7181
// No DefaultBodyLimit: Cloudflare enforces a request-body cap at the
7282
// edge (100 MB, higher on paid plans) and 413s oversized bodies there.
7383
// Clients on body-limited platforms truncate at a package boundary and
@@ -78,7 +88,6 @@ pub(crate) async fn add_entries(
7888
//
7989
// The whole (decoded) body is buffered before processing; a follow-up
8090
// commit streams it instead of buffering it all.
81-
let (parts, body) = req.into_parts();
8291
let raw = body::read_decoded_body(&parts.headers, body).await?;
8392
let mut cursor = Cursor::new(raw.as_slice());
8493

@@ -182,6 +191,20 @@ pub(crate) async fn add_entries(
182191
.await
183192
}
184193

194+
/// Return true iff the request's `Content-Type` is
195+
/// `application/octet-stream`, ignoring any parameters (e.g. charset).
196+
fn content_type_is_octet_stream(headers: &axum::http::HeaderMap) -> bool {
197+
headers
198+
.get(CONTENT_TYPE)
199+
.and_then(|v| v.to_str().ok())
200+
.unwrap_or_default()
201+
.split(';')
202+
.next()
203+
.unwrap_or_default()
204+
.trim()
205+
== "application/octet-stream"
206+
}
207+
185208
/// Read, verify, and persist the entry packages for `[upload_start,
186209
/// upload_end)`, then produce the HTTP response.
187210
///
@@ -453,13 +476,25 @@ async fn cosign_and_serve(
453476
if committed.size != header.upload_end {
454477
// The DO refused to rewind: a concurrent commit already advanced
455478
// the mirror checkpoint past upload_end, so ours was skipped.
479+
// Fetch the latest state so the 409 mirror-info body advertises
480+
// the current pending size and next entry, not the stale snapshot
481+
// from the start of this request.
456482
log::info!(
457483
"add-entries: commit skipped, mirror checkpoint {} already past upload_end {}; \
458484
returning 409",
459485
committed.size,
460486
header.upload_end,
461487
);
462-
return Ok(mirror_info_409(env, snapshot, &header.log_origin));
488+
let fresh_snapshot = match fetch_snapshot(env, &header.log_origin).await {
489+
Ok(s) => s,
490+
Err(e) => {
491+
log::warn!(
492+
"add-entries: failed to fetch fresh snapshot for 409; using stale: {e:?}"
493+
);
494+
snapshot.clone()
495+
}
496+
};
497+
return Ok(mirror_info_409(env, &fresh_snapshot, &header.log_origin));
463498
}
464499

465500
Ok((
@@ -892,7 +927,9 @@ impl HashReader for MapReader<'_> {
892927

893928
#[cfg(test)]
894929
mod tests {
895-
use super::{MapReader, excess_entries, verify_package};
930+
use super::{
931+
CONTENT_TYPE, MapReader, content_type_is_octet_stream, excess_entries, verify_package,
932+
};
896933
use crate::mirror_state_do::PendingCheckpoint;
897934
use std::collections::HashMap;
898935
use tlog_core::{Hash, Subtree, stored_hash_index, stored_hashes, tree_hash};
@@ -1043,4 +1080,34 @@ mod tests {
10431080
// underflow.
10441081
assert_eq!(excess_entries(5_000, 6_000, 4_000), 0);
10451082
}
1083+
1084+
#[test]
1085+
fn content_type_octet_stream_accepted() {
1086+
let mut headers = axum::http::HeaderMap::new();
1087+
headers.insert(CONTENT_TYPE, "application/octet-stream".parse().unwrap());
1088+
assert!(content_type_is_octet_stream(&headers));
1089+
}
1090+
1091+
#[test]
1092+
fn content_type_octet_stream_with_params_accepted() {
1093+
let mut headers = axum::http::HeaderMap::new();
1094+
headers.insert(
1095+
CONTENT_TYPE,
1096+
"application/octet-stream; charset=binary".parse().unwrap(),
1097+
);
1098+
assert!(content_type_is_octet_stream(&headers));
1099+
}
1100+
1101+
#[test]
1102+
fn content_type_missing_rejected() {
1103+
let headers = axum::http::HeaderMap::new();
1104+
assert!(!content_type_is_octet_stream(&headers));
1105+
}
1106+
1107+
#[test]
1108+
fn content_type_non_octet_stream_rejected() {
1109+
let mut headers = axum::http::HeaderMap::new();
1110+
headers.insert(CONTENT_TYPE, "application/json".parse().unwrap());
1111+
assert!(!content_type_is_octet_stream(&headers));
1112+
}
10461113
}

crates/mirror_worker/src/frontend_worker.rs

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ use axum::{
2929
Json, Router,
3030
body::Bytes,
3131
extract::{DefaultBodyLimit, State},
32-
http::{StatusCode, header},
32+
http::{HeaderValue, StatusCode, header},
3333
response::IntoResponse,
3434
routing::{get, post},
3535
};
@@ -56,6 +56,22 @@ fn start() {
5656
let _ = console_log::init_with_level(level);
5757
}
5858

59+
/// Middleware that adds `Accept-Encoding: gzip` to every response.
60+
///
61+
/// [c2sp.org/tlog-mirror][spec] says mirrors SHOULD advertise supported
62+
/// compression algorithms in responses so clients can compress future
63+
/// `add-entries` request bodies.
64+
///
65+
/// [spec]: https://c2sp.org/tlog-mirror#add-entries
66+
async fn add_accept_encoding(
67+
mut response: axum::http::Response<axum::body::Body>,
68+
) -> axum::http::Response<axum::body::Body> {
69+
response
70+
.headers_mut()
71+
.insert(header::ACCEPT_ENCODING, HeaderValue::from_static("gzip"));
72+
response
73+
}
74+
5975
/// Top-level `#[event(fetch)]` handler. Delegates to the axum router;
6076
/// unmatched routes return 404.
6177
#[event(fetch, respond_with_errors)]
@@ -82,6 +98,7 @@ async fn fetch(
8298
.route("/add-entries", post(crate::add_entries::add_entries))
8399
.route("/metadata", get(metadata))
84100
.route("/", get(root))
101+
.layer(axum::middleware::map_response(add_accept_encoding))
85102
.with_state(env)
86103
.call(req)
87104
.await
@@ -414,3 +431,19 @@ fn tlog_size_conflict(current: &PendingCheckpoint) -> axum::response::Response {
414431
)
415432
.into_response()
416433
}
434+
435+
#[cfg(test)]
436+
mod tests {
437+
use super::add_accept_encoding;
438+
use axum::http::header::ACCEPT_ENCODING;
439+
440+
#[tokio::test]
441+
async fn accept_encoding_middleware_adds_gzip() {
442+
let response = axum::http::Response::new(axum::body::Body::empty());
443+
let response = add_accept_encoding(response).await;
444+
assert_eq!(
445+
response.headers().get(ACCEPT_ENCODING).unwrap().as_bytes(),
446+
b"gzip"
447+
);
448+
}
449+
}

0 commit comments

Comments
 (0)