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

[Kurchi Hazra]

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