spring-projects / spring-vault

Provides familiar Spring abstractions for HashiCorp Vault
https://spring.io/projects/spring-vault
Apache License 2.0
283 stars 186 forks source link

Read-Timeout not applied with Apache Http Components and no-SSL #861

Closed p120ph37 closed 5 months ago

p120ph37 commented 7 months ago

When using Apache Http Components with an http (non-SSL) Vault URI, the read-timeout option is not applied to the HttpClientBuilder. This results in the Apache-default of 3-minutes always being used, regardless of the settings provided via Spring Boot.

I've created a trivial example project to demonstrate the issue here: https://github.com/p120ph37/spring-vault-test

I believe the fix would be to add .setResponseTimeout(Timeout.ofMilliseconds(options.getReadTimeout().toMillis())) here: https://github.com/spring-projects/spring-vault/blob/542786151495f1bde4d51434c0067705e58f62f1/spring-vault-core/src/main/java/org/springframework/vault/client/ClientHttpRequestFactoryFactory.java#L329-L333

I have prototyped this fix in the FixedClientFactoryWrapper in my test suite -- test that fix by uncommenting this line: https://github.com/p120ph37/spring-vault-test/blob/0ffcea4bc2d7401721f29b419c9596c91827023b/src/test/java/spring/vault/test/TimeoutTests.java#L43

p120ph37 commented 7 months ago

As a temporary workaround for Spring Boot applications that need to support timeouts on HTTP Vault requests, adding this bean to your application will force the use of the JDK request factory rather than the Apache one:

        // This is a workaround for https://github.com/spring-projects/spring-vault/issues/861
        @Bean
        @Order(Ordered.HIGHEST_PRECEDENCE)
        org.springframework.vault.client.RestTemplateCustomizer restTemplateCustomizer(VaultProperties vp) {
            return rt -> {
                if(vp.getSsl().getKeyStore() == null && vp.getSsl().getTrustStore() == null) {
                    SimpleClientHttpRequestFactory rf = new SimpleClientHttpRequestFactory();
                    rf.setConnectTimeout(vp.getConnectionTimeout());
                    rf.setReadTimeout(vp.getReadTimeout());
                    rt.setRequestFactory(rf);
                }
            };
        }
mp911de commented 6 months ago

Thanks for the report. We should fix that issue in HttpComponents.getHttpClientBuilder(…). Since you already had a look into the issue, care to submit a pull request?

Entea commented 5 months ago

I've stumbled upon the same problem and found this issue. I've submitted a PR