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

[Andreas Rieber]

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
>>>>
>>
>