<html><head></head><body><div>Hello, just a Nit, but should this be clientNet/AppOutbound instead of Inbound (or maybe use Response instead?) in ClientHelloInterOp#run</div><div><br></div><div>Is it Ok that there is no asserts? What cipher would be picked for example?</div><div><br></div><div>ClientHelloInterOp </div><div><br></div><div>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.</div><div><br></div><div>Typo in 388 comment (delegated)<br><br><div class="acompli_signature">Gruss<br>Bernd<br>-- <br><a dir="ltr" href="http://bernd.eckenfels.net" x-apple-data-detectors="true" x-apple-data-detectors-type="link" x-apple-data-detectors-result="0">http://bernd.eckenfels.net</a></div><br></div><br><br><br>
<div class="gmail_quote">On Thu, Nov 10, 2016 at 4:13 AM +0100, "Xuelei Fan" <span dir="ltr"><<a href="mailto:xuelei.fan@oracle.com" target="_blank">xuelei.fan@oracle.com</a>></span> wrote:<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="3D"ltr"">
<pre>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
>
>
</pre>
</div>
</blockquote>
</div>
</body></html>