On 11/14/2012 03:28 PM, Dmitry Samersoff wrote:
Weijun,
Sorry. I was not clean enough. See below.
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?
As for the possibility of 0088, I'll remove that equals check and stick with
hostport = tokenizer.nextToken() + ":" + port;
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
