> On 14 Sep 2015, at 16:45, Sebastian Sickelmann <sebastian.sickelm...@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.shepp...@oracle.com>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. ] >> >> 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.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 >>>>>>>>> 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
