Skip to content

Commit 8e7bd2a

Browse files
committed
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. 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 8e7bd2a

1 file changed

Lines changed: 100 additions & 2 deletions

File tree

payjoin-mailroom/src/ohttp_relay/gateway_prober.rs

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

248+
fn retry_after_or<B>(res: &hyper::Response<B>, default: Duration) -> Duration {
249+
res.headers()
250+
.get(hyper::header::RETRY_AFTER)
251+
.and_then(|v| v.to_str().ok())
252+
.and_then(|s| s.parse::<u64>().ok())
253+
.map(Duration::from_secs)
254+
.map(|d| d.min(MAX_RETRY_AFTER_TTL))
255+
.unwrap_or(default)
256+
}
257+
248258
/// Probes a target gateway by attempting to send a GET request.
249259
async fn probe(&self, base_url: &GatewayUri) -> Policy {
250260
// Create a GET request without a body
@@ -281,11 +291,13 @@ impl Prober {
281291
}
282292
} else if status == hyper::StatusCode::GATEWAY_TIMEOUT {
283293
ttls.http_504_gateway_timeout
294+
} else if status == hyper::StatusCode::TOO_MANY_REQUESTS {
295+
Self::retry_after_or(res, ttls.http_4xx)
284296
} else if status.is_client_error() {
285-
// TODO handle Retry-After for 429 too many requests
286297
ttls.http_4xx
298+
} else if status == hyper::StatusCode::SERVICE_UNAVAILABLE {
299+
Self::retry_after_or(res, ttls.http_5xx)
287300
} else if status.is_server_error() {
288-
// TODO handle Retry-After for 503 service unavailable
289301
ttls.http_5xx
290302
} else {
291303
ttls.default
@@ -368,6 +380,8 @@ struct TTLConfig {
368380

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

0 commit comments

Comments
 (0)