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

[Andreas Rieber]

On 20.06.13 12:55, Chris Hegarty wrote:
> On 06/20/2013 10:33 AM, Andreas Rieber wrote:
>> 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 '/'.
>
> Sorry for the confusion, my point was; should the server with a valid
> context for '/' tolerate a 'GET ?xxyyzz HTTP/1.1'?

According to the HTTP/1.1 spec, no:

3.2.2 http URL

The "http" scheme is used to locate network resources via the HTTP 
protocol. This section defines the scheme-specific syntax and semantics 
for http URLs.

http_URL = "http:" "//" host [ ":" port ] [ abs_path [ "?" query ]]

If the port is empty or not given, port 80 is assumed. The semantics are 
that the identified resource is located at the server listening for TCP 
connections on that port of that host, and the Request-URI for the 
resource is abs_path (section 5.1.2). The use of IP addresses in URLs 
SHOULD be avoided whenever possible (see RFC 1900 [24]). If the abs_path 
is not present in the URL, it MUST be given as "/" when used as a 
Request-URI for a resource (section 5.1.2). If a proxy receives a host 
name which is not a fully qualified domain name, it MAY add its domain 
to the host name it received. If a proxy receives a fully qualified 
domain name, the proxy MUST NOT change the host name.


Andreas


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