Configured RequestConfig in HttpClient gets overwritten by default [SPR-12540]

[spring-projects-issues]

Jean-Pierre Bergamin opened SPR-12540 and commented

HttpComponentsClientHttpRequestFactory.createRequest always sets the HttpClientContext.REQUEST_CONFIG attribute of the HttpContext - also with a default config if none can be found.

The HttpClientBuilder on the other hand allows to set a default RequestConfig with:

RequestConfig requestConfig = RequestConfig.custom()
        .setCookieSpec(NO_SPEC_PROVIDER)
        .build();

 HttpClient httpClient = HttpClientBuilder.create().
        ....
        setDefaultRequestConfig(requestConfig).
        ...		
        build();

The returned InternalHttpClient by the builder later sets this configured default requestConfig if no other request config has been set:

private void setupContext(final HttpClientContext context) {
    ....
    if (context.getAttribute(HttpClientContext.REQUEST_CONFIG) == null) {
        context.setAttribute(HttpClientContext.REQUEST_CONFIG, this.defaultConfig);
    }
}

But since a default RequestConfig has been set in HttpComponentsClientHttpRequestFactory.createRequest, it is not applied here and the configured RequestConfig in the HttpClientBuilder is not used.

As a workaround you have to create your own HttpComponentsClientHttpRequestFactory implementation and overwrite createHttpContext to return a configured HttpContext like:

public class StatefullHttpComponentsClientHttpRequestFactory extends HttpComponentsClientHttpRequestFactory
{
    private final HttpContext httpContext;

    public StatefullHttpComponentsClientHttpRequestFactory(HttpClient httpClient, HttpContext httpContext)
    {
        super(httpClient);
        this.httpContext = httpContext;
    }

    @Override
    protected HttpContext createHttpContext(HttpMethod httpMethod, URI uri)
    {
        return this.httpContext;
    }
}

HttpContext context = new BasicHttpContext();
context.setAttribute(HttpClientContext.REQUEST_CONFIG, requestConfig);

ClientHttpRequestFactory requestFactory = new ContextAwareHttpComponentsClientHttpRequestFactory(httpClient, context));

RestOperations restClient = new RestTemplate(requestFactory);

Conclusion: Do not set a default RequestConfig in HttpComponentsClientHttpRequestFactory if the http client is of instance InternalHttpClient and therefore configured by the HttpClientBuilder.

---

Issue Links:

• When using HttpComponentsClientHttpRequestFactory, setting a proxy through RequestConfig should not be ignored [SPR-12335] #16940 When using HttpComponentsClientHttpRequestFactory, setting a proxy through RequestConfig should not be ignored ("duplicates")
• Upgrade HttpComponentsHttpInvokerRequestExecutor to require Apache HttpComponents 4.3 [SPR-11113] #15739 Upgrade HttpComponentsHttpInvokerRequestExecutor to require Apache HttpComponents 4.3
• Merge default request config with local customizations [SPR-12583] #17184 Merge default request config with local customizations
• HttpComponentsClientHttpRequestFactory does not set connection request timeout on request config [SPR-12166] #16780 HttpComponentsClientHttpRequestFactory does not set connection request timeout on request config
• HttpComponentsAsyncClientHttpRequestFactory overrides default request config of httpclient [SPR-13125] #17716 HttpComponentsAsyncClientHttpRequestFactory overrides default request config of httpclient

Referenced from: commits 71783c5, 5236eb6

1 votes, 7 watchers

[spring-projects-issues]

Rossen Stoyanchev commented

Or is it perhaps that if we are given an HttpClient, i.e. the HttpComponentsClientHttpRequestFactory(HttpClient httpClient) constructor is used, then none of the other properties in the factory (connectTimeout, socketTimeout) apply and therefore we should not change the RequestContext? InternalHttpClient is an internal class so I'm not sure we can rely on it i any way.

[spring-projects-issues]

Jean-Pierre Bergamin commented

