Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issue #580 : support for configured TLS and cipherSuites #628

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pheenomenon
Copy link
Contributor

This fix is for #580
This PR has fix as per #2 below.

Few options to fix this are:
1] stop setting ConnectionManager and set useSystemProperties() while building HttpClients, in which case HttpClientBuilder will invoke the code to read systemProperties.
reuseStrategyCopy = new SSLConnectionSocketFactory((SSLSocketFactory)SSLSocketFactory.getDefault(), keepAliveStrategyCopy, targetAuthStrategyCopy, (HostnameVerifier)proxyAuthStrategyCopy);

2] Another approach is to update EwsSSLProtocolSocketFactory.build() to support the call to SSLConnectionSocketFactory constructor using SSL protocol and cipherSuite.

@codecov-io
Copy link

codecov-io commented Aug 18, 2017

Codecov Report

Merging #628 into master will increase coverage by <.01%.
The diff coverage is 33.33%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #628      +/-   ##
============================================
+ Coverage     10.43%   10.43%   +<.01%     
- Complexity        0      471     +471     
============================================
  Files           550      550              
  Lines         20436    20444       +8     
  Branches       2626     2084     -542     
============================================
+ Hits           2132     2134       +2     
- Misses        18151    18155       +4     
- Partials        153      155       +2
Impacted Files Coverage Δ Complexity Δ
...ervices/data/core/EwsSSLProtocolSocketFactory.java 59.09% <33.33%> (-19.49%) 6 <1> (+6)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e1ba52...de58c02. Read the comment docs.

@pheenomenon
Copy link
Contributor Author

@serious6 @vbauer wondering if you had some feedback on this fix.

@pheenomenon
Copy link
Contributor Author

@serious6 @vbauer @vboctor I have squashed the separate checkins to a single one. Please let me know if you recommend anything more on this.

@pheenomenon
Copy link
Contributor Author

Hello experts, @serious6 @vbauer @vboctor any inputs here will be appreciated.

@pheenomenon
Copy link
Contributor Author

Hello experts, @serious6 @vbauer @vboctor any inputs on when you guys plan to release the latest build.

2 similar comments
@pheenomenon
Copy link
Contributor Author

Hello experts, @serious6 @vbauer @vboctor any inputs on when you guys plan to release the latest build.

@pheenomenon
Copy link
Contributor Author

Hello experts, @serious6 @vbauer @vboctor any inputs on when you guys plan to release the latest build.

@pheenomenon pheenomenon closed this Sep 7, 2017
@pheenomenon pheenomenon reopened this Sep 7, 2017
@kentongray
Copy link
Contributor

so sad having such great prs go unnoticed, can you please allow a 3rd party maintainer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants