xiangfu0 commented on code in PR #20136:
URL: https://github.com/apache/pinot/pull/20136#discussion_r4453872250


##########
pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/DefaultSslContextProvider.java:
##########
@@ -45,6 +53,14 @@ public DefaultAsyncHttpClientConfig.Builder 
configure(DefaultAsyncHttpClientConf
       builder.setSslEngineFactory((config, peerHost, peerPort) -> {
         SSLEngine engine = sslContext.createSSLEngine(peerHost, peerPort);
         engine.setUseClientMode(true);
+        // A JDK SSLEngine verifies the peer hostname only when an endpoint 
identification algorithm is set. This
+        // factory replaces AsyncHttpClient's own engine factory, so apply the 
same check it would, honoring
+        // disableHttpsEndpointIdentificationAlgorithm.
+        if (!config.isDisableHttpsEndpointIdentificationAlgorithm()) {

Review Comment:
   **Compatibility (MINOR, owner decision):** this branch is broader than 
"custom SSLContext". `withConnectionProperties` (`scheme=https`, 
`JsonAsyncHttpPinotClientTransportFactory.java:91-92`) and 
`PinotDriver.java:103-104` always pass a non-null context, so every HTTPS 
Java/JDBC broker query now verifies the hostname.
   
   Brokers register IPs by default (`pinot.set.instance.id.to.hostname=false` 
outside Helm). So brokers whose certs have only DNS SANs will fail after a 
client-only upgrade, and the only opt-out is the JVM-wide AHC property, which 
also turns off verification for every other AHC client in the JVM. I suggest a 
release-note/upgrade entry, and possibly a Pinot-scoped client/JDBC opt-out.
   
   ---
   AI-generated review via IntentLab ยท Model: Claude Opus 5.5 (claude-opus-5-5).



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to