Code Review Request: 7171415: java.net.URI.equals/hashCode not consistent for some URIs

[Kurchi Subhra Hazra]

Thanks a lot for your comments David.

On 1/8/13 6:39 PM, David Schlosnagle wrote:
> Hi Kurchi,
>
> I had a couple quick comments:
>
> On line 1756, you might want to allocate the StringBuilder normalizedS
> = new StringBuilder(s.length()); to avoid resizing during the
> appending.
- I am now wondering if I should directly calculate the hashCode instead of
using a StringBuilder.

>
> On line 1754 and 1759, is it worth restructuring the logic to avoid
> performing indexOf twice to find the first '%'? This might be
> premature optimization as hopefully the URIs are short and that
> indexOf would be replaced with an intrinsic if hot enough.
It is possible to store the value of first occurrence of '%' in a 
variable, but I was
trying to keep the case with no '%' clutter-free.

- Kurchi

>
> Thanks,
> Dave
>
> On Tue, Jan 8, 2013 at 8:19 PM, Kurchi Hazra
> <kurchi.subhra.hazra at oracle.com>  wrote:
>> Hi,
>>
>>      According to RFC 3986[1], hexadecimal digits encoded by a '%' should be
>> case-insensitive, for example,%A2 and %a2 should be
>> considered equal. Although, URI.equals() does take this into consideration,
>> the implementation of URI.hashCode() does not and
>> returns different hashcodes for two URIs that are similar in all respects
>> except for the case of the percent-encoded hexadecimal digits.
>> This fix attempts to construct a normalized string from the string
>> representing a component before calculating its hashCode.
>> I converted to upper case for the normalization(and not lower case) as
>> required by [1].
>>
>>      For testing the fix, I added an additional test scenario to an existing
>> test (jdk/test/java/net/URI/Test.java). While I was there, I also made
>> minor changes to the test so that it does not produce rawtype and other lint
>> warnings.
>>
>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7171415
>> Webrev: http://cr.openjdk.java.net/~khazra/7171415/webrev.00/
>>
>> URI.compareTo() still suffers from the same problem - I am not sure if it
>> should be dealt with as a separate bug.
>>
>> Thanks,
>> Kurchi
>>
>> [1] http://tools.ietf.org/html/rfc3986#section-6.2.2.1