On 08/08/2018 07:46 AM, Sean Mullan wrote:
On 8/7/18 8:05 PM, Xuelei Fan wrote:
Hi Tony,
The Specification section looks more like the implementation details.
We may change the implementation details again in the future. It may
be more suitable to move it to the Solution section, or just remove it.
I agree, I would omit the diffs and put N/A for the Specification section.
In the Specification section, I may just say something like, "No APIs
changes. The SunJSSE provider is updated to throw
UnsupportedOperationException if SSLContext.SSLServerSocketFactory()
or SSLContext.SSLSocketFactory() get called for DTLS algorithms
SSLContext".
I think the CSR should also say that the SunJSSE implementation of the
engineGetSocketFactory and engineGetServerSocketFactory methods of
SSLContextSpi have been changed to throw UnsupportedOperationException.
That is the specific API behavior change.
A few other comments on the CSR:
"SSLContext.getSSLSocketFactory() and
SSLContext.getSSLServerSocketFactory()"
Typo, there is no "SSL" in the method names.
The Compatibility Risk field has several typos ("there" -> "their", "how
-> now", ...) and is a bit hard to understand. Wasn't TLS being used
before instead of DTLS in certain scenarios? Because the API silently
passed in certain cases, and now we will be throwing an Exception, maybe
it might be better to say the risk is low.
I fixed the typos and redid the compatibility part that hopefully is
more to the point.
In the Summary section, it says "..., but it is not documented." I
suggest opening a docs bug to improve the DTLS documentation so that it
is more clear this scenario is not supported.
I think the Interface Kind should be Java API since we are changing the
behavior of an implementation of a standard API. I asked Joe Darcy this
question yesterday, and he agreed.
I thought about API, but since it was a behavior change, API didn't
sound completely correct. But that's fine if that's what he wants.
--Sean
Thanks,
Xuelei
On 8/7/2018 4:14 PM, Anthony Scarpino wrote:
Hi Xuelei,
I have updated the csr and I believe I have addressed your comments.
thanks
Tony
On 08/07/2018 01:43 PM, Xuelei Fan wrote:
Hi Tony,
Would you mind make it clear that this impact the JDK JSSE provider
only? Third party's provider may be able to support DTLS with
SSLSocket.
I think there may be no specification change. The
SSLContext.getServerSocketFactory() and
SSLContext.getSocketFactory() defines the spec if the algorithm is
not supported by the underlying provider,
"UnsupportedOperationException - if the underlying provider does not
implement the operation.". I may prefer to make it clear that this
is just a behavior change of the JDK JSSE provider (SunJSSE). The
SunJSSE provider now throws UnsupportedOperationException for
creating SSL(Server)SocketFactory with DTLS SSLContext, because it
does not actually support DTLS SSLSocket.
In Solution section, "Throwing a UnsupportedOperationException when
getting a socket from the SSLServerSocketFactory or SSLSocketFactory
for DTLS." I guess you meant, throwing a UOE when calling
SSLContext.getServerSocketFactory() and SSLContext.getSocketFactory()?
Thanks,
Xuelei
On 8/7/2018 12:17 PM, Anthony Scarpino wrote:
I need a review of a CSR for SSLSocket should throw an exception
when configuring DTLS. We are targeting this for 12 right now.
https://bugs.openjdk.java.net/browse/JDK-8209031
thanks
Tony
