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:
Honestly, I think redefining `sslContext` 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, 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]
