Re: Code review request, JDK-8040062 Need to add new methods in BaseSSLSocketImpl
There is a similar one for HttpsURLConnection, as we had problems in the past where the networking changes didn't update the SSL code. jdk/sun/net/www/protocol/https/HttpsURLConnection/CheckMethods.java Just a reminder to please include the JSSE reg tests when making Socket/HttpsURLConnection/etc. API changes or when changing code that is common to these methods. Thankfully we were running it anyway and was caught quickly. Thanks, Brad On 4/14/2014 6:45 AM, Chris Hegarty wrote: +1 . Thanks for updating this Xuelei. Nice test that caught this. -Chris. On 14/04/14 13:59, Sean Mullan wrote: Looks fine to me. Can you add a relates to link to JDK-8036979 and an appropriate noreg label? --Sean On 04/14/2014 06:04 AM, Xuelei Fan wrote: Hi, Please review the fix for JDK-8040062: http://cr.openjdk.java.net/~xuelei/8040062/webrev.00/ In JDK-8036979, there are news methods are added to java.net.Socket. These methods need to be overridden in SSL socket implementation, sun/security/ssl/BaseSSLSocketImpl.java. No new regression test. The existing test: sun/security/ssl/SSLSocketImpl/CheckMethods.java can be used to verify this fix. Thanks, Xuelei
Re: Code review request, JDK-8040062 Need to add new methods in BaseSSLSocketImpl
+1 . Thanks for updating this Xuelei. Nice test that caught this. -Chris. On 14/04/14 13:59, Sean Mullan wrote: Looks fine to me. Can you add a relates to link to JDK-8036979 and an appropriate noreg label? --Sean On 04/14/2014 06:04 AM, Xuelei Fan wrote: Hi, Please review the fix for JDK-8040062: http://cr.openjdk.java.net/~xuelei/8040062/webrev.00/ In JDK-8036979, there are news methods are added to java.net.Socket. These methods need to be overridden in SSL socket implementation, sun/security/ssl/BaseSSLSocketImpl.java. No new regression test. The existing test: sun/security/ssl/SSLSocketImpl/CheckMethods.java can be used to verify this fix. Thanks, Xuelei
Re: Code review request, JDK-8040062 Need to add new methods in BaseSSLSocketImpl
On 4/14/2014 8:59 PM, Sean Mullan wrote: > Looks fine to me. Can you add a relates to link to JDK-8036979 and an > appropriate noreg label? > Made the update in JBS. Thanks for the review. Xuelei > --Sean > > On 04/14/2014 06:04 AM, Xuelei Fan wrote: >> Hi, >> >> Please review the fix for JDK-8040062: >> >> http://cr.openjdk.java.net/~xuelei/8040062/webrev.00/ >> >> In JDK-8036979, there are news methods are added to java.net.Socket. >> These methods need to be overridden in SSL socket implementation, >> sun/security/ssl/BaseSSLSocketImpl.java. >> >> No new regression test. The existing test: >> >> sun/security/ssl/SSLSocketImpl/CheckMethods.java >> >> can be used to verify this fix. >> >> Thanks, >> Xuelei >>
Re: Code review request, JDK-8040062 Need to add new methods in BaseSSLSocketImpl
Looks fine to me. Can you add a relates to link to JDK-8036979 and an appropriate noreg label? --Sean On 04/14/2014 06:04 AM, Xuelei Fan wrote: Hi, Please review the fix for JDK-8040062: http://cr.openjdk.java.net/~xuelei/8040062/webrev.00/ In JDK-8036979, there are news methods are added to java.net.Socket. These methods need to be overridden in SSL socket implementation, sun/security/ssl/BaseSSLSocketImpl.java. No new regression test. The existing test: sun/security/ssl/SSLSocketImpl/CheckMethods.java can be used to verify this fix. Thanks, Xuelei
Code review request, JDK-8040062 Need to add new methods in BaseSSLSocketImpl
Hi, Please review the fix for JDK-8040062: http://cr.openjdk.java.net/~xuelei/8040062/webrev.00/ In JDK-8036979, there are news methods are added to java.net.Socket. These methods need to be overridden in SSL socket implementation, sun/security/ssl/BaseSSLSocketImpl.java. No new regression test. The existing test: sun/security/ssl/SSLSocketImpl/CheckMethods.java can be used to verify this fix. Thanks, Xuelei
