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

[Sebastian Sickelmann]

Am 13.09.2015 um 16:25 schrieb Chris Hegarty:
>
>> On 13 Sep 2015, at 14:07, Mark Sheppard <mark.sheppard at oracle.com
>> <mailto: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 ).

I think we could change the behavior of the constructors which receive a
"spec" parameter. In non of those javadocs the -1 is mentioned. But this
is maybe another discussion, unrelated to cleanup the javadoc. I would
start it in another thread, when you agree to discuss this separatly. 
For now i would leave the testcase and add a longer comment to it.

>
>> 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.
>
I think we can remove almost all references in the javadoc to the
special -1 port value. getPort() is one place where it cannot
removed, and the mention for the constructor with the 5 parameters could
be remove when we introduce a constructur
with almost the same signature just leave the port away. But i am not
sure if intrudicing another constructor is worth the
-1 removale in the javadoc.

If it is not worth it, the -1 reference can at least be removed in the 2
and 3 parameter constructors javadoc. I will prepare this.

What do you think?

-- Sebastian
> 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/
>>>
>>> -- 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> wrote:
>>>>
>>>>> On 8 Sep 2015, at 21:01, Sebastian Sickelmann <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 <http://cr.openjdk.java.net>.
>>>>>>
>>>>>> -- Sebastian Sickelmann
>>>>>>
>>>>>> [1] http://cr.openjdk.java.net/~dbuck/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
>>>>>>
>>>>>> [3] 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/d5f1c9ab/attachment.html>