Copilot commented on code in PR #3764:
URL: https://github.com/apache/celeborn/pull/3764#discussion_r3636486565
##########
common/src/main/java/org/apache/celeborn/common/network/client/TransportClientFactory.java:
##########
@@ -306,7 +306,9 @@ private TransportClient internalCreateClient(
new ChannelInitializer<SocketChannel>() {
@Override
public void initChannel(SocketChannel ch) {
- TransportChannelHandler clientHandler =
context.initializePipeline(ch, decoder, true);
+ TransportChannelHandler clientHandler =
+ context.initializePipeline(
+ ch, decoder, true, address.getHostString(),
address.getPort());
clientRef.set(clientHandler.getClient());
Review Comment:
The new SNI behavior (threading peer host/port into SSLEngine creation) is
not covered by tests, so regressions (e.g., accidentally dropping
peerHost/peerPort in the call chain) would be hard to catch. Consider adding a
focused test in the existing SSL test suites (e.g.,
common/src/test/java/.../ssl/SslConnectivitySuiteJ) that verifies client-side
engines are created with the expected peer host/port
(engine.getPeerHost()/getPeerPort()), and/or that SNI is present during
handshake if you have a way to observe it on the server side.
--
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]
