Add withDefaultRequestConfigCustomizer method to HttpComponentsClientHttpRequestFactoryBuilder

[oyvindhorneland]

Apache HttpClient 5.4.x (in upcoming Spring Boot 3.4) has by default enabled HTTP/1.1 TLS Upgrade in apache/httpcomponents-client#542. This causes an issue for k8s deployments using Istio service mesh (and Envoy proxies) as described in istio/istio#53239 where outbound http requests will receive a HTTP status 403 with "upgrade_failed".

The issue has been reported to the Apache project in https://issues.apache.org/jira/browse/HTTPCLIENT-2344, where it has been closed as invalid since they believe Envoy is not behaving correctly.

The issue has been reported to Envoy in envoyproxy/envoy#36305 where discussions are ongoing.

Note that the protocol upgrade is only enabled for OPTIONS, HEAD and GET requests and clients may therefore observe that some requests work and others don't (Envoy will block the ones containing the TLS upgrade headers).

Code based workaround is to change protocolUpgradeEnabled to false when creating the HttpClient's RequestConfig.

        RequestConfig requestConfig = RequestConfig.custom()
                // ...
                .setProtocolUpgradeEnabled(false)
                .build();
        HttpClient httpClient = HttpClientBuilder.create()
                // ...
                .setDefaultRequestConfig(requestConfig)
                .build();

There is currently no system property in Apache HttpClient5 to disable protocolUpgradeEnable.

Should this known issue and possible workarounds be listed in the migration guide?

Not sure if a configuration option is a good possibility here, but HttpComponentsClientHttpRequestFactory will currently by default try to use HttpClients.createSystem() with protocolUpgrade enabled by default.

[nosan]

A possible workaround is configuring your own ClientHttpRequestFactoryBuilder:

@Bean
public HttpComponentsClientHttpRequestFactoryBuilder httpComponentsClientHttpRequestFactoryBuilder() {
	return ClientHttpRequestFactoryBuilder.httpComponents()
			.withHttpClientCustomizer(this::disableProtocolUpgrade);
}

private void disableProtocolUpgrade(HttpClientBuilder builder) {
	builder.setDefaultRequestConfig(RequestConfig.custom()
			.setProtocolUpgradeEnabled(false)
			.build());
}

[oyvindhorneland]

Thank you and yes, that is probably a better workaround if it affects all autoconfigured HttpClient instances.

[nosan]

if it affects all autoconfigured HttpClient instances.

The ClientHttpRequestFactoryBuilder is used by RestClientAutoConfiguration, RestTemplateAutoConfiguration, and WebServiceTemplateAutoConfiguration.

[oyvindhorneland]

Any thoughts on if the new default settings for Apache HttpClient's protocolUpgradeEnabled should be kept as a default in Spring Boot or if the default in Spring Boot should be to disable it or make it easy to configure it through a built-in configuration property? The reasoning in https://issues.apache.org/jira/browse/HTTPCLIENT-2344 argues for a secure and spec compliant default, but this change will also mean changing the behavior of outbound connections for existing apps and that some apps may break. And it currently looks like Envoy may continue being strict and keep rejecting it by default due to fear of security issues.

[philwebb]

I think we should probably align with whatever defaults HttpClient provides. That's generally the rule that we try to follow. It's not ideal that there's a mismatch between what the HttpClient and Envoy maintainers consider sensible default behavior.

I'm not too keen to add a configuration property for this because we don't currently have any client specific properties. It would need to be something like spring.http.client.httpcomponents.protocol.upgrade.enabled=false. That then opens the door for a whole bunch of other properties being requested.

I do think we can make #43139 (comment) a little easier to apply if we tweak our code. Something like:

@Bean
public HttpComponentsClientHttpRequestFactoryBuilder httpComponentsClientHttpRequestFactoryBuilder() {
	return ClientHttpRequestFactoryBuilder.httpComponents()
			.withDefaultRequestConfigManagerCustomizer((builder) -> builder.setProtocolUpgradeEnabled(false));

I think we might have to do that as a first step and see what happens with https://issues.apache.org/jira/browse/HTTPCLIENT-2344 and envoyproxy/envoy#36305

[nosan]

Thanks @philwebb

Would it make sense to add a note in the migration guide as well, if feasible?

[philwebb]

Good idea, I'll do that as well.

[rafalh]

Where did the "manager" word came from in withDefaultRequestConfigManagerCustomizer? I think it would be nicer to have it named withDefaultRequestConfigCustomizer

[philwebb]

I think I made a mistake copy/pasting another property. Thanks for noticing @rafalh

[nosan]

@philwebb 😄

 * @param defaultRequestConfigManagerCustomizer the customizer to apply
	public HttpComponentsClientHttpRequestFactoryBuilder withDefaultRequestConfigCustomizer(
			Consumer<RequestConfig.Builder> **defaultRequestConfigManagerCustomizer**) {
		Assert.notNull(**defaultRequestConfigManagerCustomizer**,
				"'**defaultRequestConfigManagerCustomizer**' must not be null");

[philwebb]

This issue is cursed 😂

[rafalh]

@philwebb It's definitely cursed :P You forgot to update Spring Boot 3.4 Release Notes. It still references withDefaultRequestConfigManagerCustomizer

[wilkinsona]

Thanks, @rafalh. I've updated the release notes.