+1 to everything Sean said, and just to add ...
This change will prevent an SSLContext from giving out socket
factories when it has been configured with DTLS. What about
SSL[Server]SocketFactory::getDefault when the default SSL
context is DTLS? I don’t see that getDefault can throw an UOE,
should it? Or should / could this be resolved at the socket
factory level, when trying to create new sockets rather than at
the factory retrieval time?
-Chris.
> On 8 Aug 2018, at 15:46, Sean Mullan <sean.mul...@oracle.com> 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.
>
> 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.
>
> --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
>>>>>
>>>
