[
https://issues.apache.org/jira/browse/THRIFT-106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12921562#action_12921562
]
Bryan Duxbury commented on THRIFT-106:
--------------------------------------
A few comments:
* The spacing is off. We use 2-space indentation.
* The addition of getServerSocket to TServerSocket seems unnecessary, since
it's not used anywhere.
* Why override TSocket.open() at all in TSSLSocket? TSocket throws an exception
if it's already open, but TSSLSocket will silently continue. Is this a
necessary/intended semantic change? If we don't need that change, do we need a
TSSLSocket at all?
* TSSLServerSocket also seems unnecessary.
* Would it be possible to get a separate unit test for TSSLSocketFactory? Maybe
just set up a simple server-client pair and push some bytes around? The
tutorial stuff is great, but it's not going to be part of our standard test
suite, so we could break it by accident.
* Hate to sounds pedantic, but do you mind using ifs with curly braces, even
though the then clause is single-line? Just for readability/consistency.
> TSSLServerSocket
> ----------------
>
> Key: THRIFT-106
> URL: https://issues.apache.org/jira/browse/THRIFT-106
> Project: Thrift
> Issue Type: New Feature
> Components: Java - Library
> Environment: n/a
> Reporter: rico sec
> Priority: Trivial
> Attachments: java-ssl.patch,
> Sample-keystore-for-tests-and-tutorial.patch, ssl.patch,
> TSSLServerSocket-TSSLSocket-and-factory.patch,
> Updated-tutorial-with-ssl-sample.patch
>
> Original Estimate: 6h
> Remaining Estimate: 6h
>
> SSL Connection w/ autogenerated self signed x509 certs seems to be the state
> of the art for rpc layers.
> if thrift had one ...that would be very good.
> http://java.sun.com/javase/6/docs/technotes/guides/security/jsse/JSSERefGuide.html
> if someone does this pls ping/email me, I will do some testing and write a
> simple key mgmt utility.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
