Thanks for the updates Xuelei, some minor comments inline.. > On 1 Nov 2018, at 23:42, Xuelei Fan <xuelei....@oracle.com> wrote: > > On 11/1/2018 11:24 AM, Sean Mullan wrote: >> On 10/31/18 11:52 AM, Chris Hegarty wrote: >>> Xuelei, >>> >>> On 30/10/18 20:55, Xuelei Fan wrote: >>>> Hi, >>>> >>>> For the current HttpsURLConnection, there is not much security parameters >>>> exposed in the public APIs. An application may need richer information >>>> for the underlying TLS connections, for example the negotiated TLS >>>> protocol version. >>>> >>>> Please let me know if you have concerns to add a new method >>>> HttpsURLConnection.getSSLSession() and deprecate the duplicated methods, >>>> by the end of Nov. 2, 2018. >>>> >>>> Here is the proposal: >>>> https://bugs.openjdk.java.net/browse/JDK-8213161 >> Are there any security issues associated with returning the SSLSession, >> since it is mutable? > It should be fine. The update APIs of the session (invalidating, bind > values) does not impact the connection.
Alternatively, as is done in the new HTTP Client, an immutable SSLSession instance can be returned. >> + * SHOULD override this method with appropriate >> implementation. >> s/appropriate/an appropriate/ >> I would probably not capitalize "SHOULD" and just say "should". "SHOULD" is >> more common in RFCs. I don't see that much in javadocs. >> + * @implNote The JDK Reference Implementation supports this operation. >> + * As an application may have to use this operation for more >> + * security parameters, it is recommended to support this >> + * operation in all implementations. >> I think it should be obvious that the JDK implementation would override this >> method so not sure that first sentence is necessary. The other sentence >> seems like it could be combined with the previous sentence, ex: >> "Subclasses should override this method with an appropriate implementation >> since an application may need to access additional parameters associated >> with the SSL session." > Updated accordingly, in the CSR and webrev: > https://bugs.openjdk.java.net/browse/JDK-8213161 > <https://bugs.openjdk.java.net/browse/JDK-8213161> The CSR looks good. I made a few minor edits to the verbiage and added myself as reviewer. The title will need to be updated to reflect the addition of the new method in SecureCacheResponse. Maybe: "Add SSLSession accessors to HttpsURLConnection and SecureCacheResponse" -Chris.
