Am 13.09.2015 um 16:25 schrieb Chris Hegarty: > >> On 13 Sep 2015, at 14:07, Mark Sheppard <mark.shepp...@oracle.com >> <mailto: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 ).
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.hega...@oracle.com> wrote: >>>> >>>>> On 8 Sep 2015, at 21:01, Sebastian Sickelmann >>>>> <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 >>>>>> <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 >>> >> >