> On May 13, 2015, 3:40 p.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/network/Selector.java, lines 
> > 286-291
> > <https://reviews.apache.org/r/33620/diff/5/?file=957067#file957067line286>
> >
> >     I am a bit confused how this works during the handshake. During the 
> > handshake, we will need to read and write data from the socket. However, 
> > the read/write part will be routed to connect() and then 
> > SSLTransportLayer.handshake(), not SSLTransportLayer.read/write. So how 
> > would the handshake complete?
> 
> Sriharsha Chintalapani wrote:
>     Channel.connect is the new method I added this is not similar to 
> SocketChannel.connect. Inside Channel.connect we go through 
> transportLayer.Handshake. 
>     Incase of SSLTransportLayer handshake it will go through read/write from 
> underlying socketChannel to complete the handshake. Once the handshake is 
> done every read/write will go through transportLayer.read/write.

My confusion is the following. SSLTransportLayer.handshake() will turn on the 
proper read/write interest bit. However, at this moment, the channel is not 
ready yet since the handshake hasn't completed. In Selector.poll(), we bypass 
the actual reads/writes when the channel is not ready (only going through the 
connect() part).


> On May 13, 2015, 3:40 p.m., Jun Rao wrote:
> > clients/src/main/java/org/apache/kafka/common/network/TransportLayer.java, 
> > line 98
> > <https://reviews.apache.org/r/33620/diff/5/?file=957068#file957068line98>
> >
> >     This seems to be ssl specific. Is this needed in the interface? It 
> > doesn't seem to be used.
> 
> Sriharsha Chintalapani wrote:
>     This is added to expose SSLSession for the principal builder, as there 
> are users(linkedin) who wants to read peer's X509Certificate. I am throwing 
> UnsupporterOperation for plaintext and returns a SSLSession if the 
> transportLayer happens to be SSLTransportLayer.

In the customized principal builder, can't they just cast transportLayer to 
SSLTransportLayer and then call sslSession()? Then sslSession() only needs to 
be local to SSLTransportLayer.


- Jun


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33620/#review83322
-----------------------------------------------------------


On May 12, 2015, 11:20 p.m., Sriharsha Chintalapani wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33620/
> -----------------------------------------------------------
> 
> (Updated May 12, 2015, 11:20 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1690
>     https://issues.apache.org/jira/browse/KAFKA-1690
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1690. new java producer needs ssl support as a client.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client. SSLFactory tests.
> 
> 
> KAFKA-1690. new java producer needs ssl support as a client. Added 
> PrincipalBuilder.
> 
> 
> Diffs
> -----
> 
>   build.gradle fef515b3b2276b1f861e7cc2e33e74c3ce5e405b 
>   checkstyle/checkstyle.xml a215ff36e9252879f1e0be5a86fef9a875bb8f38 
>   checkstyle/import-control.xml f2e6cec267e67ce8e261341e373718e14a8e8e03 
>   clients/src/main/java/org/apache/kafka/clients/ClientUtils.java 
> 0d68bf1e1e90fe9d5d4397ddf817b9a9af8d9f7a 
>   clients/src/main/java/org/apache/kafka/clients/CommonClientConfigs.java 
> cf32e4e7c40738fe6d8adc36ae0cfad459ac5b0b 
>   clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java 
> bdff518b732105823058e6182f445248b45dc388 
>   clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java 
> d301be4709f7b112e1f3a39f3c04cfa65f00fa60 
>   clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java 
> 8e336a3aa96c73f52beaeb56b931baf4b026cf21 
>   clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java 
> 187d0004c8c46b6664ddaffecc6166d4b47351e5 
>   clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java 
> c4fa058692f50abb4f47bd344119d805c60123f5 
>   clients/src/main/java/org/apache/kafka/common/network/Authenticator.java 
> PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/network/Channel.java 
> PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/network/DefaultAuthenticator.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/network/PlainTextTransportLayer.java
>  PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/network/SSLFactory.java 
> PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/network/SSLTransportLayer.java 
> PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/network/Selectable.java 
> b5f8d83e89f9026dc0853e5f92c00b2d7f043e22 
>   clients/src/main/java/org/apache/kafka/common/network/Selector.java 
> 57de0585e5e9a53eb9dcd99cac1ab3eb2086a302 
>   clients/src/main/java/org/apache/kafka/common/network/TransportLayer.java 
> PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/protocol/SecurityProtocol.java 
> dab1a94dd29563688b6ecf4eeb0e180b06049d3f 
>   
> clients/src/main/java/org/apache/kafka/common/security/auth/DefaultPrincipalBuilder.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java
>  PRE-CREATION 
>   
> clients/src/main/java/org/apache/kafka/common/security/auth/PrincipalBuilder.java
>  PRE-CREATION 
>   clients/src/main/java/org/apache/kafka/common/utils/Utils.java 
> f73eedb030987f018d8446bb1dcd98d19fa97331 
>   clients/src/test/java/org/apache/kafka/common/network/EchoServer.java 
> PRE-CREATION 
>   clients/src/test/java/org/apache/kafka/common/network/SSLFactoryTest.java 
> PRE-CREATION 
>   clients/src/test/java/org/apache/kafka/common/network/SSLSelectorTest.java 
> PRE-CREATION 
>   clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java 
> d5b306b026e788b4e5479f3419805aa49ae889f3 
>   clients/src/test/java/org/apache/kafka/common/utils/UtilsTest.java 
> 2ebe3c21f611dc133a2dbb8c7dfb0845f8c21498 
>   clients/src/test/java/org/apache/kafka/test/TestSSLUtils.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/33620/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sriharsha Chintalapani
> 
>

Reply via email to