apache
fixing TLS configuration for http clients
As reported in this issue there is an issue where the created HttpClients do not respect the TLS configuration. This fixes that bug.
Codecov Report
Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
Project coverage is 61.96%. Comparing base (
59551e4) to head (d290c14). Report is 1099 commits behind head on master.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| ...http/PoolingHttpClientConnectionManagerHelper.java | 75.00% | 1 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #13477 +/- ##
============================================
+ Coverage 61.75% 61.96% +0.21%
+ Complexity 207 198 -9
============================================
Files 2436 2556 +120
Lines 133233 140715 +7482
Branches 20636 21861 +1225
============================================
+ Hits 82274 87197 +4923
- Misses 44911 46887 +1976
- Partials 6048 6631 +583
| Flag | Coverage Δ | |
|---|---|---|
| custom-integration1 | <0.01% <0.00%> (-0.01%) |
:arrow_down: |
| integration | <0.01% <0.00%> (-0.01%) |
:arrow_down: |
| integration1 | <0.01% <0.00%> (-0.01%) |
:arrow_down: |
| integration2 | 0.00% <0.00%> (ø) |
|
| java-11 | 61.92% <85.71%> (+0.21%) |
:arrow_up: |
| java-21 | 61.84% <85.71%> (+0.21%) |
:arrow_up: |
| skip-bytebuffers-false | 61.95% <85.71%> (+0.21%) |
:arrow_up: |
| skip-bytebuffers-true | 35.06% <57.14%> (+7.33%) |
:arrow_up: |
| temurin | 61.96% <85.71%> (+0.21%) |
:arrow_up: |
| unittests | 61.96% <85.71%> (+0.21%) |
:arrow_up: |
| unittests1 | 46.49% <80.00%> (-0.40%) |
:arrow_down: |
| unittests2 | 27.68% <28.57%> (-0.05%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@sullis is working on migrating from HttpClient4 to HttpClient5 in #13222. I guess these 2 PRs are related, and I assume HttpClient5 probably has fixed the bug described in #13431
@Jackie-Jiang I am not fully sure that the upgrade will actually solve for this problem. I looked through the source of the http client and it seems like it would still be constructing the socket using the default system configuration (according to this)
@Jackie-Jiang made the adjustment. Version 5 should fix the issue where the connection factory is not being picked up from the PoolingHttpClientConnectionManager so this has become a simpler change.
@Jackie-Jiang sorry for the bump but would really like this to get in. We've been running this internally for quite a while now