Both (JDK 7u2 and JDK 8) look fine to me. Thanks for the update. Xuelei
On 10/18/2011 8:41 AM, Brad Wetmore wrote: > (Max/Valerie/Sean, I am hoping to get this into 7u2, thus need a second > codereview. The code itself is pretty straightforward.) > > Xuelei wrote: >> The change of SSLEngineImpl looks fine to me. >> >> In the new regression test: > > Changes made. Thanks for the feedback, Xuelei. I tweaked things a bit > even more in the test case only. Now, I alternate between the client > and the server initiating the closing. > > http://cr.openjdk.java.net/~wetmore/7031830/ > > See the .01 versions. > > Brad > > > > > On 10/14/2011 7:23 PM, Xuelei Fan wrote: >> The change of SSLEngineImpl looks fine to me. >> >> In the new regression test: >> >> A. class name >> >> 28 * @run main/othervm SSLEngineTemplate >> 29 * >> 30 * SunJSSE does not support dynamic system properties, no way to >> re-use >> 31 * system properties in samevm/agentvm mode. >> >> I think the class name should be SSLEngineBadBufferArrayAccess. >> - 28 * @run main/othervm SSLEngineTemplate >> + 28 * @run main/othervm SSLEngineBadBufferArrayAccess >> >> B. a minor comment, reuse the serverEngine.wrap() logic. >> Is it possible to remove line 286 ~ 297, and have >> "while(!isEngineClosed())" handle the close notify? So that the server >> engine would be able to handle the client close_notify response. >> >> boolean exchangeHasDone = false; >> while (!isEngineClosed()) { >> 283 if (!exchangeHasDone&& clientMsg.length == serverIn.position()) { >> // sanity check >> ... >> serverEngine.closeOutbound(); >> exchangeHasDone = true. >> } >> } catch (...) { >> >> >> Xuelei >> >> On 10/15/2011 8:52 AM, Brad Wetmore wrote: >>> Hi Xuelei, >>> >>> I need code reviews for the bug I mentioned to you earlier. >>> >>> 7031830: bad_record_mac failure on TLSv1.2 enabled connection with >>> SSLEngine >>> >>> The MAC calculation was summing the wrong data range when using >>> non-direct byte buffers and TLS1.1/1.2. >>> >>> The new regression test will now interop-test SSLEngine with SSLSockets >>> using both direct and non-direct ByteBuffers, over SSLv3, TLSv1, >>> TLSv1.1, and TLSv1.2. >>> >>> http://cr.openjdk.java.net/~wetmore/7031830/ >>> >>> I plan to push this to both JDK 8 and 7u2, so there are 2 webrevs there. >>> They should be the same. >>> >>> Brad >>
