Skip to content

Commit 0fcf9bc

Browse files
JasonVranekJasonVranek
committed
local audit fix: honor Accept q-value ordering for deterministic content
negotiation
1 parent 1927bce commit 0fcf9bc

13 files changed

Lines changed: 330 additions & 147 deletions

File tree

‎benches/microbench/src/get_header.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
//! - `HeaderMap` allocation (created once in setup, cloned cheaply per
3737
//! iteration)
3838
39-
use std::{collections::HashSet, path::PathBuf, sync::Arc, time::Duration};
39+
use std::{path::PathBuf, sync::Arc, time::Duration};
4040

4141
use alloy::primitives::B256;
4242
use axum::http::HeaderMap;
@@ -150,7 +150,7 @@ fn bench_get_header(c: &mut Criterion) {
150150
black_box(params.clone()),
151151
black_box(headers.clone()),
152152
black_box(state.clone()),
153-
black_box(HashSet::from([EncodingType::Json, EncodingType::Ssz])),
153+
black_box(vec![EncodingType::Json, EncodingType::Ssz]),
154154
))
155155
.expect("get_header failed")
156156
})

‎crates/common/src/utils.rs‎

Lines changed: 223 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#[cfg(feature = "testing-flags")]
22
use std::cell::Cell;
33
use std::{
4-
collections::{HashMap, HashSet},
4+
collections::HashMap,
55
fmt::Display,
66
net::Ipv4Addr,
77
str::FromStr,
@@ -27,7 +27,7 @@ use lh_types::{
2727
BeaconBlock,
2828
test_utils::{SeedableRng, TestRandom, XorShiftRng},
2929
};
30-
use mediatype::MediaType;
30+
use mediatype::{MediaType, ReadParams};
3131
use rand::{Rng, distr::Alphanumeric};
3232
use reqwest::{
3333
Response,
@@ -565,38 +565,108 @@ pub fn get_user_agent_with_version(req_headers: &HeaderMap) -> eyre::Result<Head
565565
Ok(HeaderValue::from_str(&format!("commit-boost/{HEADER_VERSION_VALUE} {ua}"))?)
566566
}
567567

568-
/// Parse the ACCEPT header to get the type of response to encode the body with,
569-
/// defaulting to JSON if missing. Returns an error if malformed or unsupported
570-
/// types are requested. Supports requests with multiple ACCEPT headers or
571-
/// headers with multiple media types.
572-
pub fn get_accept_types(req_headers: &HeaderMap) -> eyre::Result<HashSet<EncodingType>> {
573-
let mut accepted_types = HashSet::new();
574-
let mut unsupported_type = false;
568+
/// Deterministic outbound `Accept` header used when PBS asks a relay for a
569+
/// response it will itself decode (validation mode On/Extra). SSZ is preferred
570+
/// for wire efficiency. Emitted verbatim so packet captures and support
571+
/// tickets are reproducible.
572+
pub const OUTBOUND_ACCEPT: &str = "application/octet-stream;q=1.0,application/json;q=0.9";
573+
574+
/// Parse the ACCEPT header into a q-value ordered list of supported
575+
/// [`EncodingType`]s (highest preference first, deduplicated), defaulting to
576+
/// the request's Content-Type when no Accept header is present. Returns an
577+
/// error only if every media type in the header is malformed or unsupported.
578+
/// Supports requests with multiple ACCEPT headers or headers with multiple
579+
/// media types. `q=0` entries are treated as explicit rejections per
580+
/// RFC 7231 §5.3.1 and are skipped.
581+
///
582+
/// The returned order honors the RFC 9110 §12.5.1 precedence rules already
583+
/// applied by `headers_accept::Accept::media_types()` (specificity, then
584+
/// q-value, then original order).
585+
pub fn get_accept_types(req_headers: &HeaderMap) -> eyre::Result<Vec<EncodingType>> {
586+
let mut ordered: Vec<EncodingType> = Vec::new();
587+
let mut saw_any = false;
588+
let mut had_supported = false;
575589
for header in req_headers.get_all(ACCEPT).iter() {
576590
let accept = Accept::from_str(header.to_str()?)
577591
.map_err(|e| eyre::eyre!("invalid accept header: {e}"))?;
578592
for mt in accept.media_types() {
579-
match mt.essence().to_string().as_str() {
580-
APPLICATION_OCTET_STREAM => {
581-
accepted_types.insert(EncodingType::Ssz);
582-
}
583-
APPLICATION_JSON | WILDCARD => {
584-
accepted_types.insert(EncodingType::Json);
585-
}
586-
_ => unsupported_type = true,
593+
saw_any = true;
594+
595+
// Skip q=0 entries — RFC 7231 §5.3.1: "A request without any Accept
596+
// header field implies that the user agent will accept any media
597+
// type in response. When a header field is present ... a value of
598+
// 0 means 'not acceptable'."
599+
if let Some(q) = mt.get_param(mediatype::names::Q) &&
600+
q.as_str().parse::<f32>().is_ok_and(|v| v <= 0.0)
601+
{
602+
continue;
603+
}
604+
605+
let parsed = match mt.essence().to_string().as_str() {
606+
APPLICATION_OCTET_STREAM => Some(EncodingType::Ssz),
607+
APPLICATION_JSON | WILDCARD => Some(EncodingType::Json),
608+
_ => None,
587609
};
610+
if let Some(enc) = parsed {
611+
had_supported = true;
612+
if !ordered.contains(&enc) {
613+
ordered.push(enc);
614+
}
615+
}
588616
}
589617
}
590618

591-
if accepted_types.is_empty() {
592-
if unsupported_type {
619+
if ordered.is_empty() {
620+
if saw_any && !had_supported {
593621
return Err(eyre::eyre!("unsupported accept type"));
594622
}
595623

596-
// No accept header so just return the same type as the content type
597-
accepted_types.insert(get_content_type(req_headers));
624+
// No accept header (or only q=0 rejections): fall back to the request
625+
// Content-Type, which mirrors the historical behavior.
626+
ordered.push(get_content_type(req_headers));
598627
}
599-
Ok(accepted_types)
628+
Ok(ordered)
629+
}
630+
631+
/// Pick the caller's highest-preference encoding from a list of types the
632+
/// server can actually produce. `accepts` is expected to be pre-ordered by
633+
/// descending preference (as returned by [`get_accept_types`]). Returns
634+
/// `None` if no overlap exists.
635+
pub fn preferred_encoding(
636+
accepts: &[EncodingType],
637+
supported: &[EncodingType],
638+
) -> Option<EncodingType> {
639+
accepts.iter().copied().find(|a| supported.contains(a))
640+
}
641+
642+
/// Compute the q-value for the `index`-th preferred encoding when building an
643+
/// outbound `Accept` header. The first entry gets q=1.0, each subsequent entry
644+
/// decreases by 0.1, and the value is clamped to a minimum of 0.1 so we never
645+
/// emit q=0 (which per RFC 7231 §5.3.1 means "not acceptable").
646+
fn accept_q_value_for_index(index: usize) -> f32 {
647+
// `as i32` would silently wrap for large indices (e.g. usize::MAX → -1),
648+
// which would invert the clamp. Saturate the cast explicitly.
649+
let idx = i32::try_from(index).unwrap_or(i32::MAX);
650+
let step = 10_i32.saturating_sub(idx).max(1);
651+
step as f32 / 10.0
652+
}
653+
654+
/// Format a single `Accept` header entry as `"<media-type>;q=<x.x>"`.
655+
fn format_accept_entry(enc: EncodingType, q: f32) -> String {
656+
format!("{};q={:.1}", enc.content_type(), q)
657+
}
658+
659+
/// Build an `Accept` header string that mirrors the caller's preference order
660+
/// so the relay sees the same priority the beacon node asked us for. Each
661+
/// subsequent entry receives a q-value 0.1 lower than the previous one,
662+
/// starting at 1.0. Returns an empty string for an empty preference list.
663+
pub fn build_outbound_accept(preferred: &[EncodingType]) -> String {
664+
preferred
665+
.iter()
666+
.enumerate()
667+
.map(|(i, enc)| format_accept_entry(*enc, accept_q_value_for_index(i)))
668+
.collect::<Vec<_>>()
669+
.join(",")
600670
}
601671

602672
/// Parse CONTENT TYPE header to get the encoding type of the body, defaulting
@@ -919,10 +989,11 @@ mod test {
919989
use reqwest::header::{ACCEPT, CONTENT_TYPE};
920990

921991
use super::{
922-
BodyDeserializeError, CONSENSUS_VERSION_HEADER, create_admin_jwt, create_jwt,
923-
decode_admin_jwt, decode_jwt, deserialize_body, get_consensus_version_header,
924-
get_content_type, parse_response_encoding_and_fork, random_jwt_secret, validate_admin_jwt,
925-
validate_jwt,
992+
BodyDeserializeError, CONSENSUS_VERSION_HEADER, OUTBOUND_ACCEPT, accept_q_value_for_index,
993+
build_outbound_accept, create_admin_jwt, create_jwt, decode_admin_jwt, decode_jwt,
994+
deserialize_body, format_accept_entry, get_consensus_version_header, get_content_type,
995+
parse_response_encoding_and_fork, preferred_encoding, random_jwt_secret,
996+
validate_admin_jwt, validate_jwt,
926997
};
927998
use crate::{
928999
constants::SIGNER_JWT_EXPIRATION,
@@ -967,8 +1038,7 @@ mod test {
9671038
fn test_missing_accept_header() {
9681039
let headers = HeaderMap::new();
9691040
let result = get_accept_types(&headers).unwrap();
970-
assert_eq!(result.len(), 1);
971-
assert!(result.contains(&EncodingType::Json));
1041+
assert_eq!(result, vec![EncodingType::Json]);
9721042
}
9731043

9741044
/// Test accepting JSON
@@ -977,8 +1047,7 @@ mod test {
9771047
let mut headers = HeaderMap::new();
9781048
headers.append(ACCEPT, HeaderValue::from_str(APPLICATION_JSON).unwrap());
9791049
let result = get_accept_types(&headers).unwrap();
980-
assert_eq!(result.len(), 1);
981-
assert!(result.contains(&EncodingType::Json));
1050+
assert_eq!(result, vec![EncodingType::Json]);
9821051
}
9831052

9841053
/// Test accepting SSZ
@@ -987,8 +1056,7 @@ mod test {
9871056
let mut headers = HeaderMap::new();
9881057
headers.append(ACCEPT, HeaderValue::from_str(APPLICATION_OCTET_STREAM).unwrap());
9891058
let result = get_accept_types(&headers).unwrap();
990-
assert_eq!(result.len(), 1);
991-
assert!(result.contains(&EncodingType::Ssz));
1059+
assert_eq!(result, vec![EncodingType::Ssz]);
9921060
}
9931061

9941062
/// Test accepting wildcards
@@ -997,20 +1065,18 @@ mod test {
9971065
let mut headers = HeaderMap::new();
9981066
headers.append(ACCEPT, HeaderValue::from_str(WILDCARD).unwrap());
9991067
let result = get_accept_types(&headers).unwrap();
1000-
assert_eq!(result.len(), 1);
1001-
assert!(result.contains(&EncodingType::Json));
1068+
assert_eq!(result, vec![EncodingType::Json]);
10021069
}
10031070

1004-
/// Test accepting one header with multiple values
1071+
/// Test accepting one header with multiple values (order preserved,
1072+
/// first listed wins at equal q)
10051073
#[test]
10061074
fn test_accept_header_multiple_values() {
10071075
let header_string = format!("{APPLICATION_JSON}, {APPLICATION_OCTET_STREAM}");
10081076
let mut headers = HeaderMap::new();
10091077
headers.append(ACCEPT, HeaderValue::from_str(&header_string).unwrap());
10101078
let result = get_accept_types(&headers).unwrap();
1011-
assert_eq!(result.len(), 2);
1012-
assert!(result.contains(&EncodingType::Json));
1013-
assert!(result.contains(&EncodingType::Ssz));
1079+
assert_eq!(result, vec![EncodingType::Json, EncodingType::Ssz]);
10141080
}
10151081

10161082
/// Test accepting multiple headers
@@ -1020,9 +1086,9 @@ mod test {
10201086
headers.append(ACCEPT, HeaderValue::from_str(APPLICATION_JSON).unwrap());
10211087
headers.append(ACCEPT, HeaderValue::from_str(APPLICATION_OCTET_STREAM).unwrap());
10221088
let result = get_accept_types(&headers).unwrap();
1023-
assert_eq!(result.len(), 2);
10241089
assert!(result.contains(&EncodingType::Json));
10251090
assert!(result.contains(&EncodingType::Ssz));
1091+
assert_eq!(result.len(), 2);
10261092
}
10271093

10281094
/// Test accepting one header with multiple values, including a type that
@@ -1034,9 +1100,7 @@ mod test {
10341100
let mut headers = HeaderMap::new();
10351101
headers.append(ACCEPT, HeaderValue::from_str(&header_string).unwrap());
10361102
let result = get_accept_types(&headers).unwrap();
1037-
assert_eq!(result.len(), 2);
1038-
assert!(result.contains(&EncodingType::Json));
1039-
assert!(result.contains(&EncodingType::Ssz));
1103+
assert_eq!(result, vec![EncodingType::Json, EncodingType::Ssz]);
10401104
}
10411105

10421106
/// Test rejecting an unknown accept type
@@ -1058,6 +1122,123 @@ mod test {
10581122
assert!(result.is_err());
10591123
}
10601124

1125+
/// q-values are honored: JSON@1.0 should outrank SSZ@0.1 regardless of
1126+
/// byte order in the header.
1127+
#[test]
1128+
fn test_accept_header_q_value_ordering() {
1129+
let mut headers = HeaderMap::new();
1130+
headers.append(
1131+
ACCEPT,
1132+
HeaderValue::from_str("application/json;q=1.0, application/octet-stream;q=0.1")
1133+
.unwrap(),
1134+
);
1135+
assert_eq!(get_accept_types(&headers).unwrap(), vec![
1136+
EncodingType::Json,
1137+
EncodingType::Ssz
1138+
]);
1139+
1140+
let mut headers = HeaderMap::new();
1141+
headers.append(
1142+
ACCEPT,
1143+
HeaderValue::from_str("application/octet-stream;q=0.1, application/json;q=1.0")
1144+
.unwrap(),
1145+
);
1146+
assert_eq!(get_accept_types(&headers).unwrap(), vec![
1147+
EncodingType::Json,
1148+
EncodingType::Ssz
1149+
]);
1150+
}
1151+
1152+
/// q=0 is an explicit rejection per RFC 7231 §5.3.1 and must be dropped.
1153+
#[test]
1154+
fn test_accept_header_q_zero_rejected() {
1155+
let mut headers = HeaderMap::new();
1156+
headers.append(
1157+
ACCEPT,
1158+
HeaderValue::from_str("application/json, application/octet-stream;q=0").unwrap(),
1159+
);
1160+
assert_eq!(get_accept_types(&headers).unwrap(), vec![EncodingType::Json]);
1161+
}
1162+
1163+
/// An Accept header containing only q=0 for every supported type is a
1164+
/// deliberate "I accept nothing" and must error (so the route can return
1165+
/// 406 Not Acceptable per RFC 7231 §5.3.1 and §6.5.6).
1166+
#[test]
1167+
fn test_accept_header_only_q_zero_errors() {
1168+
let mut headers = HeaderMap::new();
1169+
headers.append(
1170+
ACCEPT,
1171+
HeaderValue::from_str("application/json;q=0, application/octet-stream;q=0").unwrap(),
1172+
);
1173+
assert!(get_accept_types(&headers).is_err());
1174+
}
1175+
1176+
/// `preferred_encoding` picks the caller's first choice that the server
1177+
/// can actually produce.
1178+
#[test]
1179+
fn test_preferred_encoding_picks_highest_q_match() {
1180+
let accepts = [EncodingType::Json, EncodingType::Ssz];
1181+
let supported = [EncodingType::Ssz, EncodingType::Json];
1182+
assert_eq!(preferred_encoding(&accepts, &supported), Some(EncodingType::Json));
1183+
1184+
let accepts = [EncodingType::Ssz];
1185+
let supported = [EncodingType::Json];
1186+
assert_eq!(preferred_encoding(&accepts, &supported), None);
1187+
}
1188+
1189+
/// Outbound Accept should be deterministic and q-ordered to match caller
1190+
/// preference.
1191+
#[test]
1192+
fn test_build_outbound_accept_deterministic() {
1193+
let s = build_outbound_accept(&[EncodingType::Ssz, EncodingType::Json]);
1194+
assert_eq!(s, "application/octet-stream;q=1.0,application/json;q=0.9");
1195+
1196+
let s = build_outbound_accept(&[EncodingType::Json, EncodingType::Ssz]);
1197+
assert_eq!(s, "application/json;q=1.0,application/octet-stream;q=0.9");
1198+
1199+
// Stable across repeats
1200+
for _ in 0..100 {
1201+
assert_eq!(
1202+
build_outbound_accept(&[EncodingType::Ssz, EncodingType::Json]),
1203+
"application/octet-stream;q=1.0,application/json;q=0.9"
1204+
);
1205+
}
1206+
}
1207+
/// Snapshot test: constant emits exactly what we document in
1208+
/// OUTBOUND_ACCEPT.
1209+
#[test]
1210+
fn test_outbound_accept_constant_snapshot() {
1211+
assert_eq!(OUTBOUND_ACCEPT, "application/octet-stream;q=1.0,application/json;q=0.9");
1212+
}
1213+
1214+
/// q-value ladder: first entry is 1.0, each subsequent entry drops by 0.1.
1215+
#[test]
1216+
fn test_accept_q_value_for_index_ladder() {
1217+
assert!((accept_q_value_for_index(0) - 1.0).abs() < f32::EPSILON);
1218+
assert!((accept_q_value_for_index(1) - 0.9).abs() < f32::EPSILON);
1219+
assert!((accept_q_value_for_index(5) - 0.5).abs() < f32::EPSILON);
1220+
assert!((accept_q_value_for_index(9) - 0.1).abs() < f32::EPSILON);
1221+
}
1222+
1223+
/// Clamp at 0.1: we never emit q=0 (which per RFC 7231 §5.3.1 would mean
1224+
/// "not acceptable").
1225+
#[test]
1226+
fn test_accept_q_value_for_index_clamps_to_minimum() {
1227+
assert!((accept_q_value_for_index(10) - 0.1).abs() < f32::EPSILON);
1228+
assert!((accept_q_value_for_index(100) - 0.1).abs() < f32::EPSILON);
1229+
// Even an adversarial usize::MAX must not underflow or drop to zero.
1230+
assert!((accept_q_value_for_index(usize::MAX) - 0.1).abs() < f32::EPSILON);
1231+
}
1232+
1233+
/// Entry formatter emits the spec-shaped string.
1234+
#[test]
1235+
fn test_format_accept_entry_shape() {
1236+
assert_eq!(format_accept_entry(EncodingType::Ssz, 1.0), "application/octet-stream;q=1.0");
1237+
assert_eq!(format_accept_entry(EncodingType::Json, 0.9), "application/json;q=0.9");
1238+
// One decimal place, even when the value has more precision.
1239+
assert_eq!(format_accept_entry(EncodingType::Json, 0.12345), "application/json;q=0.1");
1240+
}
1241+
10611242
#[test]
10621243
fn test_jwt_validation_with_payload() {
10631244
// Pretend payload

0 commit comments

Comments
 (0)