spring-cloud / spring-cloud-commons

Common classes used in different Spring Cloud implementations
Apache License 2.0
706 stars 705 forks source link

Provided custom ssl context gets overriden when using autoconfiguration for apache httpclient #945

Open krasowskiM opened 3 years ago

krasowskiM commented 3 years ago

Describe the bug Spring Cloud version: 3.0.1 In our project I had to provide our own keystore/truststore for spring-cloud-openfeign with apache httpclient. I wanted to leave as much as I could for autoconfig and I also saw that RegistryBuilder is available 👇

https://github.com/spring-cloud/spring-cloud-openfeign/blob/96268d16dafcabb38e9829b0b42d9f783d862852/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignAutoConfiguration.java#L165-L171

But it looks like the SSLConnectionSocketFactory gets overriden when invoking DefaultApacheHttpClientConnectionManagerFactory.newConnectionManager(...). What looks suspicious to me are lines: https://github.com/spring-cloud/spring-cloud-commons/blob/087b94a905c5428c89e4db22a6a284999d5d0a7d/spring-cloud-commons/src/main/java/org/springframework/cloud/commons/httpclient/DefaultApacheHttpClientConnectionManagerFactory.java#L77-L80 RegistryBuilder contains a map underneath, so when I would try to provide my own SSLConnectionSocketFactory then it gets overriden.

Not sure if this should be posted as a bug, but it gave me a bit of a head scratch. For now I've decided to extend the DefaultApacheHttpClientConnectionManagerFactory bean and pass to connection manager our own SSLContext. Maybe someone comes here and finds this helpful.

Sample I've created a test case for this issue here 👇 Hope the sample gives clear information about what happens. https://github.com/krasowskiM/spring-cloud-commons/commit/c986b81f918e736c0fdf7a7df6b7a85bc0faa2fe

saculo commented 2 years ago

What is needed to do there? @spencergibb

krasowskiM commented 2 years ago

So uhh... I have posted this quite some time ago and I have made some changes but I think they would have to be reviewed cautiously before going to main. Sorry guys, just forgot about this.

https://github.com/spring-cloud/spring-cloud-commons/compare/main...krasowskiM:custom-ssl-context-overriden