> On 13 Sep 2015, at 14:07, Mark Sheppard <mark.shepp...@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.hega...@oracle.com> >>> <mailto:chris.hega...@oracle.com> wrote: >>> >>>> On 8 Sep 2015, at 21:01, Sebastian Sickelmann >>>> <sebastian.sickelm...@gmx.de> <mailto:sebastian.sickelm...@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> >> >
