[GitHub] [pulsar] michaeljmarshall commented on a diff in pull request #17543: [fix][admin] Add SNI header when tlsHostnameVerification is not enabled

[GitBox <gi...@apache.org>]

michaeljmarshall commented on code in PR #17543:
URL: https://github.com/apache/pulsar/pull/17543#discussion_r966588366


##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/HttpClient.java:
##########
@@ -144,6 +145,10 @@ public boolean keepAlive(InetSocketAddress remoteAddress, Request ahcRequest,
 
                 confBuilder.setUseInsecureTrustManager(conf.isTlsAllowInsecureConnection());
                 confBuilder.setDisableHttpsEndpointIdentificationAlgorithm(!conf.isTlsHostnameVerificationEnable());
+                if (!conf.isTlsHostnameVerificationEnable()) {
+                    confBuilder.setSslEngineFactory(new WithSNISslEngineFactory(serviceNameResolver
+                            .resolveHostUri().getHost()));
+                }

Review Comment:
   Looks like this declaration needs to be moved up like it is in the `AsyncHttpConnector` so that it does not override the `sslEngineFactory` when using a keystore.



-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org