Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spring Web reactive client does not configure a proxy using system properties by default #31023

Closed
codebje opened this issue Aug 9, 2023 · 5 comments
Labels
status: invalid An issue that we don't feel is valid

Comments

@codebje
Copy link

codebje commented Aug 9, 2023

Affects: Spring Framework 6.0.11 (and HEAD)

The current ReactorClientHttpConnector creates itself an HttpClient and configures it to enable compression. It should also enable proxies using system properties by default by adding .proxyWithSystemProperties() to the defaultInitializer here.

Without this there are a number of beans that are difficult or impossible to configure to use the system proxy settings, such as those involved in OAuth2/OIDC client login - obtaining a token, fetching user info, and fetching JWKS data all use a separate WebClient instance created ultimately using the default ReactorClientHttpConnector.

I am not sure why this client connector isn't using the system proxy settings already: the default RestTemplate does, and if a proxy has been configured at the java system level it is likely to be important.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 9, 2023
@snicoll
Copy link
Member

snicoll commented Aug 10, 2023

I am not sure why this client connector isn't using the system proxy settings already: the default RestTemplate does, and if a proxy has been configured at the java system level it is likely to be important.

I am not sure what you mean by that. I am not aware our codebase reacts to a system property to configure a proxy. Can you please clarify what you mean by that? Perhaps the connector that you are using is doing this for you?

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Aug 10, 2023
@codebje
Copy link
Author

codebje commented Aug 11, 2023

I am not sure why this client connector isn't using the system proxy settings already: the default RestTemplate does, and if a proxy has been configured at the java system level it is likely to be important.

I am not sure what you mean by that. I am not aware our codebase reacts to a system property to configure a proxy. Can you please clarify what you mean by that? Perhaps the connector that you are using is doing this for you?

Hi, thanks for responding.

It is the connector being used that's doing so, but it's not the one I'm using - it's the one that various Spring components are using whenever they invoke new RestTemplate() or WebClient.create().

In my specific circumstances I hit the problem during OpenID Connect login processing, which makes three network calls to fetch resources (four, if one includes the start-up fetch of provider info), all of which use a different WebClient, two of which are relatively easy to provide with a customised WebClient and one of which is not. The start-up fetch is always non-reactive and simply uses the proxy properties. The Spring Security team have a long-standing issue about the general problem of configuring WebClients used here and in other auth components (see spring-projects/spring-security#8882).

Because the choice of connector is hard-wired in WebClientBuilder based on classes available in the class path, and the connectors are not configurable at an application-wide level, I raised this issue to see if there's a specific reason to exclude the use of proxy properties, or if it's something that could be changed at this level.

This change doesn't solve all the concerns that Spring Security are looking into - their issue covers timeouts and load balancing, at least - but it does prevent other people from playing whack-a-mole with finding components that make connections using WebClient.create() that fail when proxies must be used.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Aug 11, 2023
@snicoll
Copy link
Member

snicoll commented Aug 11, 2023

it's the one that various Spring components are using whenever they invoke new RestTemplate() or WebClient.create().

Sorry, I don't think that answers my question. Can you provide the reference of what is reacting to a system property with RestTemplate?

@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Aug 11, 2023
@codebje
Copy link
Author

codebje commented Aug 11, 2023

The Java URL object that's ultimately used to make the connection with RestTemplate is using the system proxy properties, as described here but not very well at all on the javadoc for URL.

The asynchronous connectors used by DefaultWebClientBuilder (Netty, Netty 5, Jetty, OkHttp, and JDK, chosen in that order) are usually operating at the socket level and bypass the Java HTTP framework - except, of course, for JdkClientHttpConnector, but DefaultWebClientBuilder only uses that as a last resort.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Aug 11, 2023
@snicoll
Copy link
Member

snicoll commented Oct 8, 2023

Sorry but there's still no evidence that our web stack is configuring a proxy from system properties. I think you're misleading when you say that RestTemplate does it by default. It can certainly be a feature of the connector that you use, but not something that Spring does.

As a framework, I think we should keep things this way.

@snicoll snicoll closed this as not planned Won't fix, can't repro, duplicate, stale Oct 8, 2023
@snicoll snicoll added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Oct 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

3 participants