Sure. I'll do some basic test before requesting for backport. Tomorrow. 10pm here in Beijing.

Thanks
Max

On 11/19/2012 07:37 PM, Severin Gehwolf wrote:
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















Reply via email to