Hi Chris and Sean,
There are a few feedback for the CSR approval. I updated to use
Optional<SSLSession> for the returned value. For more details, please
refer to:
https://bugs.openjdk.java.net/browse/JDK-8213161
http://cr.openjdk.java.net/~xuelei/8212261/webrev.03/
Please let me know if you are OK with this change.
Thanks,
Xuelei
On 11/2/2018 11:42 AM, Xuelei Fan wrote:
Thanks for the review and suggestions, Chris and Sean.
I just finalized the CSR.
Thanks,
Xuelei
On 11/2/2018 5:58 AM, Chris Hegarty wrote:
Thanks for the updates Xuelei, some minor comments inline..
On 1 Nov 2018, at 23:42, Xuelei Fan <xuelei....@oracle.com
<mailto: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
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.
