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

Chris Hegarty chris.hegarty at oracle.com
Mon Sep 14 18:28:18 UTC 2015 

> On 14 Sep 2015, at 16:45, Sebastian Sickelmann <sebastian.sickelmann at gmx.de> wrote:
> 
> Am 14.09.2015 um 10:53 schrieb Chris Hegarty:
>> On 13/09/15 20:18, Sebastian Sickelmann wrote:
>>> Am 13.09.2015 um 16:25 schrieb Chris Hegarty:
>>>> 
>>>>> On 13 Sep 2015, at 14:07, Mark Sheppard
>>>>> <<mailto:mark.sheppard at oracle.com>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. ]
>> 
>> I would be opposed to changing the behavior of any of these constructors.
>>> 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.
>> 
>> Separately, if at all.
> I think the constructors with the "spec" parameters are the only one
> that can be
> changed without much damage. But you are right the risk is to big for
> the very
> little value. So i will not start another thread about this.
>> 
>>> For now i would leave the testcase and add a longer comment to it.
>> 
>> For now, in the test, just remove the string spec constructors, and
>> leave the ones accepting the port arg.
> I removed the line and my comment. Do you think i should add it back as
> comment to the test-case?

No. I don’t think this is necessary.

>        // As discussed in
>        //
> http://mail.openjdk.java.net/pipermail/net-dev/2015-September/009138.html <http://mail.openjdk.java.net/pipermail/net-dev/2015-September/009138.html>
>        // and the follow up thread. This is a very unusual way to
> specify the use
>        // of the default port. Sadly it is possible but the risk of
> breaking things is to high
>        // for the value a change would offer.
>        // url = new URL("http://server:-1/path <http://server:-1/path>"); // Is this realy a
> valid one?
> 
> 
>> 
>>>>> 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.
>> 
>> I don't think we can do this without breaking many things. And it adds
>> little 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?
>> 
>> I do not think this is worth pursuing.
>> 
>> -Chris.
> Here is the new webrev-url:
> 
> http://cr.openjdk.java.net/~dbuck/4906983.2/ <http://cr.openjdk.java.net/~dbuck/4906983.2/>

My preference is to not touch any of the ‘-1’ / default port wording. I just see it as unnecessary.

-Chris.

> Thanks to all for the review and all other support.
> 
> -- Sebastian
>> 
>>> -- 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
>>>>>>>>> oncr.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/20150914/8a481e14/attachment-0001.html>

More information about the net-dev mailing list