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

Chris Hegarty chris.hegarty at oracle.com
Thu Jun 20 01:40:50 PDT 2013


I see Kurchi pushed this change for you.
   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2b156531b7eb

For extra credits ;-) does it make sense to something similar on the 
server side, sun.net.httpserver???

-Chris.

On 06/19/2013 07:29 PM, 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
>>>>>
>>>
>>
>



More information about the net-dev mailing list