andreigurau commented on code in PR #24229:
URL: https://github.com/apache/beam/pull/24229#discussion_r1026836133
##########
sdks/java/io/splunk/src/main/java/org/apache/beam/sdk/io/splunk/HttpEventPublisher.java:
##########
@@ -349,14 +378,24 @@ private CloseableHttpClient getHttpClient(
? NoopHostnameVerifier.INSTANCE
: new DefaultHostnameVerifier();
- SSLContextBuilder sslContextBuilder = SSLContextBuilder.create();
+ SSLContext sslContext = SSLContextBuilder.create().build();
if (disableCertificateValidation) {
LOG.info("Certificate validation is disabled");
- sslContextBuilder.loadTrustMaterial((TrustStrategy) (chain,
authType) -> true);
+ sslContext =
+ SSLContextBuilder.create()
Review Comment:
I think creating another `SSLContextBuilder` in this if block is the
cleanest solution I can think of. The thing is that in the `else if
(rootCaCertificate != null)` block, we are expecting the sslContext to already
be defined so we can call it's `init` function. If we are going to use the same
SSLContextBuilder instead thoughout the whole if/else if blocks, the code might
look something like this
```
SSLContextBuilder sslContextBuilder = SSLContextBuilder.create();
SSLContext sslContext;
if(disableCertificateValidation) {
sslContextBuilder.loadTrustMaterial((TrustStrategy) (chain, authType) ->
true);
sslContext = sslContextBuilder.build();
} else if (rootCaCertificate != null) {
...
sslContext = sslContextBuilder.build();
sslContext.init(...)
} else {
sslContext = sslContextBuilder.build();
}
```
In the above snippet, I'm calling ``sslContext =
sslContextBuilder.build();`` multiple times. Because of that, I think it's just
easier and cleaner to use another SSLContextBuilder and build the SSLContext in
the if block
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
