mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Honor disabled redirects in route-aware HTTP clients (#34978)
## Why With system-proxy-aware routing, `RouteAwareClientPool` handles redirects itself so each hop can resolve its own route. That manual path must also respect clients configured not to follow redirects. ## What changed - Add no-redirect constructors for standard and ChatGPT Cloudflare-cookie route-aware client pools. - Skip manual redirect handling when the underlying client builder has redirects disabled, returning the redirect response to the caller. ## Testing - Cover both outbound proxy policies and verify that a no-redirect pool returns the initial `302 Found` response after one request. GitOrigin-RevId: 7eec73f21f506a46e5d4b82f425ddbc9761ca61c
This commit is contained in:
@@ -78,6 +78,10 @@ impl HttpClientBuilder {
|
||||
self
|
||||
}
|
||||
|
||||
pub(crate) fn follows_redirects(&self) -> bool {
|
||||
self.follow_redirects
|
||||
}
|
||||
|
||||
/// Limits only connection establishment, not the request as a whole.
|
||||
pub fn connect_timeout(mut self, timeout: Duration) -> Self {
|
||||
self.connect_timeout = Some(timeout);
|
||||
|
||||
@@ -226,6 +226,21 @@ impl RouteAwareClientPool {
|
||||
Self::with_builder(http_client_factory, route_class, HttpClientBuilder::new())
|
||||
}
|
||||
|
||||
/// Creates a pool that returns redirect responses without following them.
|
||||
///
|
||||
/// This applies both when reqwest owns redirect handling and when the pool follows redirects
|
||||
/// manually so each hop can receive its own proxy-route decision.
|
||||
pub fn new_without_redirects(
|
||||
http_client_factory: HttpClientFactory,
|
||||
route_class: ClientRouteClass,
|
||||
) -> Self {
|
||||
Self::with_builder(
|
||||
http_client_factory,
|
||||
route_class,
|
||||
HttpClientBuilder::new().without_redirects(),
|
||||
)
|
||||
}
|
||||
|
||||
/// Creates a pool whose clients limit only connection establishment.
|
||||
///
|
||||
/// The timeout applies to every client built for a resolved route, including redirect hops.
|
||||
@@ -278,6 +293,21 @@ impl RouteAwareClientPool {
|
||||
)
|
||||
}
|
||||
|
||||
/// Creates a no-redirect pool that retains the Cloudflare cookies required by ChatGPT
|
||||
/// endpoints.
|
||||
pub fn with_chatgpt_cloudflare_cookies_without_redirects(
|
||||
http_client_factory: HttpClientFactory,
|
||||
route_class: ClientRouteClass,
|
||||
) -> Self {
|
||||
Self::with_builder(
|
||||
http_client_factory,
|
||||
route_class,
|
||||
HttpClientBuilder::new()
|
||||
.with_chatgpt_cloudflare_cookie_store()
|
||||
.without_redirects(),
|
||||
)
|
||||
}
|
||||
|
||||
/// Creates a ChatGPT Cloudflare-cookie pool without URL or response-header diagnostics.
|
||||
pub fn with_chatgpt_cloudflare_cookies_without_request_logging(
|
||||
http_client_factory: HttpClientFactory,
|
||||
@@ -354,8 +384,9 @@ impl RouteAwareClientPool {
|
||||
{
|
||||
let request_method = request.method().clone();
|
||||
let request_url = request.url().to_string();
|
||||
let follows_redirects_manually = self.http_client_factory.outbound_proxy_policy()
|
||||
== OutboundProxyPolicy::RespectSystemProxy;
|
||||
let follows_redirects_manually = self.client_builder.follows_redirects()
|
||||
&& self.http_client_factory.outbound_proxy_policy()
|
||||
== OutboundProxyPolicy::RespectSystemProxy;
|
||||
let timeout_deadline = request
|
||||
.timeout()
|
||||
.copied()
|
||||
@@ -421,7 +452,7 @@ impl RouteAwareClientPool {
|
||||
}
|
||||
};
|
||||
let status = response.status();
|
||||
if !is_redirect(status) {
|
||||
if !follows_redirects_manually || !is_redirect(status) {
|
||||
if follows_redirects_manually {
|
||||
client.log_response(&request_method, &request_url, &response);
|
||||
}
|
||||
|
||||
@@ -156,6 +156,38 @@ async fn reqwest_default_route_preserves_transport_redirects() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn no_redirect_pool_returns_redirect_response() {
|
||||
for outbound_proxy_policy in [
|
||||
OutboundProxyPolicy::ReqwestDefault,
|
||||
OutboundProxyPolicy::RespectSystemProxy,
|
||||
] {
|
||||
let (address, server) = spawn_response_server(vec![
|
||||
"HTTP/1.1 302 Found\r\nLocation: /final\r\nContent-Length: 0\r\nConnection: close\r\n\r\n"
|
||||
.to_string(),
|
||||
]);
|
||||
let pool = RouteAwareClientPool::new_without_redirects(
|
||||
HttpClientFactory::new(outbound_proxy_policy),
|
||||
ClientRouteClass::Api,
|
||||
);
|
||||
let initial_url = format!("http://{address}/start");
|
||||
let request = reqwest::Request::new(
|
||||
Method::GET,
|
||||
reqwest::Url::parse(&initial_url).expect("request URL should parse"),
|
||||
);
|
||||
|
||||
let response = pool
|
||||
.send_with_resolver(request, |_| async { Ok(OutboundProxyRoute::Direct) })
|
||||
.await
|
||||
.expect("no-redirect request should finish");
|
||||
|
||||
assert_eq!(response.status(), StatusCode::FOUND);
|
||||
let requests = server.join().expect("redirect server should finish");
|
||||
assert_eq!(requests.len(), 1);
|
||||
assert!(requests[0].starts_with("GET /start HTTP/1.1\r\n"));
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn bounds_cached_routes_and_rebuilds_an_evicted_route() {
|
||||
let pool = RouteAwareClientPool::with_builder(
|
||||
@@ -488,6 +520,9 @@ fn spawn_response_server(
|
||||
Err(error) => panic!("response server should accept: {error}"),
|
||||
}
|
||||
};
|
||||
stream
|
||||
.set_nonblocking(false)
|
||||
.expect("response stream should become blocking");
|
||||
stream
|
||||
.set_read_timeout(Some(Duration::from_secs(2)))
|
||||
.expect("response stream should get a read timeout");
|
||||
|
||||
Reference in New Issue
Block a user