RFR: 8305763 : Parsing a URI with an underscore goes through a silent exception, negatively impacting performance [v2]

[Daniel Fuchs]

On Thu, 4 May 2023 20:31:32 GMT, Dhamoder Nalla <duke at openjdk.org> wrote:

>> Issue 8305763 : Using underscores in the name for a URI triggers a silent exception in the java standard library, which consumes 5% of the CPU.
>> 
>> Exception:
>> java.net.URISyntaxException: Illegal character in hostname at index N: xyz1_abcd.com
>>     at java.base/java.net.URI$Parser.fail(URI.java:2943)
>>     at java.base/java.net.URI$Parser.parseHostname(URI.java:3487)
>>     at java.base/java.net.URI$Parser.parseServer(URI.java:3329)
>>     
>>     This exception is silent and does not produce any messages, except for ODP profiler, there is no other evidence that it’s happening (the stack trace above was printed after changes to Java library). The reason for this is because of how the URI creation is implemented in the java.net.URI class. There are two paths for creating a valid URI, and one of them goes through an exception.
>> 
>> We can see that if parseServer fails, there is still a way the authority gets assigned and we don’t throw an exception from the method. This means, not being able to parse the server is ok and the exception is silenced. In our case, the server parsing fails because we find an illegal character, as only alphanumeric and dash characters are allowed.
>
> Dhamoder Nalla has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Addressing CR comments

I have given this a round of testing and I believe the logic is now right.
I'd suggest adding curly braces around else blocks in case of nested if statements though (see suggestion below) as it makes it easier to verify which belongs to which.

src/java.base/share/classes/java/net/URI.java line 3314:

> 3312:                             failExpecting("end of authority", q);
> 3313:                     } else
> 3314:                         authority = input.substring(p, n);

Suggestion:

                        } else {
                            failExpecting("end of authority", q);
                        }
                    } else {
                        authority = input.substring(p, n);
                    }

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

PR Review: https://git.openjdk.org/jdk/pull/13430#pullrequestreview-1441834815
PR Review Comment: https://git.openjdk.org/jdk/pull/13430#discussion_r1204123026