RFR 4906983: java.net.URL constructors throw MalformedURLException in undocumented way

[Chris Hegarty]

> On 13 Sep 2015, at 14:07, Mark Sheppard <mark.sheppard at oracle.com> wrote:
> 
> Hi
>   I don't think the URL string  http://server:-1/path can be considered a valid URL as the port value -1 is not legal port value.

I agree, but java.net <http://java.net/>.URL is a legacy API. The only changes we can realistically make to it now are to tighten the spec as per existing behaviour ( in a manner so as to not encourage it to be used in an incorrect way ).

> However, the URL class doesn't object and throw a MalformedURLException, which is something of an
> anomaly, caused by the spec allowing a port value of -1 for the 4 arg constructor, which maps to
> the default value for the associated protocol.  Using an input port value of -1 is unnecessary, as the 3 arg constructor
> is sufficient to specify a URL with a default port value.

Right, it could be argued that the original authors added to much “convenience” here.

> It would seem more appropriate to encapsulate the
> -1 value as an implementation detail.

Right, it is a pity that the ‘-1’ has made it into the spec :-(

> A further anomaly arises when a subsequent getPort() method is invoked, and this returns a value of -1, rather than
> the protocol's associated default value. The -1 doesn't seem to activate until a connection is made using the URL's details.

Another unfortunate mistake.

> a more generalized description for  MalformedURLException  could be used, e.g.
> 
> if the parsed URL fails to comply with the scheme specific syntax of the associated protocol.

Is this suggested wording for the “spec” accepting constructors?  I think what we have for the constructors accepting protocol, host, port, etc, is more accurate.

-Chris.

> regards
> Mark
> 
> 
> 
> On 10/09/2015 20:32, Sebastian Sickelmann wrote:
>> Hi,
>> 
>> first thanks to Chris and David for their helpful input . I looked through the existing 
>> Testcases and found one that is already testing for negative-port numbers.
>> So i extended the @bug line with "4906983" which I hope is the right solution to do it.
>> 
>> I am with Chris, when he says normally you only have numbers between 1 and
>> 65535 (because many protocols are using tcp). So i changed to documentation as 
>> Chris suggested it.
>> 
>> But ports above this "natural" barrier are valid too. It depends on the protocol what
>> to do with the port information. So I also extended the testcase to check that their are
>> valid port numbers also above 65535 and the special -1.
>> 
>> But i asked myself should
>> 	new URL("http://server:-1/path");
>> be realy a valid URL? What do you think?
>> 
>> Special thanks to David who hosted the new webrev:
>> http://cr.openjdk.java.net/~dbuck/4906983.1/ <http://cr.openjdk.java.net/%7Edbuck/4906983.1/>
>> 
>> -- Sebastian
>> 
>> 
>> Am 10.09.2015 um 12:38 schrieb Chris Hegarty:
>>> Another minor comment...
>>> 
>>> While what you have suggested is not incorrect, I’m afraid it is giving the wrong impression about the typical acceptable port ranges. A port of Integer.MAX_VALUE is not all that useful, since it typically maps to a TCP port number ( but not always ). Maybe just remove the valid values from @param port, and add something like the following to MalformedURLException: “.., or the port is a negative number other than -1” ?
>>> 
>>> -Chris.
>>> 
>>> On 10 Sep 2015, at 11:13, Chris Hegarty <chris.hegarty at oracle.com> <mailto:chris.hegarty at oracle.com> wrote:
>>> 
>>>> On 8 Sep 2015, at 21:01, Sebastian Sickelmann <sebastian.sickelmann at gmx.de> <mailto:sebastian.sickelmann at gmx.de> wrote:
>>>> 
>>>>> Hi,
>>>>> 
>>>>> Please find my small patch[1] to javadoc in java.net.URL that adresses
>>>>> JDK-4906983(javadoc-fix)[2].
>>>>> 
>>>>> I signed the SCA/OCA some time ago. Feel free to check at the OCA
>>>>> Signatures-List[3]
>>>>> 
>>>>> thanks to david buck for hosting this patch on cr.openjdk.java.net.
>>>>> 
>>>>> -- Sebastian Sickelmann
>>>>> 
>>>>> [1] http://cr.openjdk.java.net/~dbuck/4906983.0/ <http://cr.openjdk.java.net/%7Edbuck/4906983.0/>
>>>> Just to confirm this is a spec only change, that documents long standing existing behaviour, right?
>>>> 
>>>> I think we should add a minimal testcase to cover this.
>>>> 
>>>> Thanks,
>>>> -Chris.
>>>> 
>>>>> [2] https://bugs.openjdk.java.net/browse/JDK-4906983 <https://bugs.openjdk.java.net/browse/JDK-4906983>
>>>>> 
>>>>> [3] http://www.oracle.com/technetwork/community/oca-486395.html <http://www.oracle.com/technetwork/community/oca-486395.html>
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/net-dev/attachments/20150913/14472107/attachment.html>