Max, Can we get the fix[1] backported to JDK7? Patch applies cleanly on top of JDK 7 sources for me.
Thanks, Severin [1] http://hg.openjdk.java.net/jdk8/tl/jdk/rev/f740a9ac6eb6 On Wed, 2012-11-14 at 17:04 +0400, Dmitry Samersoff wrote: > 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 > >>>>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>> > >>>>>> > >>>> > >>>> > >> > >> > >
