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

[Daniel Fuchs]

On Tue, 7 Jun 2022 07:00:17 GMT, KIRIYAMA Takuya <duke at openjdk.java.net> wrote:

>> I fixed sun.net.www.ParseUtil.decode().
>> 
>> ParseUtil.decode() always tries to decode after parsing '%', so if '%' is located at the end of the String, IndexOutOfBoundsException is thrown. Also, if '%' is shown after decodable string and following string is not decodable (e.g: "%25%s%G1"), ParseUtil.decode() throws IllegalArgumentException.
>> 
>> But URL standard says below (https://url.spec.whatwg.org/#percent-decode).
>> 
>> 
>> Otherwise, if byte is 0x25 (%) and the next two bytes after byte in input are not in the ranges 
>> 0x30 (0) to 0x39 (9), 0x41 (A) to 0x46 (F), and 0x61 (a) to 0x66 (f), all inclusive, append byte to output.
>> 
>> 
>> So, there should be used isEscaped() to judge to decode.
>> 
>> Would you please review this fix?
>
> KIRIYAMA Takuya has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
> 
>  - #8282395 URL.openConnection can throw IOOBE
>  - Merge branch 'master' into 8282395
>  - 8282395: URL.openConnection can throw IOOBE

Changes requested by dfuchs (Reviewer).

src/java.base/share/classes/java/net/URL.java line 1101:

> 1099:             throw new MalformedURLException(e.getMessage());
> 1100:         }
> 1101:         return url;

Please revert the changes in java.net.URL. It's not the proper place to do the conversion. If a conversion is needed, it should take place in the handler.
One possible experiment could be to  change `ParseUtil::decode` to declare `throws MalformedURLException` and see what no longer compiles. If compilation passes - then maybe that's the way to go. If it doesn't - then we could analyze what to do on a case-by-case basis.

src/java.base/share/classes/sun/net/www/ParseUtil.java line 204:

> 202:                     bb.put(unescape(s, i));
> 203:                 } catch (NumberFormatException | IndexOutOfBoundsException e) {
> 204:                     throw new IllegalArgumentException("Malformed escape pair: " + s);

I would suggest turning the assert at line 200 into a proper bound check and throw IllegalArgumentException if `n - i >= 2`

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

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