Weijun, On 2012-11-14 12:08, Weijun Wang wrote: >> On 2012-11-14 04:36, Weijun Wang wrote: >>> Yes, but with line 288, hostport will be "host:1". Isn't that expected? >> >> I'm OK with ll. 288 but against ll.285. I'm against comparing integers >> as strings - port could (imaginary) be specified as 0088 and comparison >> fails. > > I see. > > Actually, in KdcComm.java when the string "host:port" is really been > used, Integer.parseInt(port) is called and when an exception is thrown > host:88 is tried, although this might not be the correct guess. > > Are you ok with this "delayed" check?
Yes. > > As for the possibility of 0088, I'll remove that equals check and stick > with > > hostport = tokenizer.nextToken() + ":" + port; I'm OK with it. -Dmitry > > You can see my DNS.java test already prepared for this by comparing to > both a.com.:88 and a.com. :) > > Thanks > Max > >> >> >>>> Customer allowed to use service name here instead of numeric port >>>> number >>>> if my memory is not bogus. >>> >>> rfc2782 [1] says it can be only a number: >> >> Thank you for clarification. >> -Dmitry >> >>> >>> Port >>> The port on this target host of this service. The range is 0- >>> 65535. This is a 16 bit unsigned integer in network byte >>> order. >>> This is often as specified in Assigned Numbers but need not be. >>> >>> If it's really a service name, I might have to ignore it at the moment >>> because I don't know an API to perform getportbyname(). >>> >>>> >>>> 2. >>>>> shadow the real one by prepending it to the bootclasspath. >>>> jtreg allows you to change boot classpath in othervm mode >>>> >>>> e.g. : >>>> >>>> @run main/othervm -Xbootclasspath/a:../classes/serviceability >>>> -XX:+UnlockDiagnosticVMOptions ... ParserTest >>> >>> Good suggestion, but the class is built into test.classes and test run >>> in scratch. I cannot find a way to get the relative path to the class. I >>> reply on "javac -d ." to output the class into scratch. >>> >>> Thanks >>> Max >>> >>> [1] http://tools.ietf.org/html/rfc2782 >>> >>>> >>>> >>>> -Dmitry >>>> >>>> On 2012-11-13 16:07, Weijun Wang wrote: >>>>> >>>>> >>>>> On 11/13/2012 07:44 PM, Dmitry Samersoff wrote: >>>>>> Weijun, >>>>>> >>>>>> Config.java:1162 >>>>>> This code is unclear to me. >>>>>> if srvs[i] could be "" this code could insert extra space in >>>>>> the middle of kdcs string. >>>>> >>>>> It should never be empty. KrbServiceLocator.java:281 makes sure it's a >>>>> valid DNS SRV record. >>>>> >>>>>> >>>>>> if srvc[i] couldn't be empty we can return null just >>>>>> after line 1160 if srvs.length == 0 >>>>> >>>>> Yes, we can. >>>>> >>>>>> >>>>>> KrbServiceLocator.java:285 >>>>>> >>>>>> Kerberos could be run on non standard port - rare case but >>>>>> AFAIK we don't limit it. So it's better to use parseInt here. >>>>> >>>>> That's line 288. Are you suggesting that port string can be >>>>> non-numeric >>>>> and need a check? >>>>> >>>>>> >>>>>> dns.sh: >>>>>> Why we need Shell script here? >>>>> >>>>> I cannot use the real NamingManager inside JDK because it will attempt >>>>> to access a real DNS server, thus I write my own fake provider and >>>>> shadow the real one by prepending it to the bootclasspath. >>>>> >>>>> Thanks >>>>> Max >>>>> >>>>> >>>>>> >>>>>> Regards, >>>>>> -Dmitry >>>>>> >>>>>> >>>>>> >>>>>> On 2012-11-13 15:05, Weijun Wang wrote: >>>>>>> Ping again. >>>>>>> >>>>>>> The webrev contains codes by myself so I need another reviewer. >>>>>>> >>>>>>> Thanks >>>>>>> Max >>>>>>> >>>>>>> >>>>>>> On 11/09/2012 08:38 AM, Weijun Wang wrote: >>>>>>>> Hi Severin >>>>>>>> >>>>>>>> I've created an OpenJDK bug and created a new webrev: >>>>>>>> >>>>>>>> http://cr.openjdk.java.net/~weijun/8002344/webrev.00/ >>>>>>>> >>>>>>>> The Config.java change is identical to yours, and I added a small >>>>>>>> tweak >>>>>>>> in KrbServiceLocator, and a quite ugly regression test. >>>>>>>> >>>>>>>> Anyone can review all the changes? >>>>>>>> >>>>>>>> After the code review, I'll push the change to tl/jdk. I don't >>>>>>>> see an >>>>>>>> OpenJDK user id for you at http://db.openjdk.java.net/people, so I >>>>>>>> add >>>>>>>> your name in >>>>>>>> >>>>>>>> Contributed-by: Severin Gehwolf <[email protected]> >>>>>>>> >>>>>>>> Thanks >>>>>>>> Max >>>>>>>> >>>>>>>> >>>>>>>> On 11/08/2012 11:46 PM, Severin Gehwolf wrote: >>>>>>>>> Hi Max, >>>>>>>>> >>>>>>>>> Thanks for the review! >>>>>>>>> >>>>>>>>> On Wed, 2012-11-07 at 08:52 +0800, Weijun Wang wrote: >>>>>>>>>> The fix looks fine. There is one place it might get enhanced: >>>>>>>>>> >>>>>>>>>> if (value.charAt(j) == ':') { >>>>>>>>>> kdcs = (value.substring(0, j)).trim(); >>>>>>>>>> } >>>>>>>>>> >>>>>>>>>> So this changes a.com:88 to a.com. If the port is really 88, >>>>>>>>>> it's OK. >>>>>>>>>> Otherwise, info gets lost. Maybe we can keep the whole string. >>>>>>>>> >>>>>>>>> I've removed the entire loop which strips the port from the >>>>>>>>> returned >>>>>>>>> record. Updated webrev is here: >>>>>>>>> >>>>>>>>> http://jerboaa.fedorapeople.org/bugs/openjdk/2376501/webrev.1/ >>>>>>>>> >>>>>>>>>> BTW, are you OK with contributing the fix into OpenJDK main repo? >>>>>>>>> >>>>>>>>> Yes, of course :) Just let me know what's to be done to get it >>>>>>>>> pushed. >>>>>>>>> >>>>>>>>> Cheers, >>>>>>>>> Severin >>>>>>>>> >>>>>>>>>> On 11/06/2012 11:08 PM, Severin Gehwolf wrote: >>>>>>>>>>> Hi, >>>>>>>>>>> >>>>>>>>>>> In Config.java, line 1234 in method getKDCFromDNS(String realm) >>>>>>>>>>> there is >>>>>>>>>>> a loop which discards earlier values of KDCs returned rather >>>>>>>>>>> than >>>>>>>>>>> concatenating them. This results in a behaviour where only >>>>>>>>>>> one KDC >>>>>>>>>>> in a >>>>>>>>>>> seemingly random fashion is returned. In fact, the KDC returned >>>>>>>>>>> depends >>>>>>>>>>> on the order which KrbServiceLocator.getKerberosService(realm, >>>>>>>>>>> "_udp") >>>>>>>>>>> returns the servers. The correct behaviour should be to return a >>>>>>>>>>> String >>>>>>>>>>> containing ALL KDCs available via DNS (separated by spaces). >>>>>>>>>>> >>>>>>>>>>> The webrev is here: >>>>>>>>>>> http://jerboaa.fedorapeople.org/bugs/openjdk/2376501/webrev/ >>>>>>>>>>> >>>>>>>>>>> Comments and suggestions very welcome! >>>>>>>>>>> >>>>>>>>>>> Thanks, >>>>>>>>>>> Severin >>>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>> >>>>>> >>>> >>>> >> >> -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * Give Rabbit time, and he'll always get the answer