You're right. InternalHttpClient is a package private class so you can't use it.
You really could argue that if an HttpClient is given to the HttpComponentsClientHttpRequestFactory (e.g. in the c'tor), it's supposed to be configured and the factory should not change the RequestContext anymore. Not sure how much client code will break with such a change though...

[spring-projects-issues]

Stéphane Nicoll commented

The factory allows to tune very specific details and it's true that it now overrides a bunch of unrelated customizations. We have a similar problem with HttpComponentsHttpInvokerRequestExecutor actually. The best of both worlds would be to merge the things that we want to customize over the things that were set by a configured HttpClient but unfortunately I don't see how we could do that with the current API. Maybe the best solution would be to opt-in for that behaviour with a boolean flag or separate that in two different factories. We still have users that have 4.3 on their classpath but pass a deprecated HttpClient instance as an argument so I am not sure how that would work if we choose this to be the default.

Another idea would be to pass the default RequestConfig alongside the HttpClient instance and we would use that to merge our settings with the ones coming from those defaults. The problem with this approach is that it's not "XML-friendly" and it feels weird to set both the HttpClient and the RequestConfig since the latter can be set in the former.

I have also reported that problem on the HttpComponents mailing list

Thoughts?

[spring-projects-issues]

Stéphane Nicoll commented

You could also do the following in the meantime to make sure your default config is used

class MyHttpComponentsClientHttpRequestFactory extends HttpComponentsClientHttpRequestFactory {

	private RequestConfig defaultRequestConfig;

	public MyHttpComponentsClientHttpRequestFactory(RequestConfig defaultRequestConfig) {
		this.defaultRequestConfig = defaultRequestConfig;
	}

	@Override
	protected HttpContext createHttpContext(HttpMethod httpMethod, URI uri) {
		HttpClientContext context = HttpClientContext.create();
		context.setRequestConfig(this.defaultRequestConfig);
		return context;
	}
}

[spring-projects-issues]

Stéphane Nicoll commented

I have actually pushed a proposal in my branch - We're still figuring out if we could manage that in a better way.

[spring-projects-issues]

Stéphane Nicoll commented

We're now creating a custom RequestConfig only if customizations are applied on the factory. Otherwise, the defaults of the http client are applied.

[spring-projects-issues]

Phil Krasko commented

Stephane - thanks for looking at this issue. It actually hit me 2 days ago and I ended up creating a default request config on a subclass of HttpComponentsClientHttpRequestFactory, ironically almost identical to what you posted above.

I needed to alter the CookieSpec on the RequestConfig to ignore all cookies.
The RequestConfig has about 14 configurable properties, where only 3 of them are actually configurable in the new HttpComponentsClientHttpRequestFactory.

The good news is if you have the defaults setup properly on the HttpClient then you can totally forget about RequestConfigs on a per request level, and you wouldn't need to call setConnectTimeout, setConnectionRequestTimeout, and setReadTimeout. My guess is this will be the 90% use case.

However, if one would want to alter the behavior per request the 3 exposed methods don't really do much justice compared to the 14 configurable properties on the RequestConfig. Would it make sense to have a protected method to access the RequestConfig with the URI and HttpMethod as arguments? That would really give a consumer full flexibility. It's just a thought I had while I was reading all the source the other night, and was actually going to file a ticket when I saw 4.1.4 was released and it addressed the very thing I was working on.

[spring-projects-issues]

Stéphane Nicoll commented

Hi Phil - we actually discussed this before the fix was applied. The problem is that RequestConfig can be set on an HttpClient but there is no way you can retrieve it so the only way really is to offer a way to pass a HttpClient and a RequestConfig which would be weird. Specifying a RequestConfig extra parameter would be as equally weird to me since you'd get the default of the client, the default of the factory and then the actual instance the request would be using....

We've reported that problem to the mailing list and HC 4.4 will have a public API for that. This actually reminds me I forgot to create the issue to take this new API into account. I've created #17184. In that case our local RequestConfig will be a merge between the RequestConfig of the HttpClient and any local customizations. You'll need Http Components 4.4. for this to work.

Does that make sense? Thanks for the feedback!

[spring-projects-issues]

Phil Krasko commented

Yes, that makes perfect sense.
In 4.3 you would be forced to supply both the default RequestConfig, and the HttpClient in the constructor, which is less than ideal. It's nice to know that 4.4 will fix the issue by providing an API. Thanks again.