7025238 : HttpURLConnection does not handle URLs with an empty path component

[Kurchi Hazra]

Hi Andreas,

I was not talking about changing the URL implementation, but just 
pointing out that your change doesn't cause incompatibilities in the way we
store elements in the cookiehandler, which is good.

I ran all networking tests on all supported platforms, and things look 
green. I'll push the fix for you in sometime.

Thanks,
Kurchi


On 6/19/2013 11:29 AM, Andreas Rieber wrote:
> Hi Kurchi,
>
> to change the path in URL.java would not be a good idea, it supports 
> many other protocols as well and what i can read out of the URL 
> specification is that path can be empty. True, ParserUtil.toUri() uses 
> the path and query elements separate. Here in HttpClient.java i guess 
> it saves at least one null check ;-)
>
> cheers
> Andreas
>
>
> On 19.06.13 20:02, Kurchi Hazra wrote:
>> Hi Andreas,
>>
>>   I looked at your changes, and they look good to me. Although we are 
>> not changing the path of the URL itself (it is not modifiable
>> too), but from what I see the only other relevant place where 
>> URL.path is logically used in http client is in ParseUtil.toURI(), 
>> which basically does
>> the same thing as your fix.
>>
>>  I'll run the fix against all networking tests on all platforms and 
>> let you know how things look.
>>
>> Thanks!
>> Kurchi
>>
>> On 6/19/2013 7:14 AM, Andreas Rieber wrote:
>>> Hi Chris and Kurchi,
>>>
>>> i have updated and rerun the test (removed the "@run main/othervm 
>>> B7025238").
>>>
>>> New webrev is here:
>>> http://cr.openjdk.java.net/~arieber/7025238/webrev.01/
>>>
>>> thanks
>>> Andreas
>>>
>>>
>>> On 19.06.13 15:33, Chris Hegarty wrote:
>>>> Hi Andreas,
>>>>
>>>> On 18/06/2013 20:19, Andreas Rieber wrote:
>>>>> Hi,
>>>>>
>>>>> i am looking for a sponsor of this issue.
>>>>>
>>>>> The bug is here:
>>>>> http://bugs.sun.com/view_bug.do?bug_id=7025238
>>>>>
>>>>> First i verified that the problem still exists. Then i checked the
>>>>> problem against some other web servers. Apache handles a missing 
>>>>> "/" in
>>>>> the path. Tomcat, Microsoft-HTTPAPI/2.0 and the openjdk build in http
>>>>> server behave with same response: 400 Bad Request.
>>>>
>>>> Nice. Thanks for checking this.
>>>>
>>>>> I checked the URL specification but could not see any problem with 
>>>>> empty
>>>>> path. The HTTP/1.1 specification is there a bit more detailed. So i
>>>>> checked HttpURLconnection.java and HttpClient.java where i found the
>>>>> problem. If the path/file from url.getFile() is null or empty, a 
>>>>> "/" is
>>>>> used but not if the url.getFile() returns only a query string. In 
>>>>> that
>>>>> case the path is empty and should have also a "/".
>>>>
>>>> Sounds reasonable.
>>>>
>>>>> A webrev can be found here (to be discussed, i am still new to 
>>>>> openjdk):
>>>>> http://cr.openjdk.java.net/~arieber/7025238/webrev.00/
>>>>
>>>> The source changes look good to me.
>>>>
>>>>> To write the jtreg test and run them all took longer than the fix 
>>>>> ;-) I
>>>>
>>>> Yes, this can often be the case, but thanks for adding a test.
>>>>
>>>> Trivially, the test does not need to be run in other VM mode. You 
>>>> can simply remove the line "@run main/othervm B7025238". The 
>>>> default action for jtreg is to run the test, essentially "@run main 
>>>> B7025238".
>>>>
>>>>> did run jtreg on: |test/java/net, | |test/sun/net, | 
>>>>> |test/java/security
>>>>> and | |test/sun/security but sure i don't have all relevant 
>>>>> platfo||rms.|
>>>>
>>>> Kurchi sent me mail offline, she has agreed to sponsor this change 
>>>> for you. I would request that she runs all the networking tests on 
>>>> all the platforms before pushing. Not a big problem for us here, we 
>>>> have access to all supported platforms.
>>>>
>>>> Thanks again,
>>>> -Chris.
>>>>
>>>>>
>>>>> thanks
>>>>> Andreas
>>>>>
>>>
>>
>

-- 
-Kurchi