Hello, just a Nit, but should this be clientNet/AppOutbound instead of Inbound (or maybe use Response instead?) in ClientHelloInterOp#run Is it Ok that there is no asserts? What cipher would be picked for example? ClientHelloInterOp In 271+304 I would use try-with-resource or not close the in memory stream at all. Also while it is possible to use getBytes() I used to use StandardCharset.ASCII to make it explicit. Typo in 388 comment (delegated)
Gruss Bernd -- http://bernd.eckenfels.net On Thu, Nov 10, 2016 at 4:13 AM +0100, "Xuelei Fan" <xuelei....@oracle.com> wrote: Thanks for review. I will go ahead and push the changeset. The latest update is: http://cr.openjdk.java.net/~xuelei/8169362/webrev.01/ Xuelei On 11/10/2016 9:41 AM, Bradford Wetmore wrote: > Hi Xuelei, > > Nothing major, mostly nits and some Netbeans suggestions. > > On 11/9/2016 5:17 AM, Xuelei Fan wrote: >> Hi, >> >> Please review this test enhancement: >> >> http://cr.openjdk.java.net/~xuelei/8169362/webrev.00/ >> >> This update adds a interop new test case with Chrome ClientHello message. > > ClientHelloInterOp.java > ======================= > 34: import not needed. > > 43/92/138/178: could be final. > > 265: is value of null is never used. > > 290: Generally prefer () groupings if you can. > > 316: Nit, you could combine: > > Certificate[] chain = new Certificate [] { keyCert }; > > 200/211/285: jcheck problems. > > You could make a lot of these methods/fields private unless you plan to > use them later. > > > ClientHelloChromeInterOp.java > ============================= > 37: import not needed. > > 46: could be final. > > I didn't check the format of the ClientHello, assuming it does what you > need. Do you need me to do a secondary check? > > Ok otherwise, > > Brad > >
