|
1 | | -Some deployments need a general HTTPS proxy for public services while selected |
2 | | -hosts must still be reached directly. On Solaris, the reqwest system proxy |
3 | | -matcher can route an MCP HTTP request through the proxy even when NO_PROXY |
4 | | -contains the destination host. |
| 1 | +Codex 0.147 moved outbound HTTP traffic to a shared route-aware client. Its |
| 2 | +RespectSystemProxy policy explicitly resolves environment proxies on Solaris |
| 3 | +and attaches NO_PROXY/no_proxy to the selected proxy, avoiding reqwest's |
| 4 | +system proxy autodetection for those requests. However, that policy is an |
| 5 | +under-development feature and remains disabled by default upstream. |
5 | 6 |
|
6 | | -What it does: |
7 | | -- evaluates NO_PROXY/no_proxy in Codex before constructing the per-request |
8 | | - reqwest client |
9 | | -- disables proxy autodetection only for destinations matching NO_PROXY/no_proxy |
10 | | -- keeps existing proxy behavior for all other destinations |
| 7 | +Keep the upstream default on other platforms, but enable the shared policy by |
| 8 | +default on Solaris. This replaces the older exec-server-only workaround and |
| 9 | +also covers auth, API, WebSocket, and redirect routing through the shared HTTP |
| 10 | +client factory. |
11 | 11 |
|
12 | | -This was offered to upstream via: |
| 12 | +The original Solaris problem was reported upstream via: |
13 | 13 | https://github.com/openai/codex/issues/4242 |
14 | 14 |
|
15 | | -diff --git a/codex-rs/exec-server/src/client/reqwest_http_client.rs b/codex-rs/exec-server/src/client/reqwest_http_client.rs |
16 | | -index d0cffa6..dd849de 100644 |
17 | | ---- a/codex-rs/exec-server/src/client/reqwest_http_client.rs |
18 | | -+++ b/codex-rs/exec-server/src/client/reqwest_http_client.rs |
19 | | -@@ -49,20 +49,25 @@ pub(crate) struct PendingReqwestHttpBodyStream { |
20 | | - /// Validates `http/request` parameters and runs the actual `reqwest` call used |
21 | | - /// by the exec-server route and the local [`HttpClient`] backend. |
22 | | - pub(crate) struct ReqwestHttpRequestRunner { |
23 | | -- client: reqwest::Client, |
24 | | -+ timeout_ms: Option<u64>, |
25 | | -+ redirect_policy: HttpRedirectPolicy, |
26 | | - } |
27 | | - |
28 | | - impl ReqwestHttpClient { |
29 | | - fn build_client( |
30 | | - timeout_ms: Option<u64>, |
31 | | - redirect_policy: HttpRedirectPolicy, |
32 | | -+ url: &Url, |
33 | | - ) -> Result<reqwest::Client, ExecServerError> { |
34 | | -- let builder = match timeout_ms { |
35 | | -+ let mut builder = match timeout_ms { |
36 | | - None => reqwest::Client::builder(), |
37 | | - Some(timeout_ms) => { |
38 | | - reqwest::Client::builder().timeout(Duration::from_millis(timeout_ms)) |
39 | | - } |
40 | | - }; |
41 | | -+ if request_matches_no_proxy(url) { |
42 | | -+ builder = builder.no_proxy(); |
43 | | -+ } |
44 | | - let builder = match redirect_policy { |
45 | | - HttpRedirectPolicy::Follow => builder, |
46 | | - HttpRedirectPolicy::Stop => builder.redirect(reqwest::redirect::Policy::none()), |
47 | | -@@ -125,9 +130,10 @@ impl ReqwestHttpRequestRunner { |
48 | | - timeout_ms: Option<u64>, |
49 | | - redirect_policy: HttpRedirectPolicy, |
50 | | - ) -> Result<Self, JSONRPCErrorError> { |
51 | | -- let client = ReqwestHttpClient::build_client(timeout_ms, redirect_policy) |
52 | | -- .map_err(|error| internal_error(error.to_string()))?; |
53 | | -- Ok(Self { client }) |
54 | | -+ Ok(Self { |
55 | | -+ timeout_ms, |
56 | | -+ redirect_policy, |
57 | | -+ }) |
58 | | - } |
59 | | - |
60 | | - pub(crate) async fn run( |
61 | | -@@ -159,7 +165,9 @@ impl ReqwestHttpRequestRunner { |
62 | | - ); |
63 | | - let mut headers = Self::build_headers(params.headers)?; |
64 | | - codex_otel::inject_span_w3c_trace_headers(&request_span, &mut headers); |
65 | | -- let mut request = self.client.request(method.clone(), url).headers(headers); |
66 | | -+ let client = ReqwestHttpClient::build_client(self.timeout_ms, self.redirect_policy, &url) |
67 | | -+ .map_err(|error| internal_error(error.to_string()))?; |
68 | | -+ let mut request = client.request(method.clone(), url).headers(headers); |
69 | | - if let Some(body) = params.body { |
70 | | - request = request.body(body.into_inner()); |
71 | | - } |
72 | | -@@ -320,3 +328,98 @@ fn error_source_chain(error: &reqwest::Error) -> Option<String> { |
73 | | - } |
74 | | - (!sources.is_empty()).then(|| sources.join(": ")) |
75 | | - } |
76 | | -+ |
77 | | -+fn request_matches_no_proxy(url: &Url) -> bool { |
78 | | -+ let Some(host) = url.host_str() else { |
79 | | -+ return false; |
80 | | -+ }; |
81 | | -+ let Some(no_proxy) = configured_no_proxy() else { |
82 | | -+ return false; |
83 | | -+ }; |
84 | | -+ no_proxy_list_matches_host(&no_proxy, host) |
85 | | -+} |
86 | | -+ |
87 | | -+fn configured_no_proxy() -> Option<String> { |
88 | | -+ ["NO_PROXY", "no_proxy"].into_iter().find_map(|key| { |
89 | | -+ let value = std::env::var(key).ok()?; |
90 | | -+ if value.trim().is_empty() { |
91 | | -+ return None; |
92 | | -+ } |
93 | | -+ Some(value) |
94 | | -+ }) |
95 | | -+} |
96 | | -+ |
97 | | -+fn no_proxy_list_matches_host(no_proxy: &str, host: &str) -> bool { |
98 | | -+ let host = normalize_no_proxy_host(host); |
99 | | -+ if host.is_empty() { |
100 | | -+ return false; |
101 | | -+ } |
102 | | -+ |
103 | | -+ no_proxy.split(',').any(|entry| { |
104 | | -+ let entry = normalize_no_proxy_host(strip_no_proxy_port(entry.trim())); |
105 | | -+ if entry.is_empty() { |
106 | | -+ return false; |
107 | | -+ } |
108 | | -+ if entry == "*" { |
109 | | -+ return true; |
110 | | -+ } |
111 | | -+ |
112 | | -+ let entry = entry.strip_prefix('.').unwrap_or(&entry); |
113 | | -+ host == entry || host.ends_with(&format!(".{entry}")) |
114 | | -+ }) |
115 | | -+} |
116 | | -+ |
117 | | -+fn normalize_no_proxy_host(host: &str) -> String { |
118 | | -+ host.trim() |
119 | | -+ .trim_matches(&['[', ']'][..]) |
120 | | -+ .trim_end_matches('.') |
121 | | -+ .to_ascii_lowercase() |
122 | | -+} |
123 | | -+ |
124 | | -+fn strip_no_proxy_port(entry: &str) -> &str { |
125 | | -+ let Some((host, port)) = entry.rsplit_once(':') else { |
126 | | -+ return entry; |
127 | | -+ }; |
128 | | -+ if host.contains(':') || port.is_empty() || !port.bytes().all(|byte| byte.is_ascii_digit()) { |
129 | | -+ return entry; |
130 | | -+ } |
131 | | -+ host |
132 | | -+} |
133 | | -+ |
134 | | -+#[cfg(test)] |
135 | | -+mod no_proxy_tests { |
136 | | -+ use super::no_proxy_list_matches_host; |
137 | | -+ |
138 | | -+ #[test] |
139 | | -+ fn no_proxy_matches_exact_host() { |
140 | | -+ assert!(no_proxy_list_matches_host( |
141 | | -+ "service.example.com", |
142 | | -+ "service.example.com" |
143 | | -+ )); |
144 | | -+ } |
145 | | -+ |
146 | | -+ #[test] |
147 | | -+ fn no_proxy_matches_parent_domain() { |
148 | | -+ assert!(no_proxy_list_matches_host( |
149 | | -+ ".example.com", |
150 | | -+ "service.example.com" |
151 | | -+ )); |
152 | | -+ assert!(no_proxy_list_matches_host( |
153 | | -+ "example.com", |
154 | | -+ "service.example.com" |
155 | | -+ )); |
156 | | -+ } |
157 | | -+ |
158 | | -+ #[test] |
159 | | -+ fn no_proxy_rejects_partial_suffix() { |
160 | | -+ assert!(!no_proxy_list_matches_host("example.com", "notexample.com")); |
161 | | -+ } |
162 | | -+ |
163 | | -+ #[test] |
164 | | -+ fn no_proxy_ignores_optional_port() { |
165 | | -+ assert!(no_proxy_list_matches_host( |
166 | | -+ "service.example.com:443", |
167 | | -+ "service.example.com" |
168 | | -+ )); |
169 | | -+ } |
170 | | -+} |
| 15 | +diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs |
| 16 | +--- a/codex-rs/features/src/lib.rs |
| 17 | ++++ b/codex-rs/features/src/lib.rs |
| 18 | +@@ -1100,7 +1100,7 @@ const FEATURES: &[FeatureSpec] = &[ |
| 19 | + id: Feature::RespectSystemProxy, |
| 20 | + key: "respect_system_proxy", |
| 21 | + stage: Stage::UnderDevelopment, |
| 22 | +- default_enabled: false, |
| 23 | ++ default_enabled: cfg!(target_os = "solaris"), |
| 24 | + }, |
| 25 | + FeatureSpec { |
| 26 | + id: Feature::Collab, |
0 commit comments