From 814fcbffb362879502ac9b4e0f0a036a15efdf15 Mon Sep 17 00:00:00 2001 From: Ophir LOJKINE Date: Sun, 8 Mar 2026 02:52:47 +0100 Subject: [PATCH 1/8] add timeout to OIDC HTTP response body read A stalled OIDC provider that sends HTTP headers but never completes the body would cause response.body().await to hang forever, freezing the entire SQLPage process. Add a 5-second timeout on the body stream read using awc's ClientResponse::timeout(). Fixes #1231 Co-Authored-By: Claude Opus 4.6 --- Cargo.toml | 1 + src/webserver/oidc.rs | 2 ++ tests/oidc/mod.rs | 82 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index cef0ff45..47554aa8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -89,6 +89,7 @@ lambda-web = ["dep:lambda-web", "odbc-static"] [dev-dependencies] actix-http = "3" +tokio = { version = "1", features = ["macros", "rt", "time", "test-util"] } [build-dependencies] awc = { version = "3", features = ["rustls-0_23-webpki-roots"] } diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index ea429acc..b681720d 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -48,6 +48,7 @@ const SQLPAGE_NONCE_COOKIE_NAME: &str = "sqlpage_oidc_nonce"; const SQLPAGE_TMP_LOGIN_STATE_COOKIE_PREFIX: &str = "sqlpage_oidc_state_"; const OIDC_CLIENT_MAX_REFRESH_INTERVAL: Duration = Duration::from_secs(60 * 60); const OIDC_CLIENT_MIN_REFRESH_INTERVAL: Duration = Duration::from_secs(5); +const OIDC_HTTP_BODY_TIMEOUT: Duration = OIDC_CLIENT_MIN_REFRESH_INTERVAL; const SQLPAGE_OIDC_REDIRECT_COUNT_COOKIE: &str = "sqlpage_oidc_redirect_count"; const MAX_OIDC_REDIRECTS: u8 = 3; const AUTH_COOKIE_EXPIRATION: awc::cookie::time::Duration = @@ -849,6 +850,7 @@ async fn execute_oidc_request_with_awc( for (name, value) in head { resp_builder = resp_builder.header(name.as_str(), value.to_str()?); } + let mut response = response.timeout(OIDC_HTTP_BODY_TIMEOUT); let body = response .body() .await diff --git a/tests/oidc/mod.rs b/tests/oidc/mod.rs index 7cf128c1..e42ce396 100644 --- a/tests/oidc/mod.rs +++ b/tests/oidc/mod.rs @@ -12,6 +12,7 @@ use serde_json::json; use sqlpage::webserver::http::create_app; use std::collections::HashMap; use std::sync::{Arc, Mutex}; +use tokio::sync::Notify; use tokio_util::sync::{CancellationToken, DropGuard}; fn base64url_encode(data: &[u8]) -> String { @@ -50,6 +51,7 @@ struct ProviderState<'a> { client_id: String, auth_codes: HashMap, // code -> nonce jwt_customizer: Option>>, + token_endpoint_gate: Option>, } type ProviderStateWithLifetime<'a> = ProviderState<'a>; @@ -117,6 +119,19 @@ async fn token_endpoint( state: Data, req: web::Form>, ) -> impl Responder { + let gate = state.lock().unwrap().token_endpoint_gate.clone(); + if let Some(gate) = gate { + // Send HTTP response headers immediately, but stall the body. + // This reproduces the exact bug: send_body().await succeeds (headers arrived), + // but response.body().await hangs forever with no timeout. + let body = futures_util::stream::once(async move { + gate.notified().await; + Ok::(web::Bytes::new()) + }); + return HttpResponse::Ok() + .insert_header((header::CONTENT_TYPE, "application/json")) + .streaming(body); + } let mut state = state.lock().unwrap(); let Some(code) = req.get("code") else { return HttpResponse::BadRequest().body("Missing code"); @@ -185,6 +200,7 @@ impl FakeOidcProvider { client_id: client_id.clone(), auth_codes: HashMap::new(), jwt_customizer: None, + token_endpoint_gate: None, })); let state_for_server = Arc::clone(&state); @@ -226,6 +242,12 @@ impl FakeOidcProvider { f(&mut state) } + pub fn gate_token_endpoint(&self) -> Arc { + let gate = Arc::new(Notify::new()); + self.with_state_mut(|s| s.token_endpoint_gate = Some(gate.clone())); + gate + } + pub fn store_auth_code(&self, code: String, nonce: String) { self.with_state_mut(|s| { s.auth_codes.insert(code, nonce); @@ -540,3 +562,63 @@ async fn test_oidc_logout_uses_correct_scheme() { let post_logout = params.get("post_logout_redirect_uri").unwrap(); assert_eq!(post_logout, "https://example.com/logged_out"); } + +/// Regression test: a stalled OIDC provider must not freeze the server. +/// See https://github.com/sqlpage/SQLPage/issues/1231 +#[actix_web::test] +async fn test_stalled_token_endpoint_does_not_freeze_server() { + let (app, provider) = setup_oidc_test(|_| {}).await; + let mut cookies: Vec> = Vec::new(); + + // Step 1: initiate login — get redirected to auth provider + let resp = request_with_cookies!(app, test::TestRequest::get().uri("/"), cookies); + assert_eq!(resp.status(), StatusCode::SEE_OTHER); + let auth_url = Url::parse(resp.headers().get("location").unwrap().to_str().unwrap()).unwrap(); + let state_param = get_query_param(&auth_url, "state"); + let nonce = get_query_param(&auth_url, "nonce"); + let redirect_uri = get_query_param(&auth_url, "redirect_uri"); + provider.store_auth_code("test_auth_code".to_string(), nonce); + + // Step 2: gate the token endpoint so it sends headers but stalls the body + let _gate = provider.gate_token_endpoint(); + + // Step 3: hit the OIDC callback — the server will try to exchange the auth + // code for a token. The token endpoint sends headers but the body never + // arrives. With the bug, response.body().await hangs forever (no timeout). + let callback_uri = format!( + "{}?code=test_auth_code&state={}", + Url::parse(&redirect_uri).unwrap().path(), + state_param + ); + + // Detect the hang without wall-clock delays using a two-phase approach: + // Phase 1 (yields): let the localhost TCP round trip complete so the awc + // client receives HTTP headers and enters body().await. + // Phase 2 (pause + sleep): freeze tokio time then sleep past the body + // timeout. With auto-advance this resolves instantly. If there IS a body + // timeout (fix applied), auto-advance fires it and the request completes. + // If there is NO body timeout (bug), nothing wakes the body read, and + // the sleep branch completes first → panic. + tokio::select! { + _resp = async { + let mut req = test::TestRequest::get().uri(&callback_uri); + for cookie in cookies.iter() { + req = req.cookie(cookie.clone()); + } + test::call_service(&app, req.to_request()).await + } => {} // request completed — bug is fixed + _ = async { + for _ in 0..1000 { + tokio::task::yield_now().await; + } + tokio::time::pause(); + tokio::time::sleep(std::time::Duration::from_secs(60)).await; + } => { + panic!( + "OIDC callback request hung — the server froze because \ + response.body().await has no timeout when the OIDC provider \ + stalls after sending headers" + ); + } + } +} From f028f71be0b7e3c099b2471744e341c752a43983 Mon Sep 17 00:00:00 2001 From: Ophir LOJKINE Date: Sun, 8 Mar 2026 02:56:31 +0100 Subject: [PATCH 2/8] simplify test comment --- tests/oidc/mod.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/oidc/mod.rs b/tests/oidc/mod.rs index e42ce396..03b71b41 100644 --- a/tests/oidc/mod.rs +++ b/tests/oidc/mod.rs @@ -121,9 +121,8 @@ async fn token_endpoint( ) -> impl Responder { let gate = state.lock().unwrap().token_endpoint_gate.clone(); if let Some(gate) = gate { - // Send HTTP response headers immediately, but stall the body. - // This reproduces the exact bug: send_body().await succeeds (headers arrived), - // but response.body().await hangs forever with no timeout. + // Simulate a provider that stalls mid-response: send headers + // immediately but never complete the body. let body = futures_util::stream::once(async move { gate.notified().await; Ok::(web::Bytes::new()) From 8dec037bd368588f6514d4146920445d8133bc5d Mon Sep 17 00:00:00 2001 From: Ophir LOJKINE Date: Sun, 8 Mar 2026 03:07:11 +0100 Subject: [PATCH 3/8] simplify test: use token_endpoint_delay instead of Notify gate --- tests/oidc/mod.rs | 67 +++++++++++++++++------------------------------ 1 file changed, 24 insertions(+), 43 deletions(-) diff --git a/tests/oidc/mod.rs b/tests/oidc/mod.rs index 03b71b41..966e7ff5 100644 --- a/tests/oidc/mod.rs +++ b/tests/oidc/mod.rs @@ -12,7 +12,7 @@ use serde_json::json; use sqlpage::webserver::http::create_app; use std::collections::HashMap; use std::sync::{Arc, Mutex}; -use tokio::sync::Notify; +use std::time::Duration; use tokio_util::sync::{CancellationToken, DropGuard}; fn base64url_encode(data: &[u8]) -> String { @@ -51,7 +51,7 @@ struct ProviderState<'a> { client_id: String, auth_codes: HashMap, // code -> nonce jwt_customizer: Option>>, - token_endpoint_gate: Option>, + token_endpoint_delay: Duration, } type ProviderStateWithLifetime<'a> = ProviderState<'a>; @@ -119,18 +119,6 @@ async fn token_endpoint( state: Data, req: web::Form>, ) -> impl Responder { - let gate = state.lock().unwrap().token_endpoint_gate.clone(); - if let Some(gate) = gate { - // Simulate a provider that stalls mid-response: send headers - // immediately but never complete the body. - let body = futures_util::stream::once(async move { - gate.notified().await; - Ok::(web::Bytes::new()) - }); - return HttpResponse::Ok() - .insert_header((header::CONTENT_TYPE, "application/json")) - .streaming(body); - } let mut state = state.lock().unwrap(); let Some(code) = req.get("code") else { return HttpResponse::BadRequest().body("Missing code"); @@ -156,6 +144,9 @@ async fn token_endpoint( .map(|customizer| customizer(claims.clone(), &state.secret)) .unwrap_or_else(|| make_jwt(&claims, &state.secret)); + let delay = state.token_endpoint_delay; + drop(state); + let response = TokenResponse { access_token: "test_access_token".to_string(), token_type: "Bearer".to_string(), @@ -163,9 +154,14 @@ async fn token_endpoint( expires_in: 3600, }; + let json_bytes = serde_json::to_vec(&response).unwrap(); + let body = futures_util::stream::once(async move { + tokio::time::sleep(delay).await; + Ok::(web::Bytes::from(json_bytes)) + }); HttpResponse::Ok() .insert_header((header::CONTENT_TYPE, "application/json")) - .json(response) + .streaming(body) } pub struct FakeOidcProvider { @@ -199,7 +195,7 @@ impl FakeOidcProvider { client_id: client_id.clone(), auth_codes: HashMap::new(), jwt_customizer: None, - token_endpoint_gate: None, + token_endpoint_delay: Duration::ZERO, })); let state_for_server = Arc::clone(&state); @@ -241,10 +237,8 @@ impl FakeOidcProvider { f(&mut state) } - pub fn gate_token_endpoint(&self) -> Arc { - let gate = Arc::new(Notify::new()); - self.with_state_mut(|s| s.token_endpoint_gate = Some(gate.clone())); - gate + pub fn set_token_endpoint_delay(&self, delay: Duration) { + self.with_state_mut(|s| s.token_endpoint_delay = delay); } pub fn store_auth_code(&self, code: String, nonce: String) { @@ -562,14 +556,13 @@ async fn test_oidc_logout_uses_correct_scheme() { assert_eq!(post_logout, "https://example.com/logged_out"); } -/// Regression test: a stalled OIDC provider must not freeze the server. +/// A slow OIDC provider must not freeze the server. /// See https://github.com/sqlpage/SQLPage/issues/1231 #[actix_web::test] -async fn test_stalled_token_endpoint_does_not_freeze_server() { +async fn test_slow_token_endpoint_does_not_freeze_server() { let (app, provider) = setup_oidc_test(|_| {}).await; let mut cookies: Vec> = Vec::new(); - // Step 1: initiate login — get redirected to auth provider let resp = request_with_cookies!(app, test::TestRequest::get().uri("/"), cookies); assert_eq!(resp.status(), StatusCode::SEE_OTHER); let auth_url = Url::parse(resp.headers().get("location").unwrap().to_str().unwrap()).unwrap(); @@ -578,26 +571,18 @@ async fn test_stalled_token_endpoint_does_not_freeze_server() { let redirect_uri = get_query_param(&auth_url, "redirect_uri"); provider.store_auth_code("test_auth_code".to_string(), nonce); - // Step 2: gate the token endpoint so it sends headers but stalls the body - let _gate = provider.gate_token_endpoint(); + provider.set_token_endpoint_delay(Duration::from_secs(999)); - // Step 3: hit the OIDC callback — the server will try to exchange the auth - // code for a token. The token endpoint sends headers but the body never - // arrives. With the bug, response.body().await hangs forever (no timeout). let callback_uri = format!( "{}?code=test_auth_code&state={}", Url::parse(&redirect_uri).unwrap().path(), state_param ); - // Detect the hang without wall-clock delays using a two-phase approach: - // Phase 1 (yields): let the localhost TCP round trip complete so the awc - // client receives HTTP headers and enters body().await. - // Phase 2 (pause + sleep): freeze tokio time then sleep past the body - // timeout. With auto-advance this resolves instantly. If there IS a body - // timeout (fix applied), auto-advance fires it and the request completes. - // If there is NO body timeout (bug), nothing wakes the body read, and - // the sleep branch completes first → panic. + // Race the callback request against a deadline to detect hangs without + // wall-clock delays. Phase 1 (yields) lets the localhost TCP round trip + // complete. Phase 2 (pause + sleep) uses tokio auto-advance to instantly + // skip past any body-read timeout that may be set on the HTTP client. tokio::select! { _resp = async { let mut req = test::TestRequest::get().uri(&callback_uri); @@ -605,19 +590,15 @@ async fn test_stalled_token_endpoint_does_not_freeze_server() { req = req.cookie(cookie.clone()); } test::call_service(&app, req.to_request()).await - } => {} // request completed — bug is fixed + } => {} _ = async { for _ in 0..1000 { tokio::task::yield_now().await; } tokio::time::pause(); - tokio::time::sleep(std::time::Duration::from_secs(60)).await; + tokio::time::sleep(Duration::from_secs(60)).await; } => { - panic!( - "OIDC callback request hung — the server froze because \ - response.body().await has no timeout when the OIDC provider \ - stalls after sending headers" - ); + panic!("OIDC callback hung on a slow token endpoint"); } } } From d38a8fc1c1fa8d05b0944316ceab0a7b9befb39d Mon Sep 17 00:00:00 2001 From: Ophir LOJKINE Date: Sun, 8 Mar 2026 03:10:21 +0100 Subject: [PATCH 4/8] simplify test: use tokio time pause + auto-advance instead of select --- Cargo.toml | 2 +- tests/oidc/mod.rs | 40 ++++++++++++++++++---------------------- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 47554aa8..4399ae85 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -89,7 +89,7 @@ lambda-web = ["dep:lambda-web", "odbc-static"] [dev-dependencies] actix-http = "3" -tokio = { version = "1", features = ["macros", "rt", "time", "test-util"] } +tokio = { version = "1", features = ["time", "test-util"] } [build-dependencies] awc = { version = "3", features = ["rustls-0_23-webpki-roots"] } diff --git a/tests/oidc/mod.rs b/tests/oidc/mod.rs index 966e7ff5..e9b321b4 100644 --- a/tests/oidc/mod.rs +++ b/tests/oidc/mod.rs @@ -573,32 +573,28 @@ async fn test_slow_token_endpoint_does_not_freeze_server() { provider.set_token_endpoint_delay(Duration::from_secs(999)); + // Pause tokio time so all sleeps and timeouts auto-advance instantly. + tokio::time::pause(); + let callback_uri = format!( "{}?code=test_auth_code&state={}", Url::parse(&redirect_uri).unwrap().path(), state_param ); - // Race the callback request against a deadline to detect hangs without - // wall-clock delays. Phase 1 (yields) lets the localhost TCP round trip - // complete. Phase 2 (pause + sleep) uses tokio auto-advance to instantly - // skip past any body-read timeout that may be set on the HTTP client. - tokio::select! { - _resp = async { - let mut req = test::TestRequest::get().uri(&callback_uri); - for cookie in cookies.iter() { - req = req.cookie(cookie.clone()); - } - test::call_service(&app, req.to_request()).await - } => {} - _ = async { - for _ in 0..1000 { - tokio::task::yield_now().await; - } - tokio::time::pause(); - tokio::time::sleep(Duration::from_secs(60)).await; - } => { - panic!("OIDC callback hung on a slow token endpoint"); - } - } + // The callback exchanges the auth code for a token via the slow endpoint. + // All tokio timers auto-advance, so this returns immediately. With a body + // read timeout the request fails in ~5s of virtual time; without one the + // full 999s endpoint delay must elapse first. + let start = tokio::time::Instant::now(); + let _resp = request_with_cookies!( + app, + test::TestRequest::get().uri(&callback_uri), + cookies + ); + let elapsed = start.elapsed(); + assert!( + elapsed < Duration::from_secs(60), + "token exchange should time out quickly, took {elapsed:?} of virtual time" + ); } From abc4ad51df5dd82a5c8642dd3376a5dc23c52cad Mon Sep 17 00:00:00 2001 From: Ophir LOJKINE Date: Sun, 8 Mar 2026 03:15:40 +0100 Subject: [PATCH 5/8] use spawn_local + advance instead of select to detect hang --- Cargo.toml | 2 +- tests/oidc/mod.rs | 39 +++++++++++++++++++++++---------------- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4399ae85..6d8f5e26 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -89,7 +89,7 @@ lambda-web = ["dep:lambda-web", "odbc-static"] [dev-dependencies] actix-http = "3" -tokio = { version = "1", features = ["time", "test-util"] } +tokio = { version = "1", features = ["rt", "time", "test-util"] } [build-dependencies] awc = { version = "3", features = ["rustls-0_23-webpki-roots"] } diff --git a/tests/oidc/mod.rs b/tests/oidc/mod.rs index e9b321b4..e5e0deca 100644 --- a/tests/oidc/mod.rs +++ b/tests/oidc/mod.rs @@ -573,28 +573,35 @@ async fn test_slow_token_endpoint_does_not_freeze_server() { provider.set_token_endpoint_delay(Duration::from_secs(999)); - // Pause tokio time so all sleeps and timeouts auto-advance instantly. - tokio::time::pause(); - let callback_uri = format!( "{}?code=test_auth_code&state={}", Url::parse(&redirect_uri).unwrap().path(), state_param ); - // The callback exchanges the auth code for a token via the slow endpoint. - // All tokio timers auto-advance, so this returns immediately. With a body - // read timeout the request fails in ~5s of virtual time; without one the - // full 999s endpoint delay must elapse first. - let start = tokio::time::Instant::now(); - let _resp = request_with_cookies!( - app, - test::TestRequest::get().uri(&callback_uri), - cookies - ); - let elapsed = start.elapsed(); + // Spawn the callback request. It will exchange the auth code for a token, + // but the token endpoint body is delayed by 999s (headers arrive immediately). + let handle = tokio::task::spawn_local(async move { + let mut req = test::TestRequest::get().uri(&callback_uri); + for cookie in cookies.iter() { + req = req.cookie(cookie.clone()); + } + test::call_service(&app, req.to_request()).await + }); + + // Let the localhost TCP round trip complete in real time (microseconds). + for _ in 0..1000 { + tokio::task::yield_now().await; + } + + // Freeze time and advance past the body-read timeout. If one is set, the + // request completes. If not, only the 999s endpoint delay would wake it. + tokio::time::pause(); + tokio::time::advance(Duration::from_secs(60)).await; + tokio::task::yield_now().await; + assert!( - elapsed < Duration::from_secs(60), - "token exchange should time out quickly, took {elapsed:?} of virtual time" + handle.is_finished(), + "OIDC callback hung on a slow token endpoint" ); } From 81bbbd9b65f0434b78e90ab5228522bb6e07e766 Mon Sep 17 00:00:00 2001 From: Ophir LOJKINE Date: Sun, 8 Mar 2026 03:28:44 +0100 Subject: [PATCH 6/8] use Notify sync point instead of yield loop for deterministic test Co-Authored-By: Claude Opus 4.6 --- tests/oidc/mod.rs | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/tests/oidc/mod.rs b/tests/oidc/mod.rs index e5e0deca..a607f24f 100644 --- a/tests/oidc/mod.rs +++ b/tests/oidc/mod.rs @@ -13,6 +13,7 @@ use sqlpage::webserver::http::create_app; use std::collections::HashMap; use std::sync::{Arc, Mutex}; use std::time::Duration; +use tokio::sync::Notify; use tokio_util::sync::{CancellationToken, DropGuard}; fn base64url_encode(data: &[u8]) -> String { @@ -52,6 +53,7 @@ struct ProviderState<'a> { auth_codes: HashMap, // code -> nonce jwt_customizer: Option>>, token_endpoint_delay: Duration, + token_endpoint_started: Option>, } type ProviderStateWithLifetime<'a> = ProviderState<'a>; @@ -145,6 +147,7 @@ async fn token_endpoint( .unwrap_or_else(|| make_jwt(&claims, &state.secret)); let delay = state.token_endpoint_delay; + let started = state.token_endpoint_started.clone(); drop(state); let response = TokenResponse { @@ -156,6 +159,10 @@ async fn token_endpoint( let json_bytes = serde_json::to_vec(&response).unwrap(); let body = futures_util::stream::once(async move { + // Signal that HTTP headers have been sent and the body stream started. + if let Some(started) = started { + started.notify_one(); + } tokio::time::sleep(delay).await; Ok::(web::Bytes::from(json_bytes)) }); @@ -196,6 +203,7 @@ impl FakeOidcProvider { auth_codes: HashMap::new(), jwt_customizer: None, token_endpoint_delay: Duration::ZERO, + token_endpoint_started: None, })); let state_for_server = Arc::clone(&state); @@ -237,8 +245,15 @@ impl FakeOidcProvider { f(&mut state) } - pub fn set_token_endpoint_delay(&self, delay: Duration) { - self.with_state_mut(|s| s.token_endpoint_delay = delay); + /// Set a delay on the token endpoint body and return a Notify that fires + /// once the endpoint has sent response headers and started the body stream. + pub fn set_token_endpoint_delay(&self, delay: Duration) -> Arc { + let started = Arc::new(Notify::new()); + self.with_state_mut(|s| { + s.token_endpoint_delay = delay; + s.token_endpoint_started = Some(started.clone()); + }); + started } pub fn store_auth_code(&self, code: String, nonce: String) { @@ -571,7 +586,7 @@ async fn test_slow_token_endpoint_does_not_freeze_server() { let redirect_uri = get_query_param(&auth_url, "redirect_uri"); provider.store_auth_code("test_auth_code".to_string(), nonce); - provider.set_token_endpoint_delay(Duration::from_secs(999)); + let body_started = provider.set_token_endpoint_delay(Duration::from_secs(999)); let callback_uri = format!( "{}?code=test_auth_code&state={}", @@ -589,10 +604,15 @@ async fn test_slow_token_endpoint_does_not_freeze_server() { test::call_service(&app, req.to_request()).await }); - // Let the localhost TCP round trip complete in real time (microseconds). - for _ in 0..1000 { - tokio::task::yield_now().await; - } + // Wait until the token endpoint has sent HTTP headers and started the body + // stream. At this point headers are in the TCP buffer but the awc client + // may not have read them yet. Yield to let it process the I/O events and + // enter response.body().await — this is deterministic because the data is + // already available in the socket buffer. + body_started.notified().await; + tokio::task::yield_now().await; + tokio::task::yield_now().await; + tokio::task::yield_now().await; // Freeze time and advance past the body-read timeout. If one is set, the // request completes. If not, only the 999s endpoint delay would wake it. From c2a18e453c70617ba32910e4022255f17cd49215 Mon Sep 17 00:00:00 2001 From: Ophir LOJKINE Date: Sun, 8 Mar 2026 03:40:01 +0100 Subject: [PATCH 7/8] simplify test: replace Notify+yields with sleep+time-advance Remove the Notify synchronization and yield_now() calls. Instead, use a small real-time sleep for TCP to complete, then pause+advance tokio time. Assert the actual response status instead of is_finished(). Co-Authored-By: Claude Opus 4.6 --- tests/oidc/mod.rs | 51 +++++++++++++---------------------------------- 1 file changed, 14 insertions(+), 37 deletions(-) diff --git a/tests/oidc/mod.rs b/tests/oidc/mod.rs index a607f24f..027fe6ac 100644 --- a/tests/oidc/mod.rs +++ b/tests/oidc/mod.rs @@ -13,7 +13,6 @@ use sqlpage::webserver::http::create_app; use std::collections::HashMap; use std::sync::{Arc, Mutex}; use std::time::Duration; -use tokio::sync::Notify; use tokio_util::sync::{CancellationToken, DropGuard}; fn base64url_encode(data: &[u8]) -> String { @@ -53,7 +52,6 @@ struct ProviderState<'a> { auth_codes: HashMap, // code -> nonce jwt_customizer: Option>>, token_endpoint_delay: Duration, - token_endpoint_started: Option>, } type ProviderStateWithLifetime<'a> = ProviderState<'a>; @@ -147,7 +145,6 @@ async fn token_endpoint( .unwrap_or_else(|| make_jwt(&claims, &state.secret)); let delay = state.token_endpoint_delay; - let started = state.token_endpoint_started.clone(); drop(state); let response = TokenResponse { @@ -159,10 +156,6 @@ async fn token_endpoint( let json_bytes = serde_json::to_vec(&response).unwrap(); let body = futures_util::stream::once(async move { - // Signal that HTTP headers have been sent and the body stream started. - if let Some(started) = started { - started.notify_one(); - } tokio::time::sleep(delay).await; Ok::(web::Bytes::from(json_bytes)) }); @@ -203,7 +196,6 @@ impl FakeOidcProvider { auth_codes: HashMap::new(), jwt_customizer: None, token_endpoint_delay: Duration::ZERO, - token_endpoint_started: None, })); let state_for_server = Arc::clone(&state); @@ -245,15 +237,8 @@ impl FakeOidcProvider { f(&mut state) } - /// Set a delay on the token endpoint body and return a Notify that fires - /// once the endpoint has sent response headers and started the body stream. - pub fn set_token_endpoint_delay(&self, delay: Duration) -> Arc { - let started = Arc::new(Notify::new()); - self.with_state_mut(|s| { - s.token_endpoint_delay = delay; - s.token_endpoint_started = Some(started.clone()); - }); - started + pub fn set_token_endpoint_delay(&self, delay: Duration) { + self.with_state_mut(|s| s.token_endpoint_delay = delay); } pub fn store_auth_code(&self, code: String, nonce: String) { @@ -586,7 +571,7 @@ async fn test_slow_token_endpoint_does_not_freeze_server() { let redirect_uri = get_query_param(&auth_url, "redirect_uri"); provider.store_auth_code("test_auth_code".to_string(), nonce); - let body_started = provider.set_token_endpoint_delay(Duration::from_secs(999)); + provider.set_token_endpoint_delay(Duration::from_secs(999)); let callback_uri = format!( "{}?code=test_auth_code&state={}", @@ -594,8 +579,6 @@ async fn test_slow_token_endpoint_does_not_freeze_server() { state_param ); - // Spawn the callback request. It will exchange the auth code for a token, - // but the token endpoint body is delayed by 999s (headers arrive immediately). let handle = tokio::task::spawn_local(async move { let mut req = test::TestRequest::get().uri(&callback_uri); for cookie in cookies.iter() { @@ -604,24 +587,18 @@ async fn test_slow_token_endpoint_does_not_freeze_server() { test::call_service(&app, req.to_request()).await }); - // Wait until the token endpoint has sent HTTP headers and started the body - // stream. At this point headers are in the TCP buffer but the awc client - // may not have read them yet. Yield to let it process the I/O events and - // enter response.body().await — this is deterministic because the data is - // already available in the socket buffer. - body_started.notified().await; - tokio::task::yield_now().await; - tokio::task::yield_now().await; - tokio::task::yield_now().await; - - // Freeze time and advance past the body-read timeout. If one is set, the - // request completes. If not, only the 999s endpoint delay would wake it. + // Let the localhost TCP round-trip complete so awc reads response headers. + tokio::time::sleep(Duration::from_millis(50)).await; + + // Freeze time and advance past the body-read timeout. tokio::time::pause(); tokio::time::advance(Duration::from_secs(60)).await; - tokio::task::yield_now().await; - assert!( - handle.is_finished(), - "OIDC callback hung on a slow token endpoint" - ); + // The body timeout should have fired, completing the request with an error + // that SQLPage handles by redirecting to the OIDC provider. + let resp = tokio::time::timeout(Duration::from_secs(1), handle) + .await + .expect("OIDC callback hung on a slow token endpoint") + .unwrap(); + assert_eq!(resp.status(), StatusCode::SEE_OTHER); } From f438985e93f38574ed4e58cfbd69de5a5b1741d7 Mon Sep 17 00:00:00 2001 From: Ophir LOJKINE Date: Sun, 8 Mar 2026 03:43:33 +0100 Subject: [PATCH 8/8] fix clippy: remove unnecessary mut on response Co-Authored-By: Claude Opus 4.6 --- src/webserver/oidc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/webserver/oidc.rs b/src/webserver/oidc.rs index b681720d..a6d83e20 100644 --- a/src/webserver/oidc.rs +++ b/src/webserver/oidc.rs @@ -838,7 +838,7 @@ async fn execute_oidc_request_with_awc( req = req.insert_header((name.as_str(), value.to_str()?)); } let (req_head, body) = request.into_parts(); - let mut response = req.send_body(body).await.map_err(|e| { + let response = req.send_body(body).await.map_err(|e| { anyhow!(e.to_string()).context(format!( "Failed to send request: {} {}", &req_head.method, &req_head.uri