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

[Andreas Rieber]

I see, short test without leading "/" in the path causes the httpserver 
to throws exception:

Exception in thread "main" java.lang.IllegalArgumentException: Illegal 
value for path or protocol
	at sun.net.httpserver.HttpContextImpl.<init>(HttpContextImpl.java:60)
	at sun.net.httpserver.ServerImpl.createContext(ServerImpl.java:214)
	at sun.net.httpserver.HttpServerImpl.createContext(HttpServerImpl.java:74)
	at sun.net.httpserver.HttpServerImpl.createContext(HttpServerImpl.java:39)
	at SimpleServer.main(SimpleServer.java:11)

but the api is clear there: The first character of path must be '/'.

If you want we can change that to "should" in the api and add a "/" if 
missing. I guess there are other issues to fix first.

Andreas


On 20.06.13 10:40, Chris Hegarty wrote:
> 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
>>>>>>
>>>>
>>>
>>
>
>