Skip to content

Commit f731428

Browse files
codaMWcaarloshenriq
andcommitted
Handle Retry-After header for 429 and 503 in gateway prober
When a gateway returns 429 Too Many Requests or 503 Service Unavailable, the Retry-After response header specifies how long to wait before retrying. Previously these responses fell back to static TTL defaults, ignoring the server-specified duration. Add a retry_after_or helper that parses the Retry-After header value as seconds and uses it as the cache TTL, capped at MAX_RETRY_AFTER_TTL (24 hours) to prevent attacker-controlled delays. Falls back to the configured default when absent. Only the seconds form of Retry-After is handled per RFC 7231 section 7.1.3. The HTTP-date form silently falls back to default. Add tests covering 429 with header, 503 with header, and 429 without header fallback to default TTL. Closes #1474 Co-authored-by: caarloshenriq <117552173+caarloshenriq@users.noreply.github.com>
1 parent 45d286f commit f731428

1 file changed

Lines changed: 102 additions & 2 deletions

File tree

payjoin-mailroom/src/ohttp_relay/gateway_prober.rs

Lines changed: 102 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,18 @@ impl Prober {
245245
None
246246
}
247247

248+
fn retry_after_or<B>(res: &hyper::Response<B>, default: Duration) -> Duration {
249+
// Only the seconds form of Retry-After is handled (RFC 7231 §7.1.3).
250+
// The HTTP-date form is not parsed and silently falls back to `default`.
251+
res.headers()
252+
.get(hyper::header::RETRY_AFTER)
253+
.and_then(|v| v.to_str().ok())
254+
.and_then(|s| s.parse::<u64>().ok())
255+
.map(Duration::from_secs)
256+
.map(|d| d.min(MAX_RETRY_AFTER_TTL))
257+
.unwrap_or(default)
258+
}
259+
248260
/// Probes a target gateway by attempting to send a GET request.
249261
async fn probe(&self, base_url: &GatewayUri) -> Policy {
250262
// Create a GET request without a body
@@ -281,11 +293,13 @@ impl Prober {
281293
}
282294
} else if status == hyper::StatusCode::GATEWAY_TIMEOUT {
283295
ttls.http_504_gateway_timeout
296+
} else if status == hyper::StatusCode::TOO_MANY_REQUESTS {
297+
Self::retry_after_or(res, ttls.http_4xx)
284298
} else if status.is_client_error() {
285-
// TODO handle Retry-After for 429 too many requests
286299
ttls.http_4xx
300+
} else if status == hyper::StatusCode::SERVICE_UNAVAILABLE {
301+
Self::retry_after_or(res, ttls.http_5xx)
287302
} else if status.is_server_error() {
288-
// TODO handle Retry-After for 503 service unavailable
289303
ttls.http_5xx
290304
} else {
291305
ttls.default
@@ -368,6 +382,8 @@ struct TTLConfig {
368382

369383
/// Different probing results/conditions and the time to live when caching that
370384
/// information.
385+
/// Maximum TTL allowed from a server-supplied Retry-After header.
386+
const MAX_RETRY_AFTER_TTL: Duration = Duration::from_secs(24 * 60 * 60);
371387
impl Default for TTLConfig {
372388
fn default() -> Self {
373389
/// A week
@@ -764,4 +780,88 @@ mod tests {
764780
"the element should be the bip77 opt-in magic string"
765781
);
766782
}
783+
784+
#[tokio::test(start_paused = true)]
785+
async fn test_mock_429_retry_after() {
786+
let mut server = Server::new_async().await;
787+
let url =
788+
GatewayUri::from_str(&server.url()).expect("must be able to parse mock server URL");
789+
let prober = Prober::default();
790+
791+
let retry_after_secs = 120u64;
792+
let mock_429 = server
793+
.mock("GET", RFC_9540_GATEWAY_PATH)
794+
.match_query(mockito::Matcher::Regex("^allowed_purposes$".into()))
795+
.with_status(429)
796+
.with_header("Retry-After", &retry_after_secs.to_string())
797+
.create_async()
798+
.await;
799+
800+
let status = prober.check_opt_in(&url).await.expect("probing must succeed");
801+
mock_429.assert_async().await;
802+
assert!(!status.bip77_allowed, "rate-limited gateway should not be considered opt-in");
803+
804+
// Advance time by less than Retry-After — should use cached result (no new request)
805+
tokio::time::advance(Duration::from_secs(retry_after_secs - 1)).await;
806+
let status = prober.check_opt_in(&url).await.expect("cached probe must succeed");
807+
assert!(!status.bip77_allowed, "result should still be cached before Retry-After expires");
808+
809+
// Advance past Retry-After — cache should be expired, new request would be sent
810+
tokio::time::advance(Duration::from_secs(2)).await;
811+
let mock_429_again = server
812+
.mock("GET", RFC_9540_GATEWAY_PATH)
813+
.match_query(mockito::Matcher::Regex("^allowed_purposes$".into()))
814+
.with_status(429)
815+
.with_header("Retry-After", &retry_after_secs.to_string())
816+
.create_async()
817+
.await;
818+
let _ = prober.check_opt_in(&url).await;
819+
mock_429_again.assert_async().await;
820+
}
821+
822+
#[tokio::test]
823+
async fn test_mock_503_retry_after() {
824+
let mut server = Server::new_async().await;
825+
let url =
826+
GatewayUri::from_str(&server.url()).expect("must be able to parse mock server URL");
827+
let prober = Prober::default();
828+
829+
let mock_503 = server
830+
.mock("GET", RFC_9540_GATEWAY_PATH)
831+
.match_query(mockito::Matcher::Regex("^allowed_purposes$".into()))
832+
.with_status(503)
833+
.with_header("Retry-After", "30")
834+
.create_async()
835+
.await;
836+
837+
let status = prober.check_opt_in(&url).await.expect("probing must succeed");
838+
mock_503.assert_async().await;
839+
assert!(!status.bip77_allowed, "unavailable gateway should not be considered opt-in");
840+
}
841+
842+
#[tokio::test]
843+
async fn test_mock_429_no_retry_after_uses_default() {
844+
let mut server = Server::new_async().await;
845+
let url =
846+
GatewayUri::from_str(&server.url()).expect("must be able to parse mock server URL");
847+
let prober = Prober::default();
848+
849+
let mock_429 = server
850+
.mock("GET", RFC_9540_GATEWAY_PATH)
851+
.match_query(mockito::Matcher::Regex("^allowed_purposes$".into()))
852+
.with_status(429)
853+
.create_async()
854+
.await;
855+
856+
let status = prober.check_opt_in(&url).await.expect("probing must succeed");
857+
mock_429.assert_async().await;
858+
assert!(
859+
!status.bip77_allowed,
860+
"rate-limited gateway without Retry-After should not be opt-in"
861+
);
862+
863+
// Should be cached — no new request sent
864+
let status = prober.check_opt_in(&url).await.expect("cached probe must succeed");
865+
assert!(!status.bip77_allowed, "result should be cached with default TTL");
866+
}
767867
}

0 commit comments

Comments
 (0)