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
