RFR: 8282395: URL.openConnection can throw IOOBE [v2]

[KIRIYAMA Takuya]

On Wed, 6 Jul 2022 14:39:37 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> @dfuch 
>> Could you tell me your opinion based on this information, please?
>
> Let me try to summarize: there are several things that we could do:
> 
>  1. nothing: continue throwing IIOBE
>  2. throw IAE instead of IIOBE, and let IAE trickle up the calling stack to the callers code
>  3. throw MUE instead of IIOBE, and then update all the places where ParseUtils::decode is called to either let the MUE trickle up the stack if possible, or deal with it if not.
>  4. throw IAE instead of IIOBE and somewhere up the stack catch that and transform it into MUE.
>    4.1. do the catch & transform in URL::openConnection when calling URLStreamHandler::openConnection
>    4.2. do the catch & transform in every protocol handler implementation that use ParseUtils::decode
>     
> Now let's see pro & cons:
>  1. is not ideal because it looks like a bug (and it is)
>  2. is expedient. There seems to be some precedent for that since ParseUtils::decode already throws IAE if it can't parse the port. However IAE is not specified to be thrown by openConnection. So maybe we should update the specification of URL::openConnection and URLStreamHandler::openConnection to allow for IAE. This is however a bit strange to use IAE because in this case the URL is indeed malformed, so MUE would be more appropriate.
>  3. has the advantage of being clear, and to force all callers of ParseUtils to deal with the situation. However, ParseUtils seems to be widely used, so each places where it's called needs to be examined, to see what would be the more appropriate solution in each case 
>  4. The advantage here is that callers of ParseUtils don't need to be changed if IAE is already appropriate for them. But those for which MUE would be more appropriate would benefit from having IAE transformed in MUE, which can be achieved by 4.1 or 4.2
>   4.1 the issue here is that URLStreamHandler::openConnection is not specified to throw IAE, yet we catch IAE that might be thrown by custom URLStreamHandler implementations not present in the JDK, where the IAE that they throw might have nothing to do with a Malformed URL. So we risk introducing regressions for applications that provide their own custom URLStreamHandlers implementations, and that might not expect the wrapping of the IAE they throw into MUE.  I believe that if we did that it would need to be specified (in URL::openConnection) and it would require a CSR.
>   4.2 we need to examine all protocol handler implementations present in the JDK and do the wrapping there. URLStreamHandler::openConnection is specified to throw IOException, so throwing MUE is within the bounds. However - if we don't have ParseUtils::decode throw MUE, we're not guaranteed not to miss some places.
>   
> I'd say from a consistency point of view, 3. is probably best. Then failing that, from a compatibility vs consistency point of view 4.2 might be our best shot. 2. might be envisaged, but maybe at the expense of logging a follow up issue to clean that up and later decide between 3. or 4.2.
> I would tend to rule out 4.1 due to the possible compatibility issue, and also because if URLStreamHandler::openConnection wanted to throw MUE it could, so there is no reason for doing 4.1.

Thank you for your valuable comments. I think 3 is a good idea.  I will try fix, but it takes some time to see the impact on many other methods.

-------------

PR: https://git.openjdk.org/jdk/pull/8155