Skip to content

Conversation

arturobernalg
Copy link
Member

Enable env-proxy support ([HTTPCLIENT-2381](Enable env-proxy support (HTTPCLIENT-2381) – add EnvironmentProxyConfigurer, wire it into HttpClientBuilder, revert extra fallbacks, and introduce JUnit test (System-Lambda) with dep-management cleanup.)) – add EnvironmentProxyConfigurer, wire it into HttpClientBuilder, revert extra fallbacks, and introduce JUnit test (System-Lambda) with dep-management cleanup.

@arturobernalg arturobernalg requested a review from ok2c July 17, 2025 14:49
@arturobernalg arturobernalg force-pushed the HTTPCLIENT-2381 branch 2 times, most recently from 0f8cc97 to 537d126 Compare July 17, 2025 17:12
@arturobernalg arturobernalg force-pushed the HTTPCLIENT-2381 branch 2 times, most recently from 77210eb to 1fa05b7 Compare July 18, 2025 16:24
@arturobernalg arturobernalg requested a review from ok2c July 18, 2025 16:24
…Y environment variables to standard JDK proxy system properties via new EnvironmentProxyConfigurer and make HttpClientBuilder use it by default
Copy link
Member

@ok2c ok2c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arturobernalg You should also add a similar option to HttpAsyncClientBuilder.

Please also invite the original reporter to take a look at the proposed fix. From I understand (but I can well be wrong) the reporter actually wants us to read system properties by default instead of opt-in. I am not sure this change-set is going to be enough.

@ok2c
Copy link
Member

ok2c commented Jul 20, 2025

@arturobernalg Actually I think the fix is misplaced. This logic should go into SystemDefaultRoutePlanner and our code must not change the system properties at all but populate HttpRoutes instead based on environment

@@ -237,6 +237,9 @@ private ExecInterceptorEntry(

private List<Closeable> closeables;

private boolean applyEnvProxy;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arturobernalg This should no longer be necessary, should it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arturobernalg This should no longer be necessary, should it?

done

…r HTTP(S)_PROXY / NO_PROXY directly without altering JVM system properties
…roxySelector first, then HTTP(S)_PROXY/NO_PROXY).

Add disableProxyAutodetection() to preserve legacy behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants