Re: RFR [8022584] Memory leak in some NetworkInterface methods
Thanks for taking this Ivan. Can you please make the changes suggested by both David and Alan ( simply return NULL/-1/JNI_FALSE, as appropriate, if GetStringUTFChars fails ( returns NULL ), then I will sponsor this change into jdk8 for you. Please post an update webrev to cr.openjdk.java.net. -Chris. On 08/08/2013 06:01 AM, David Holmes wrote: Ivan, On 8/08/2013 2:05 PM, Ivan Gerasimov wrote: David, Alan, I added checking for NULL results and throwing OOMException if necessary. You don't need to throw it yourself: JNU_ThrowOutOfMemoryError(env, NULL); Assuming a correct VM implementation if NULL is returned then an OOME should already be pending and will be thrown as soon as your native code returns to Java. Thanks, David I've also added some spaces along the code to improve indentation. Would you please review the updated webrev? http://washi.ru.oracle.com/~igerasim/webrevs/8022584/1/webrev/ Sincerely yours, Ivan On 08.08.2013 5:35, David Holmes wrote: On 8/08/2013 11:20 AM, Ivan Gerasimov wrote: Thanks Alan! I've checked that and it turns out that GetStringUTFChars cannot return NULL. For allocation of the result string it calls AllocateHeap() with the default EXIT_OOM fail strategy. Thus, in case of being unable to allocate memory it simply stops VM. Ouch! That is a hotspot bug. GetStringUTFChars is supposed to throw OutOfMemoryError if it has memory issues, not abort the VM! I recommend that you check for NULL and/or a pending exception. David Sincerely yours, Ivan On 08.08.2013 5:05, Alan Bateman wrote: (cc'ing net-dev). The change looks okay to me. One suggestion (while you are there) is to check the return from GetStringUTFChars so that the name returns when it fails with NULL. -Alan. On 07/08/2013 17:46, Ivan Gerasimov wrote: Hello everybody! Some methods of NetworkInterface class were reported to leak memory. http://bugs.sun.com/view_bug.do?bug_id=JDK-8022584 (not yet available.) Examples are isUp() and isLoopback(). Affected versions of jdk are 6, 7 and 8 Would you please review a simple fix that removes the unnecessary allocation? http://cr.openjdk.java.net/~igerasim/8022584/0/webrev/ Sincerely yours, Ivan
hg: jdk8/tl/langtools: 8019486: javac, generates erroneous LVT for a test case with lambda code
Changeset: b8610a65fbf9 Author:vromero Date: 2013-08-08 11:49 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/b8610a65fbf9 8019486: javac, generates erroneous LVT for a test case with lambda code Reviewed-by: mcimadamore ! src/share/classes/com/sun/tools/javac/comp/LambdaToMethod.java + test/tools/javac/T8019486/WrongLVTForLambdaTest.java
hg: jdk8/tl/jdk: 8016594: Native Windows ccache still reads DES tickets
Changeset: b7d594716f86 Author:weijun Date: 2013-08-08 21:13 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/b7d594716f86 8016594: Native Windows ccache still reads DES tickets Reviewed-by: dsamersoff, xuelei ! src/share/classes/sun/security/krb5/Credentials.java ! src/share/native/sun/security/krb5/nativeccache.c ! src/windows/native/sun/security/krb5/NativeCreds.c
Re: RFR [8022584] Memory leak in some NetworkInterface methods
Hello Chris! Here's the update: http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/ Thanks for offering the sponsorship! Here's the hg-export http://cr.openjdk.java.net/~igerasim/2commit/8022584-jdk8-Memleak-in-NetworkInterface.patch Sincerely yours, Ivan On 08.08.2013 12:43, Chris Hegarty wrote: Thanks for taking this Ivan. Can you please make the changes suggested by both David and Alan ( simply return NULL/-1/JNI_FALSE, as appropriate, if GetStringUTFChars fails ( returns NULL ), then I will sponsor this change into jdk8 for you. Please post an update webrev to cr.openjdk.java.net. -Chris. On 08/08/2013 06:01 AM, David Holmes wrote: Ivan, On 8/08/2013 2:05 PM, Ivan Gerasimov wrote: David, Alan, I added checking for NULL results and throwing OOMException if necessary. You don't need to throw it yourself: JNU_ThrowOutOfMemoryError(env, NULL); Assuming a correct VM implementation if NULL is returned then an OOME should already be pending and will be thrown as soon as your native code returns to Java. Thanks, David I've also added some spaces along the code to improve indentation. Would you please review the updated webrev? http://washi.ru.oracle.com/~igerasim/webrevs/8022584/1/webrev/ Sincerely yours, Ivan On 08.08.2013 5:35, David Holmes wrote: On 8/08/2013 11:20 AM, Ivan Gerasimov wrote: Thanks Alan! I've checked that and it turns out that GetStringUTFChars cannot return NULL. For allocation of the result string it calls AllocateHeap() with the default EXIT_OOM fail strategy. Thus, in case of being unable to allocate memory it simply stops VM. Ouch! That is a hotspot bug. GetStringUTFChars is supposed to throw OutOfMemoryError if it has memory issues, not abort the VM! I recommend that you check for NULL and/or a pending exception. David Sincerely yours, Ivan On 08.08.2013 5:05, Alan Bateman wrote: (cc'ing net-dev). The change looks okay to me. One suggestion (while you are there) is to check the return from GetStringUTFChars so that the name returns when it fails with NULL. -Alan. On 07/08/2013 17:46, Ivan Gerasimov wrote: Hello everybody! Some methods of NetworkInterface class were reported to leak memory. http://bugs.sun.com/view_bug.do?bug_id=JDK-8022584 (not yet available.) Examples are isUp() and isLoopback(). Affected versions of jdk are 6, 7 and 8 Would you please review a simple fix that removes the unnecessary allocation? http://cr.openjdk.java.net/~igerasim/8022584/0/webrev/ Sincerely yours, Ivan
Re: RFR [8022584] Memory leak in some NetworkInterface methods
Thanks, David I've updated the webrev http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/. On 08.08.2013 9:01, David Holmes wrote: Ivan, On 8/08/2013 2:05 PM, Ivan Gerasimov wrote: David, Alan, I added checking for NULL results and throwing OOMException if necessary. You don't need to throw it yourself: JNU_ThrowOutOfMemoryError(env, NULL); Assuming a correct VM implementation if NULL is returned then an OOME should already be pending and will be thrown as soon as your native code returns to Java. It seemed to me that, JNU_ThrowOutOfMemoryError only throws an exception in a case one is not pending. But I don't mind to remove it, relaying on the correct implementation of GetStringUTFChars. Sincerely yours, Ivan Thanks, David I've also added some spaces along the code to improve indentation. Would you please review the updated webrev? http://washi.ru.oracle.com/~igerasim/webrevs/8022584/1/webrev/ Sincerely yours, Ivan On 08.08.2013 5:35, David Holmes wrote: On 8/08/2013 11:20 AM, Ivan Gerasimov wrote: Thanks Alan! I've checked that and it turns out that GetStringUTFChars cannot return NULL. For allocation of the result string it calls AllocateHeap() with the default EXIT_OOM fail strategy. Thus, in case of being unable to allocate memory it simply stops VM. Ouch! That is a hotspot bug. GetStringUTFChars is supposed to throw OutOfMemoryError if it has memory issues, not abort the VM! I recommend that you check for NULL and/or a pending exception. David Sincerely yours, Ivan On 08.08.2013 5:05, Alan Bateman wrote: (cc'ing net-dev). The change looks okay to me. One suggestion (while you are there) is to check the return from GetStringUTFChars so that the name returns when it fails with NULL. -Alan. On 07/08/2013 17:46, Ivan Gerasimov wrote: Hello everybody! Some methods of NetworkInterface class were reported to leak memory. http://bugs.sun.com/view_bug.do?bug_id=JDK-8022584 (not yet available.) Examples are isUp() and isLoopback(). Affected versions of jdk are 6, 7 and 8 Would you please review a simple fix that removes the unnecessary allocation? http://cr.openjdk.java.net/~igerasim/8022584/0/webrev/ Sincerely yours, Ivan
Re: RFR [8022584] Memory leak in some NetworkInterface methods
The patch looks good to me. I guess a regression test isn't feasible. So, the bug will be tagged noreg-hard Michael On 08/08/13 14:39, Ivan Gerasimov wrote: Thanks, David I've updated the webrev http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/. On 08.08.2013 9:01, David Holmes wrote: Ivan, On 8/08/2013 2:05 PM, Ivan Gerasimov wrote: David, Alan, I added checking for NULL results and throwing OOMException if necessary. You don't need to throw it yourself: JNU_ThrowOutOfMemoryError(env, NULL); Assuming a correct VM implementation if NULL is returned then an OOME should already be pending and will be thrown as soon as your native code returns to Java. It seemed to me that, JNU_ThrowOutOfMemoryError only throws an exception in a case one is not pending. But I don't mind to remove it, relaying on the correct implementation of GetStringUTFChars. Sincerely yours, Ivan Thanks, David I've also added some spaces along the code to improve indentation. Would you please review the updated webrev? http://washi.ru.oracle.com/~igerasim/webrevs/8022584/1/webrev/ Sincerely yours, Ivan On 08.08.2013 5:35, David Holmes wrote: On 8/08/2013 11:20 AM, Ivan Gerasimov wrote: Thanks Alan! I've checked that and it turns out that GetStringUTFChars cannot return NULL. For allocation of the result string it calls AllocateHeap() with the default EXIT_OOM fail strategy. Thus, in case of being unable to allocate memory it simply stops VM. Ouch! That is a hotspot bug. GetStringUTFChars is supposed to throw OutOfMemoryError if it has memory issues, not abort the VM! I recommend that you check for NULL and/or a pending exception. David Sincerely yours, Ivan On 08.08.2013 5:05, Alan Bateman wrote: (cc'ing net-dev). The change looks okay to me. One suggestion (while you are there) is to check the return from GetStringUTFChars so that the name returns when it fails with NULL. -Alan. On 07/08/2013 17:46, Ivan Gerasimov wrote: Hello everybody! Some methods of NetworkInterface class were reported to leak memory. http://bugs.sun.com/view_bug.do?bug_id=JDK-8022584 (not yet available.) Examples are isUp() and isLoopback(). Affected versions of jdk are 6, 7 and 8 Would you please review a simple fix that removes the unnecessary allocation? http://cr.openjdk.java.net/~igerasim/8022584/0/webrev/ Sincerely yours, Ivan
Re: RFR [8022584] Memory leak in some NetworkInterface methods
Looks good to me. Thanks Ivan. -Chris. On 08/08/2013 02:33 PM, Ivan Gerasimov wrote: Hello Chris! Here's the update: http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/ Thanks for offering the sponsorship! Here's the hg-export http://cr.openjdk.java.net/~igerasim/2commit/8022584-jdk8-Memleak-in-NetworkInterface.patch Sincerely yours, Ivan On 08.08.2013 12:43, Chris Hegarty wrote: Thanks for taking this Ivan. Can you please make the changes suggested by both David and Alan ( simply return NULL/-1/JNI_FALSE, as appropriate, if GetStringUTFChars fails ( returns NULL ), then I will sponsor this change into jdk8 for you. Please post an update webrev to cr.openjdk.java.net. -Chris. On 08/08/2013 06:01 AM, David Holmes wrote: Ivan, On 8/08/2013 2:05 PM, Ivan Gerasimov wrote: David, Alan, I added checking for NULL results and throwing OOMException if necessary. You don't need to throw it yourself: JNU_ThrowOutOfMemoryError(env, NULL); Assuming a correct VM implementation if NULL is returned then an OOME should already be pending and will be thrown as soon as your native code returns to Java. Thanks, David I've also added some spaces along the code to improve indentation. Would you please review the updated webrev? http://washi.ru.oracle.com/~igerasim/webrevs/8022584/1/webrev/ Sincerely yours, Ivan On 08.08.2013 5:35, David Holmes wrote: On 8/08/2013 11:20 AM, Ivan Gerasimov wrote: Thanks Alan! I've checked that and it turns out that GetStringUTFChars cannot return NULL. For allocation of the result string it calls AllocateHeap() with the default EXIT_OOM fail strategy. Thus, in case of being unable to allocate memory it simply stops VM. Ouch! That is a hotspot bug. GetStringUTFChars is supposed to throw OutOfMemoryError if it has memory issues, not abort the VM! I recommend that you check for NULL and/or a pending exception. David Sincerely yours, Ivan On 08.08.2013 5:05, Alan Bateman wrote: (cc'ing net-dev). The change looks okay to me. One suggestion (while you are there) is to check the return from GetStringUTFChars so that the name returns when it fails with NULL. -Alan. On 07/08/2013 17:46, Ivan Gerasimov wrote: Hello everybody! Some methods of NetworkInterface class were reported to leak memory. http://bugs.sun.com/view_bug.do?bug_id=JDK-8022584 (not yet available.) Examples are isUp() and isLoopback(). Affected versions of jdk are 6, 7 and 8 Would you please review a simple fix that removes the unnecessary allocation? http://cr.openjdk.java.net/~igerasim/8022584/0/webrev/ Sincerely yours, Ivan
Re: RFR [8022584] Memory leak in some NetworkInterface methods
On 08/08/2013 06:39, Ivan Gerasimov wrote: Thanks, David I've updated the webrev http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/. Thanks for fixing the other GetStringUTFChars usages too. This version looks good to me. -Alan.
Re: RFR [8022584] Memory leak in some NetworkInterface methods
Thank you Michael! I'm working on the test. Chris, if it's not too late, I would like to include a regtest into the change. It will be ready in a few minutes and I'll send an updated webrev. Thanks, Ivan On 08.08.2013 17:51, Michael McMahon wrote: The patch looks good to me. I guess a regression test isn't feasible. So, the bug will be tagged noreg-hard Michael On 08/08/13 14:39, Ivan Gerasimov wrote: Thanks, David I've updated the webrev http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/. On 08.08.2013 9:01, David Holmes wrote: Ivan, On 8/08/2013 2:05 PM, Ivan Gerasimov wrote: David, Alan, I added checking for NULL results and throwing OOMException if necessary. You don't need to throw it yourself: JNU_ThrowOutOfMemoryError(env, NULL); Assuming a correct VM implementation if NULL is returned then an OOME should already be pending and will be thrown as soon as your native code returns to Java. It seemed to me that, JNU_ThrowOutOfMemoryError only throws an exception in a case one is not pending. But I don't mind to remove it, relaying on the correct implementation of GetStringUTFChars. Sincerely yours, Ivan Thanks, David I've also added some spaces along the code to improve indentation. Would you please review the updated webrev? http://washi.ru.oracle.com/~igerasim/webrevs/8022584/1/webrev/ Sincerely yours, Ivan On 08.08.2013 5:35, David Holmes wrote: On 8/08/2013 11:20 AM, Ivan Gerasimov wrote: Thanks Alan! I've checked that and it turns out that GetStringUTFChars cannot return NULL. For allocation of the result string it calls AllocateHeap() with the default EXIT_OOM fail strategy. Thus, in case of being unable to allocate memory it simply stops VM. Ouch! That is a hotspot bug. GetStringUTFChars is supposed to throw OutOfMemoryError if it has memory issues, not abort the VM! I recommend that you check for NULL and/or a pending exception. David Sincerely yours, Ivan On 08.08.2013 5:05, Alan Bateman wrote: (cc'ing net-dev). The change looks okay to me. One suggestion (while you are there) is to check the return from GetStringUTFChars so that the name returns when it fails with NULL. -Alan. On 07/08/2013 17:46, Ivan Gerasimov wrote: Hello everybody! Some methods of NetworkInterface class were reported to leak memory. http://bugs.sun.com/view_bug.do?bug_id=JDK-8022584 (not yet available.) Examples are isUp() and isLoopback(). Affected versions of jdk are 6, 7 and 8 Would you please review a simple fix that removes the unnecessary allocation? http://cr.openjdk.java.net/~igerasim/8022584/0/webrev/ Sincerely yours, Ivan
Re: RFR [8022584] Memory leak in some NetworkInterface methods
Chris, if it's not too late, I'd like to include a regtest in the fix. Here's webrev that includes the test: http://cr.openjdk.java.net/~igerasim/8022584/3/webrev/ The test gets past with the fixed jdk8 and fails with jdk8-b101 and jdk7 as expected. Sincerely yours, Ivan On 08.08.2013 17:50, Chris Hegarty wrote: Looks good to me. Thanks Ivan. -Chris. On 08/08/2013 02:33 PM, Ivan Gerasimov wrote: Hello Chris! Here's the update: http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/ Thanks for offering the sponsorship! Here's the hg-export http://cr.openjdk.java.net/~igerasim/2commit/8022584-jdk8-Memleak-in-NetworkInterface.patch Sincerely yours, Ivan On 08.08.2013 12:43, Chris Hegarty wrote: Thanks for taking this Ivan. Can you please make the changes suggested by both David and Alan ( simply return NULL/-1/JNI_FALSE, as appropriate, if GetStringUTFChars fails ( returns NULL ), then I will sponsor this change into jdk8 for you. Please post an update webrev to cr.openjdk.java.net. -Chris. On 08/08/2013 06:01 AM, David Holmes wrote: Ivan, On 8/08/2013 2:05 PM, Ivan Gerasimov wrote: David, Alan, I added checking for NULL results and throwing OOMException if necessary. You don't need to throw it yourself: JNU_ThrowOutOfMemoryError(env, NULL); Assuming a correct VM implementation if NULL is returned then an OOME should already be pending and will be thrown as soon as your native code returns to Java. Thanks, David I've also added some spaces along the code to improve indentation. Would you please review the updated webrev? http://washi.ru.oracle.com/~igerasim/webrevs/8022584/1/webrev/ Sincerely yours, Ivan On 08.08.2013 5:35, David Holmes wrote: On 8/08/2013 11:20 AM, Ivan Gerasimov wrote: Thanks Alan! I've checked that and it turns out that GetStringUTFChars cannot return NULL. For allocation of the result string it calls AllocateHeap() with the default EXIT_OOM fail strategy. Thus, in case of being unable to allocate memory it simply stops VM. Ouch! That is a hotspot bug. GetStringUTFChars is supposed to throw OutOfMemoryError if it has memory issues, not abort the VM! I recommend that you check for NULL and/or a pending exception. David Sincerely yours, Ivan On 08.08.2013 5:05, Alan Bateman wrote: (cc'ing net-dev). The change looks okay to me. One suggestion (while you are there) is to check the return from GetStringUTFChars so that the name returns when it fails with NULL. -Alan. On 07/08/2013 17:46, Ivan Gerasimov wrote: Hello everybody! Some methods of NetworkInterface class were reported to leak memory. http://bugs.sun.com/view_bug.do?bug_id=JDK-8022584 (not yet available.) Examples are isUp() and isLoopback(). Affected versions of jdk are 6, 7 and 8 Would you please review a simple fix that removes the unnecessary allocation? http://cr.openjdk.java.net/~igerasim/8022584/0/webrev/ Sincerely yours, Ivan
hg: jdk8/tl/jdk: 8015666: test/tools/pack200/TimeStamp.java failing
Changeset: a388263a7287 Author:sherman Date: 2013-08-08 12:03 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/a388263a7287 8015666: test/tools/pack200/TimeStamp.java failing Summary: to keep the default behavior of ZOS unchanged, if ze extra time not explicitly set Reviewed-by: alanb, ksrini ! src/share/classes/java/util/zip/ZipConstants.java ! src/share/classes/java/util/zip/ZipEntry.java ! src/share/classes/java/util/zip/ZipFile.java ! src/share/classes/java/util/zip/ZipInputStream.java ! src/share/classes/java/util/zip/ZipOutputStream.java ! src/share/classes/java/util/zip/ZipUtils.java ! test/ProblemList.txt ! test/java/util/zip/TestExtraTime.java ! test/tools/pack200/TimeStamp.java
Re: RFR [8022584] Memory leak in some NetworkInterface methods
Main fix looks good to me.
Regression test may need some tweaking eg I think othervm will be needed.
Also this:
System.out.println("WARNING: Cannot perform memory leak detection on
this OS");
should probably just say something like "Test skipped on this OS" -
there are other tests that do this so just check if there is common
terminology. (In the future we may have keyword tags to flag this as
linux only etc.)
Thanks,
David
On 9/08/2013 4:41 AM, Ivan Gerasimov wrote:
Chris, if it's not too late, I'd like to include a regtest in the fix.
Here's webrev that includes the test:
http://cr.openjdk.java.net/~igerasim/8022584/3/webrev/
The test gets past with the fixed jdk8 and fails with jdk8-b101 and jdk7
as expected.
Sincerely yours,
Ivan
On 08.08.2013 17:50, Chris Hegarty wrote:
Looks good to me. Thanks Ivan.
-Chris.
On 08/08/2013 02:33 PM, Ivan Gerasimov wrote:
Hello Chris!
Here's the update:
http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/
Thanks for offering the sponsorship!
Here's the hg-export
http://cr.openjdk.java.net/~igerasim/2commit/8022584-jdk8-Memleak-in-NetworkInterface.patch
Sincerely yours,
Ivan
On 08.08.2013 12:43, Chris Hegarty wrote:
Thanks for taking this Ivan.
Can you please make the changes suggested by both David and Alan (
simply return NULL/-1/JNI_FALSE, as appropriate, if GetStringUTFChars
fails ( returns NULL ), then I will sponsor this change into jdk8 for
you.
Please post an update webrev to cr.openjdk.java.net.
-Chris.
On 08/08/2013 06:01 AM, David Holmes wrote:
Ivan,
On 8/08/2013 2:05 PM, Ivan Gerasimov wrote:
David, Alan,
I added checking for NULL results and throwing OOMException if
necessary.
You don't need to throw it yourself:
JNU_ThrowOutOfMemoryError(env, NULL);
Assuming a correct VM implementation if NULL is returned then an OOME
should already be pending and will be thrown as soon as your native
code
returns to Java.
Thanks,
David
I've also added some spaces along the code to improve indentation.
Would you please review the updated webrev?
http://washi.ru.oracle.com/~igerasim/webrevs/8022584/1/webrev/
Sincerely yours,
Ivan
On 08.08.2013 5:35, David Holmes wrote:
On 8/08/2013 11:20 AM, Ivan Gerasimov wrote:
Thanks Alan!
I've checked that and it turns out that GetStringUTFChars cannot
return
NULL.
For allocation of the result string it calls AllocateHeap() with
the
default EXIT_OOM fail strategy.
Thus, in case of being unable to allocate memory it simply stops
VM.
Ouch! That is a hotspot bug. GetStringUTFChars is supposed to throw
OutOfMemoryError if it has memory issues, not abort the VM!
I recommend that you check for NULL and/or a pending exception.
David
Sincerely yours,
Ivan
On 08.08.2013 5:05, Alan Bateman wrote:
(cc'ing net-dev).
The change looks okay to me. One suggestion (while you are
there) is
to check the return from GetStringUTFChars so that the name
returns
when it fails with NULL.
-Alan.
On 07/08/2013 17:46, Ivan Gerasimov wrote:
Hello everybody!
Some methods of NetworkInterface class were reported to leak
memory.
http://bugs.sun.com/view_bug.do?bug_id=JDK-8022584 (not yet
available.)
Examples are isUp() and isLoopback().
Affected versions of jdk are 6, 7 and 8
Would you please review a simple fix that removes the unnecessary
allocation?
http://cr.openjdk.java.net/~igerasim/8022584/0/webrev/
Sincerely yours,
Ivan
Re: RFR [8022584] Memory leak in some NetworkInterface methods
Yes, definitely "othervm" would be required for the test. Also, why skip
other OS'es?
The fix is only for Linux, but it might catch future leaks on Windows.
The trouble with tests like this, is they sometimes don't age well.
Future changes in OS kernel behavior could cause problems but I guess
32MB is a fairly large difference.
So, it should be okay
Michael
On 08/08/13 22:15, David Holmes wrote:
Main fix looks good to me.
Regression test may need some tweaking eg I think othervm will be needed.
Also this:
System.out.println("WARNING: Cannot perform memory leak detection on
this OS");
should probably just say something like "Test skipped on this OS" -
there are other tests that do this so just check if there is common
terminology. (In the future we may have keyword tags to flag this as
linux only etc.)
Thanks,
David
On 9/08/2013 4:41 AM, Ivan Gerasimov wrote:
Chris, if it's not too late, I'd like to include a regtest in the fix.
Here's webrev that includes the test:
http://cr.openjdk.java.net/~igerasim/8022584/3/webrev/
The test gets past with the fixed jdk8 and fails with jdk8-b101 and jdk7
as expected.
Sincerely yours,
Ivan
On 08.08.2013 17:50, Chris Hegarty wrote:
Looks good to me. Thanks Ivan.
-Chris.
On 08/08/2013 02:33 PM, Ivan Gerasimov wrote:
Hello Chris!
Here's the update:
http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/
Thanks for offering the sponsorship!
Here's the hg-export
http://cr.openjdk.java.net/~igerasim/2commit/8022584-jdk8-Memleak-in-NetworkInterface.patch
Sincerely yours,
Ivan
On 08.08.2013 12:43, Chris Hegarty wrote:
Thanks for taking this Ivan.
Can you please make the changes suggested by both David and Alan (
simply return NULL/-1/JNI_FALSE, as appropriate, if GetStringUTFChars
fails ( returns NULL ), then I will sponsor this change into jdk8 for
you.
Please post an update webrev to cr.openjdk.java.net.
-Chris.
On 08/08/2013 06:01 AM, David Holmes wrote:
Ivan,
On 8/08/2013 2:05 PM, Ivan Gerasimov wrote:
David, Alan,
I added checking for NULL results and throwing OOMException if
necessary.
You don't need to throw it yourself:
JNU_ThrowOutOfMemoryError(env, NULL);
Assuming a correct VM implementation if NULL is returned then an
OOME
should already be pending and will be thrown as soon as your native
code
returns to Java.
Thanks,
David
I've also added some spaces along the code to improve indentation.
Would you please review the updated webrev?
http://washi.ru.oracle.com/~igerasim/webrevs/8022584/1/webrev/
Sincerely yours,
Ivan
On 08.08.2013 5:35, David Holmes wrote:
On 8/08/2013 11:20 AM, Ivan Gerasimov wrote:
Thanks Alan!
I've checked that and it turns out that GetStringUTFChars cannot
return
NULL.
For allocation of the result string it calls AllocateHeap() with
the
default EXIT_OOM fail strategy.
Thus, in case of being unable to allocate memory it simply stops
VM.
Ouch! That is a hotspot bug. GetStringUTFChars is supposed to
throw
OutOfMemoryError if it has memory issues, not abort the VM!
I recommend that you check for NULL and/or a pending exception.
David
Sincerely yours,
Ivan
On 08.08.2013 5:05, Alan Bateman wrote:
(cc'ing net-dev).
The change looks okay to me. One suggestion (while you are
there) is
to check the return from GetStringUTFChars so that the name
returns
when it fails with NULL.
-Alan.
On 07/08/2013 17:46, Ivan Gerasimov wrote:
Hello everybody!
Some methods of NetworkInterface class were reported to leak
memory.
http://bugs.sun.com/view_bug.do?bug_id=JDK-8022584 (not yet
available.)
Examples are isUp() and isLoopback().
Affected versions of jdk are 6, 7 and 8
Would you please review a simple fix that removes the
unnecessary
allocation?
http://cr.openjdk.java.net/~igerasim/8022584/0/webrev/
Sincerely yours,
Ivan
Re: RFR [8022584] Memory leak in some NetworkInterface methods
Michael,
The test uses /proc/self/stat file to detect changes in virtual memory
usage.
This approach is specific for Linux.
That's why I included the check of OS in the test.
Sincerely yours,
Ivan
On 09.08.2013 1:38, Michael McMahon wrote:
Yes, definitely "othervm" would be required for the test. Also, why
skip other OS'es?
The fix is only for Linux, but it might catch future leaks on Windows.
The trouble with tests like this, is they sometimes don't age well.
Future changes in OS kernel behavior could cause problems but I guess
32MB is a fairly large difference.
So, it should be okay
Michael
On 08/08/13 22:15, David Holmes wrote:
Main fix looks good to me.
Regression test may need some tweaking eg I think othervm will be
needed.
Also this:
System.out.println("WARNING: Cannot perform memory leak detection on
this OS");
should probably just say something like "Test skipped on this OS" -
there are other tests that do this so just check if there is common
terminology. (In the future we may have keyword tags to flag this as
linux only etc.)
Thanks,
David
On 9/08/2013 4:41 AM, Ivan Gerasimov wrote:
Chris, if it's not too late, I'd like to include a regtest in the fix.
Here's webrev that includes the test:
http://cr.openjdk.java.net/~igerasim/8022584/3/webrev/
The test gets past with the fixed jdk8 and fails with jdk8-b101 and
jdk7
as expected.
Sincerely yours,
Ivan
On 08.08.2013 17:50, Chris Hegarty wrote:
Looks good to me. Thanks Ivan.
-Chris.
On 08/08/2013 02:33 PM, Ivan Gerasimov wrote:
Hello Chris!
Here's the update:
http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/
Thanks for offering the sponsorship!
Here's the hg-export
http://cr.openjdk.java.net/~igerasim/2commit/8022584-jdk8-Memleak-in-NetworkInterface.patch
Sincerely yours,
Ivan
On 08.08.2013 12:43, Chris Hegarty wrote:
Thanks for taking this Ivan.
Can you please make the changes suggested by both David and Alan (
simply return NULL/-1/JNI_FALSE, as appropriate, if
GetStringUTFChars
fails ( returns NULL ), then I will sponsor this change into jdk8
for
you.
Please post an update webrev to cr.openjdk.java.net.
-Chris.
On 08/08/2013 06:01 AM, David Holmes wrote:
Ivan,
On 8/08/2013 2:05 PM, Ivan Gerasimov wrote:
David, Alan,
I added checking for NULL results and throwing OOMException if
necessary.
You don't need to throw it yourself:
JNU_ThrowOutOfMemoryError(env, NULL);
Assuming a correct VM implementation if NULL is returned then an
OOME
should already be pending and will be thrown as soon as your native
code
returns to Java.
Thanks,
David
I've also added some spaces along the code to improve indentation.
Would you please review the updated webrev?
http://washi.ru.oracle.com/~igerasim/webrevs/8022584/1/webrev/
Sincerely yours,
Ivan
On 08.08.2013 5:35, David Holmes wrote:
On 8/08/2013 11:20 AM, Ivan Gerasimov wrote:
Thanks Alan!
I've checked that and it turns out that GetStringUTFChars cannot
return
NULL.
For allocation of the result string it calls AllocateHeap() with
the
default EXIT_OOM fail strategy.
Thus, in case of being unable to allocate memory it simply stops
VM.
Ouch! That is a hotspot bug. GetStringUTFChars is supposed to
throw
OutOfMemoryError if it has memory issues, not abort the VM!
I recommend that you check for NULL and/or a pending exception.
David
Sincerely yours,
Ivan
On 08.08.2013 5:05, Alan Bateman wrote:
(cc'ing net-dev).
The change looks okay to me. One suggestion (while you are
there) is
to check the return from GetStringUTFChars so that the name
returns
when it fails with NULL.
-Alan.
On 07/08/2013 17:46, Ivan Gerasimov wrote:
Hello everybody!
Some methods of NetworkInterface class were reported to leak
memory.
http://bugs.sun.com/view_bug.do?bug_id=JDK-8022584 (not yet
available.)
Examples are isUp() and isLoopback().
Affected versions of jdk are 6, 7 and 8
Would you please review a simple fix that removes the
unnecessary
allocation?
http://cr.openjdk.java.net/~igerasim/8022584/0/webrev/
Sincerely yours,
Ivan
Re: RFR [8022584] Memory leak in some NetworkInterface methods
Thanks David!
On 09.08.2013 1:15, David Holmes wrote:
Main fix looks good to me.
Regression test may need some tweaking eg I think othervm will be needed.
Yes, it's a good point.
Since there may be a memory leak in the test, it'd better not interfere
with other tests in jtreg.
Also this:
System.out.println("WARNING: Cannot perform memory leak detection on
this OS");
should probably just say something like "Test skipped on this OS" -
there are other tests that do this so just check if there is common
terminology. (In the future we may have keyword tags to flag this as
linux only etc.)
OK, I changed the phrase to "Test only runs on Linux".
Updated webrev is here:
http://cr.openjdk.java.net/~igerasim/8022584/4/webrev/
Updated export is at the same place:
http://cr.openjdk.java.net/~igerasim/2commit/8022584-jdk8-Memleak-in-NetworkInterface.patch
Sincerely yours,
Ivan
Thanks,
David
On 9/08/2013 4:41 AM, Ivan Gerasimov wrote:
Chris, if it's not too late, I'd like to include a regtest in the fix.
Here's webrev that includes the test:
http://cr.openjdk.java.net/~igerasim/8022584/3/webrev/
The test gets past with the fixed jdk8 and fails with jdk8-b101 and jdk7
as expected.
Sincerely yours,
Ivan
On 08.08.2013 17:50, Chris Hegarty wrote:
Looks good to me. Thanks Ivan.
-Chris.
On 08/08/2013 02:33 PM, Ivan Gerasimov wrote:
Hello Chris!
Here's the update:
http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/
Thanks for offering the sponsorship!
Here's the hg-export
http://cr.openjdk.java.net/~igerasim/2commit/8022584-jdk8-Memleak-in-NetworkInterface.patch
Sincerely yours,
Ivan
On 08.08.2013 12:43, Chris Hegarty wrote:
Thanks for taking this Ivan.
Can you please make the changes suggested by both David and Alan (
simply return NULL/-1/JNI_FALSE, as appropriate, if GetStringUTFChars
fails ( returns NULL ), then I will sponsor this change into jdk8 for
you.
Please post an update webrev to cr.openjdk.java.net.
-Chris.
On 08/08/2013 06:01 AM, David Holmes wrote:
Ivan,
On 8/08/2013 2:05 PM, Ivan Gerasimov wrote:
David, Alan,
I added checking for NULL results and throwing OOMException if
necessary.
You don't need to throw it yourself:
JNU_ThrowOutOfMemoryError(env, NULL);
Assuming a correct VM implementation if NULL is returned then an
OOME
should already be pending and will be thrown as soon as your native
code
returns to Java.
Thanks,
David
I've also added some spaces along the code to improve indentation.
Would you please review the updated webrev?
http://washi.ru.oracle.com/~igerasim/webrevs/8022584/1/webrev/
Sincerely yours,
Ivan
On 08.08.2013 5:35, David Holmes wrote:
On 8/08/2013 11:20 AM, Ivan Gerasimov wrote:
Thanks Alan!
I've checked that and it turns out that GetStringUTFChars cannot
return
NULL.
For allocation of the result string it calls AllocateHeap() with
the
default EXIT_OOM fail strategy.
Thus, in case of being unable to allocate memory it simply stops
VM.
Ouch! That is a hotspot bug. GetStringUTFChars is supposed to
throw
OutOfMemoryError if it has memory issues, not abort the VM!
I recommend that you check for NULL and/or a pending exception.
David
Sincerely yours,
Ivan
On 08.08.2013 5:05, Alan Bateman wrote:
(cc'ing net-dev).
The change looks okay to me. One suggestion (while you are
there) is
to check the return from GetStringUTFChars so that the name
returns
when it fails with NULL.
-Alan.
On 07/08/2013 17:46, Ivan Gerasimov wrote:
Hello everybody!
Some methods of NetworkInterface class were reported to leak
memory.
http://bugs.sun.com/view_bug.do?bug_id=JDK-8022584 (not yet
available.)
Examples are isUp() and isLoopback().
Affected versions of jdk are 6, 7 and 8
Would you please review a simple fix that removes the
unnecessary
allocation?
http://cr.openjdk.java.net/~igerasim/8022584/0/webrev/
Sincerely yours,
Ivan
Re: RFR [8022584] Memory leak in some NetworkInterface methods
Ivan,
Right, it's not worth trying to do the equivalent, whatever it is, for
Windows.
The test is fine with me.
Thanks
Michael
On 08/08/13 23:15, Ivan Gerasimov wrote:
Michael,
The test uses /proc/self/stat file to detect changes in virtual memory
usage.
This approach is specific for Linux.
That's why I included the check of OS in the test.
Sincerely yours,
Ivan
On 09.08.2013 1:38, Michael McMahon wrote:
Yes, definitely "othervm" would be required for the test. Also, why
skip other OS'es?
The fix is only for Linux, but it might catch future leaks on Windows.
The trouble with tests like this, is they sometimes don't age well.
Future changes in OS kernel behavior could cause problems but I guess
32MB is a fairly large difference.
So, it should be okay
Michael
On 08/08/13 22:15, David Holmes wrote:
Main fix looks good to me.
Regression test may need some tweaking eg I think othervm will be
needed.
Also this:
System.out.println("WARNING: Cannot perform memory leak detection
on this OS");
should probably just say something like "Test skipped on this OS" -
there are other tests that do this so just check if there is common
terminology. (In the future we may have keyword tags to flag this as
linux only etc.)
Thanks,
David
On 9/08/2013 4:41 AM, Ivan Gerasimov wrote:
Chris, if it's not too late, I'd like to include a regtest in the fix.
Here's webrev that includes the test:
http://cr.openjdk.java.net/~igerasim/8022584/3/webrev/
The test gets past with the fixed jdk8 and fails with jdk8-b101 and
jdk7
as expected.
Sincerely yours,
Ivan
On 08.08.2013 17:50, Chris Hegarty wrote:
Looks good to me. Thanks Ivan.
-Chris.
On 08/08/2013 02:33 PM, Ivan Gerasimov wrote:
Hello Chris!
Here's the update:
http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/
Thanks for offering the sponsorship!
Here's the hg-export
http://cr.openjdk.java.net/~igerasim/2commit/8022584-jdk8-Memleak-in-NetworkInterface.patch
Sincerely yours,
Ivan
On 08.08.2013 12:43, Chris Hegarty wrote:
Thanks for taking this Ivan.
Can you please make the changes suggested by both David and Alan (
simply return NULL/-1/JNI_FALSE, as appropriate, if
GetStringUTFChars
fails ( returns NULL ), then I will sponsor this change into
jdk8 for
you.
Please post an update webrev to cr.openjdk.java.net.
-Chris.
On 08/08/2013 06:01 AM, David Holmes wrote:
Ivan,
On 8/08/2013 2:05 PM, Ivan Gerasimov wrote:
David, Alan,
I added checking for NULL results and throwing OOMException if
necessary.
You don't need to throw it yourself:
JNU_ThrowOutOfMemoryError(env, NULL);
Assuming a correct VM implementation if NULL is returned then
an OOME
should already be pending and will be thrown as soon as your
native
code
returns to Java.
Thanks,
David
I've also added some spaces along the code to improve
indentation.
Would you please review the updated webrev?
http://washi.ru.oracle.com/~igerasim/webrevs/8022584/1/webrev/
Sincerely yours,
Ivan
On 08.08.2013 5:35, David Holmes wrote:
On 8/08/2013 11:20 AM, Ivan Gerasimov wrote:
Thanks Alan!
I've checked that and it turns out that GetStringUTFChars
cannot
return
NULL.
For allocation of the result string it calls AllocateHeap()
with
the
default EXIT_OOM fail strategy.
Thus, in case of being unable to allocate memory it simply
stops
VM.
Ouch! That is a hotspot bug. GetStringUTFChars is supposed to
throw
OutOfMemoryError if it has memory issues, not abort the VM!
I recommend that you check for NULL and/or a pending exception.
David
Sincerely yours,
Ivan
On 08.08.2013 5:05, Alan Bateman wrote:
(cc'ing net-dev).
The change looks okay to me. One suggestion (while you are
there) is
to check the return from GetStringUTFChars so that the name
returns
when it fails with NULL.
-Alan.
On 07/08/2013 17:46, Ivan Gerasimov wrote:
Hello everybody!
Some methods of NetworkInterface class were reported to leak
memory.
http://bugs.sun.com/view_bug.do?bug_id=JDK-8022584 (not yet
available.)
Examples are isUp() and isLoopback().
Affected versions of jdk are 6, 7 and 8
Would you please review a simple fix that removes the
unnecessary
allocation?
http://cr.openjdk.java.net/~igerasim/8022584/0/webrev/
Sincerely yours,
Ivan
Re: RFR [8022584] Memory leak in some NetworkInterface methods
Thank you Michael!
On 09.08.2013 2:19, Michael McMahon wrote:
Ivan,
Right, it's not worth trying to do the equivalent, whatever it is, for
Windows.
The test is fine with me.
Thanks
Michael
On 08/08/13 23:15, Ivan Gerasimov wrote:
Michael,
The test uses /proc/self/stat file to detect changes in virtual
memory usage.
This approach is specific for Linux.
That's why I included the check of OS in the test.
Sincerely yours,
Ivan
On 09.08.2013 1:38, Michael McMahon wrote:
Yes, definitely "othervm" would be required for the test. Also, why
skip other OS'es?
The fix is only for Linux, but it might catch future leaks on Windows.
The trouble with tests like this, is they sometimes don't age well.
Future changes in OS kernel behavior could cause problems but I
guess 32MB is a fairly large difference.
So, it should be okay
Michael
On 08/08/13 22:15, David Holmes wrote:
Main fix looks good to me.
Regression test may need some tweaking eg I think othervm will be
needed.
Also this:
System.out.println("WARNING: Cannot perform memory leak detection
on this OS");
should probably just say something like "Test skipped on this OS" -
there are other tests that do this so just check if there is common
terminology. (In the future we may have keyword tags to flag this
as linux only etc.)
Thanks,
David
On 9/08/2013 4:41 AM, Ivan Gerasimov wrote:
Chris, if it's not too late, I'd like to include a regtest in the
fix.
Here's webrev that includes the test:
http://cr.openjdk.java.net/~igerasim/8022584/3/webrev/
The test gets past with the fixed jdk8 and fails with jdk8-b101
and jdk7
as expected.
Sincerely yours,
Ivan
On 08.08.2013 17:50, Chris Hegarty wrote:
Looks good to me. Thanks Ivan.
-Chris.
On 08/08/2013 02:33 PM, Ivan Gerasimov wrote:
Hello Chris!
Here's the update:
http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/
Thanks for offering the sponsorship!
Here's the hg-export
http://cr.openjdk.java.net/~igerasim/2commit/8022584-jdk8-Memleak-in-NetworkInterface.patch
Sincerely yours,
Ivan
On 08.08.2013 12:43, Chris Hegarty wrote:
Thanks for taking this Ivan.
Can you please make the changes suggested by both David and Alan (
simply return NULL/-1/JNI_FALSE, as appropriate, if
GetStringUTFChars
fails ( returns NULL ), then I will sponsor this change into
jdk8 for
you.
Please post an update webrev to cr.openjdk.java.net.
-Chris.
On 08/08/2013 06:01 AM, David Holmes wrote:
Ivan,
On 8/08/2013 2:05 PM, Ivan Gerasimov wrote:
David, Alan,
I added checking for NULL results and throwing OOMException if
necessary.
You don't need to throw it yourself:
JNU_ThrowOutOfMemoryError(env, NULL);
Assuming a correct VM implementation if NULL is returned then
an OOME
should already be pending and will be thrown as soon as your
native
code
returns to Java.
Thanks,
David
I've also added some spaces along the code to improve
indentation.
Would you please review the updated webrev?
http://washi.ru.oracle.com/~igerasim/webrevs/8022584/1/webrev/
Sincerely yours,
Ivan
On 08.08.2013 5:35, David Holmes wrote:
On 8/08/2013 11:20 AM, Ivan Gerasimov wrote:
Thanks Alan!
I've checked that and it turns out that GetStringUTFChars
cannot
return
NULL.
For allocation of the result string it calls AllocateHeap()
with
the
default EXIT_OOM fail strategy.
Thus, in case of being unable to allocate memory it simply
stops
VM.
Ouch! That is a hotspot bug. GetStringUTFChars is supposed
to throw
OutOfMemoryError if it has memory issues, not abort the VM!
I recommend that you check for NULL and/or a pending exception.
David
Sincerely yours,
Ivan
On 08.08.2013 5:05, Alan Bateman wrote:
(cc'ing net-dev).
The change looks okay to me. One suggestion (while you are
there) is
to check the return from GetStringUTFChars so that the name
returns
when it fails with NULL.
-Alan.
On 07/08/2013 17:46, Ivan Gerasimov wrote:
Hello everybody!
Some methods of NetworkInterface class were reported to leak
memory.
http://bugs.sun.com/view_bug.do?bug_id=JDK-8022584 (not yet
available.)
Examples are isUp() and isLoopback().
Affected versions of jdk are 6, 7 and 8
Would you please review a simple fix that removes the
unnecessary
allocation?
http://cr.openjdk.java.net/~igerasim/8022584/0/webrev/
Sincerely yours,
Ivan
Re: Code review request, 8020842 IDN do not throw IAE when hostname ends with a trailing dot
Ping.
Thanks,
Xuelei
On 8/7/2013 11:17 PM, Xuelei Fan wrote:
> Please review the new update:
>
> http://cr.openjdk.java.net./~xuelei/8020842/webrev.01/
>
> With this update, "com." is valid (return "com."); "." and
> "example..com" are invalid. And IAE will be thrown for invalid IDN.
>
> Thanks,
> Xuelei
>
> On 8/7/2013 10:18 PM, Michael McMahon wrote:
>> On 07/08/13 15:13, Xuelei Fan wrote:
>>> On 8/7/2013 10:05 PM, Michael McMahon wrote:
Resolvers seem to accept queries using trailing dots.
eg nslookup www.oracle.com.
or InetAddress.getByName("www.oracle.com.");
The part of RFC3490 quoted below seems to me to be saying
that the empty label implied by the trailing dot is not regarded
as a label so that you don't end up calling toAscii() or toUnicode()
with an empty string. I don't think it's saying the trailing dot can't
be there.
>>> It makes sense.
>>>
>>> What's your preference to return for IDN.toASCII("www.oracle.com."),
>>> "www.oracle.com." or "www.oracle.com"? The current returned value is
>>> "www.oracle.com". I would like to reserve the behavior in this update.
>>
>> My opinion is to keep it as at present ie. "www.oracle.com."
>>
>> Michael
>>
>>> I think we are on same page soon.
>>>
>>> Thanks,
>>> Xuelei
>>>
Michael
On 07/08/13 13:44, Xuelei Fan wrote:
> On 8/7/2013 12:06 AM, Matthew Hall wrote:
>> Trailing dots are allowed in plain DNS (thus almost surely in IDN),
>> and the single dot represents the root zone. So you have to be
>> careful making this sort of change to check the DNS RFCs first.
> That's the first question we need to answer, whether IDN allow tailling
> dots ("com."), zero-length root label ("."), and zero-length label ("",
> for example ""example..com")?
>
> Per the specification of IDN.toASCII():
> ===
> "ToASCII operation can fail. ToASCII fails if any step of it fails. If
> ToASCII operation fails, an IllegalArgumentException will be thrown. In
> this case, the input string should not be used in an internationalized
> domain name.
>
> A label is an individual part of a domain name. The original ToASCII
> operation, as defined in RFC 3490, only operates on a single label.
> This
> method can handle both label and entire domain name, by assuming that
> labels in a domain name are always separated by dots. ...
>
> Throws IllegalArgumentException - if the input string doesn't
> conform to
> RFC 3490 specification"
>
> Per the specification of RFC 3490:
> ==
> [section 2]
> "A label is an individual part of a domain name. Labels are usually
>shown separated by dots; for example, the domain name
>"www.example.com" is composed of three labels: "www", "example", and
>"com". (The zero-length root label described in [STD13], which can
>be explicit as in "www.example.com." or implicit as in
>"www.example.com", is not considered a label in this
> specification.)"
>
> "An "internationalized label" is a label to which the ToASCII
>operation (see section 4) can be applied without failing (with the
>UseSTD3ASCIIRules flag unset). ...
>Although most Unicode characters can appear in
>internationalized labels, ToASCII will fail for some input strings,
>and such strings are not valid internationalized labels."
>
> "An "internationalized domain name" (IDN) is a domain name in which
>every label is an internationalized label."
>
> [Section 4.1]
> "ToASCII consists of the following steps:
>
>...
>
>8. Verify that the number of code points is in the range 1 to 63
> inclusive."
>
>
> Here are the questions:
> 1. whether "example..com" is an valid IDN?
> As dot is used as label separators, there are three labels,
> "example", "", "com". Per RFC 3490, "" is not a valid label. Hence,
> "example..com" is not a valid IDN.
>
> We need to address the issue in IDN.
>
> 2. whether "xyz." is an valid IDN?
> It's an gray area, I think. We can treat the trailing "." as root
> label, or a label separator.
> If the trailing "." is treated as label separator, "xyz." is
> invalid
> per RFC 3490.
> if the trailing "." is treated as root label, what's the expected
> return value of IDN.toASCII("xyz.")? I think the return value can be
> either "xyz." or "xyz". The current implementation returns "xyz".
>
> We may need not to update the implementation if tailing "." is
> treated as root label.
>
> 3. whether "." is an valid IDN?
> It's an gray area again, I think.
> As above, if the trailing "." is treated as root label, I think
> the
> return val
Re: Code review request, 8020842 IDN do not throw IAE when hostname ends with a trailing dot
I tried nslookup. Those with ".." inside are illegal, $ nslookup com.. nslookup: 'com..' is not a legal name (empty label) but $ nslookup . Server: 192.168.10.1 Address:192.168.10.1#53 Non-authoritative answer: *** Can't find .: No answer Also, since this bug was originally about SNIHostName, do you need to add some extra restriction there to reject "oracle.com." things? Thanks Max On 8/9/13 8:41 AM, Xuelei Fan wrote: Ping. Thanks, Xuelei On 8/7/2013 11:17 PM, Xuelei Fan wrote: Please review the new update: http://cr.openjdk.java.net./~xuelei/8020842/webrev.01/ With this update, "com." is valid (return "com."); "." and "example..com" are invalid. And IAE will be thrown for invalid IDN. Thanks, Xuelei
Re: Code review request, 8020842 IDN do not throw IAE when hostname ends with a trailing dot
On 8/9/2013 9:22 AM, Weijun Wang wrote: > I tried nslookup. Those with ".." inside are illegal, > > $ nslookup com.. > nslookup: 'com..' is not a legal name (empty label) > > but > > $ nslookup . > Server:192.168.10.1 > Address:192.168.10.1#53 > > Non-authoritative answer: > *** Can't find .: No answer > Thanks for the testing. The behaviors are the same as this fix now. Learn something new today to use nslookup. > Also, since this bug was originally about SNIHostName, do you need to > add some extra restriction there to reject "oracle.com." things? > No, we cannot restrict the format of IDN in SNIHostName more than in IDN. However, we may need to rethink about the comparing of two IDN, for example, "example.com." should equal to "example.com". I want to consider it in another bug. Can I push the changeset? Thanks, Xuelei > Thanks > Max > > On 8/9/13 8:41 AM, Xuelei Fan wrote: >> Ping. >> >> Thanks, >> Xuelei >> >> On 8/7/2013 11:17 PM, Xuelei Fan wrote: >>> Please review the new update: >>> >>> http://cr.openjdk.java.net./~xuelei/8020842/webrev.01/ >>> >>> With this update, "com." is valid (return "com."); "." and >>> "example..com" are invalid. And IAE will be thrown for invalid IDN. >>> >>> Thanks, >>> Xuelei >>>
Re: Code review request, 8020842 IDN do not throw IAE when hostname ends with a trailing dot
On 8/9/13 9:37 AM, Xuelei Fan wrote: On 8/9/2013 9:22 AM, Weijun Wang wrote: I tried nslookup. Those with ".." inside are illegal, $ nslookup com.. nslookup: 'com..' is not a legal name (empty label) but $ nslookup . Server:192.168.10.1 Address:192.168.10.1#53 Non-authoritative answer: *** Can't find .: No answer Thanks for the testing. The behaviors are the same as this fix now. No exactly. It seems nslookup still regards "." legal but just cannot find an IP for it. Learn something new today to use nslookup. Also, since this bug was originally about SNIHostName, do you need to add some extra restriction there to reject "oracle.com." things? No, we cannot restrict the format of IDN in SNIHostName more than in IDN. However, we may need to rethink about the comparing of two IDN, for example, "example.com." should equal to "example.com". I want to consider it in another bug. Not sure. Does the spec say IDN and SNIHostName are equivalent sets? And it's not one is another's subset? Can I push the changeset? I think it's better to ask someone in the networking team to make the suggestion. From what I read Michael in this thread, he does not seem totally agreed with your code changes (at least not the 00 version). Thanks Max Thanks, Xuelei Thanks Max On 8/9/13 8:41 AM, Xuelei Fan wrote: Ping. Thanks, Xuelei On 8/7/2013 11:17 PM, Xuelei Fan wrote: Please review the new update: http://cr.openjdk.java.net./~xuelei/8020842/webrev.01/ With this update, "com." is valid (return "com."); "." and "example..com" are invalid. And IAE will be thrown for invalid IDN. Thanks, Xuelei
Re: Code review request, 8020842 IDN do not throw IAE when hostname ends with a trailing dot
On 8/9/2013 10:14 AM, Weijun Wang wrote:
>
>
> On 8/9/13 9:37 AM, Xuelei Fan wrote:
>> On 8/9/2013 9:22 AM, Weijun Wang wrote:
>>> I tried nslookup. Those with ".." inside are illegal,
>>>
>>> $ nslookup com..
>>> nslookup: 'com..' is not a legal name (empty label)
>>>
>>> but
>>>
>>> $ nslookup .
>>> Server:192.168.10.1
>>> Address:192.168.10.1#53
>>>
>>> Non-authoritative answer:
>>> *** Can't find .: No answer
>>>
>> Thanks for the testing. The behaviors are the same as this fix now.
>
> No exactly. It seems nslookup still regards "." legal but just cannot
> find an IP for it.
>
I'm not sure whether a root domain name can be stand alone. Root label
is not considered as a label in IDN. I think it is safe to regard that
"." is not a valid IDN as it contains no label. Anyway, it is a corner
case.
There are many online IDN conversion web services, some of them can
convert ".", some of the cannot. In the present implementation, we
cannot recognize ".", and IDN.toASCII(".") throws
StringIndexOutOfBoundsException. With this fix, I was wondering IAE is
a better exception for IDN.toASCII(".").
>>
>> Learn something new today to use nslookup.
>>
>>> Also, since this bug was originally about SNIHostName, do you need to
>>> add some extra restriction there to reject "oracle.com." things?
>>>
>> No, we cannot restrict the format of IDN in SNIHostName more than in
>> IDN. However, we may need to rethink about the comparing of two IDN, for
>> example, "example.com." should equal to "example.com". I want to
>> consider it in another bug.
>
> Not sure. Does the spec say IDN and SNIHostName are equivalent sets? And
> it's not one is another's subset?
>
Per TLS specification, host name in SNI is an IDN. The spec of
SNIHostname says, "hostname is not a valid Internationalized Domain Name
(IDN) compliant with the RFC 3490 specification". The spec in
SNIHostName has the same means as IDN. I won't want to add additional
restrict beyond the specification of an IDN.
Xuelei
>>
>> Can I push the changeset?
>
> I think it's better to ask someone in the networking team to make the
> suggestion. From what I read Michael in this thread, he does not seem
> totally agreed with your code changes (at least not the 00 version).
>
> Thanks
> Max
>
>>
>> Thanks,
>> Xuelei
>>
>>> Thanks
>>> Max
>>>
>>> On 8/9/13 8:41 AM, Xuelei Fan wrote:
Ping.
Thanks,
Xuelei
On 8/7/2013 11:17 PM, Xuelei Fan wrote:
> Please review the new update:
>
> http://cr.openjdk.java.net./~xuelei/8020842/webrev.01/
>
> With this update, "com." is valid (return "com."); "." and
> "example..com" are invalid. And IAE will be thrown for invalid IDN.
>
> Thanks,
> Xuelei
>
>>
Re: Code review request, 8020842 IDN do not throw IAE when hostname ends with a trailing dot
But, DNS considers "." as the valid root zone...
--
Sent from my mobile device.
Xuelei Fan wrote:
>On 8/9/2013 10:14 AM, Weijun Wang wrote:
>>
>>
>> On 8/9/13 9:37 AM, Xuelei Fan wrote:
>>> On 8/9/2013 9:22 AM, Weijun Wang wrote:
I tried nslookup. Those with ".." inside are illegal,
$ nslookup com..
nslookup: 'com..' is not a legal name (empty label)
but
$ nslookup .
Server:192.168.10.1
Address:192.168.10.1#53
Non-authoritative answer:
*** Can't find .: No answer
>>> Thanks for the testing. The behaviors are the same as this fix now.
>>
>> No exactly. It seems nslookup still regards "." legal but just cannot
>> find an IP for it.
>>
>I'm not sure whether a root domain name can be stand alone. Root label
>is not considered as a label in IDN. I think it is safe to regard that
>"." is not a valid IDN as it contains no label. Anyway, it is a corner
>case.
>
>There are many online IDN conversion web services, some of them can
>convert ".", some of the cannot. In the present implementation, we
>cannot recognize ".", and IDN.toASCII(".") throws
>StringIndexOutOfBoundsException. With this fix, I was wondering IAE is
>a better exception for IDN.toASCII(".").
>
>>>
>>> Learn something new today to use nslookup.
>>>
Also, since this bug was originally about SNIHostName, do you need
>to
add some extra restriction there to reject "oracle.com." things?
>>> No, we cannot restrict the format of IDN in SNIHostName more than in
>>> IDN. However, we may need to rethink about the comparing of two IDN,
>for
>>> example, "example.com." should equal to "example.com". I want to
>>> consider it in another bug.
>>
>> Not sure. Does the spec say IDN and SNIHostName are equivalent sets?
>And
>> it's not one is another's subset?
>>
>Per TLS specification, host name in SNI is an IDN. The spec of
>SNIHostname says, "hostname is not a valid Internationalized Domain
>Name
>(IDN) compliant with the RFC 3490 specification". The spec in
>SNIHostName has the same means as IDN. I won't want to add additional
>restrict beyond the specification of an IDN.
>
>Xuelei
>
>>>
>>> Can I push the changeset?
>>
>> I think it's better to ask someone in the networking team to make the
>> suggestion. From what I read Michael in this thread, he does not seem
>> totally agreed with your code changes (at least not the 00 version).
>>
>> Thanks
>> Max
>>
>>>
>>> Thanks,
>>> Xuelei
>>>
Thanks
Max
On 8/9/13 8:41 AM, Xuelei Fan wrote:
> Ping.
>
> Thanks,
> Xuelei
>
> On 8/7/2013 11:17 PM, Xuelei Fan wrote:
>> Please review the new update:
>>
>> http://cr.openjdk.java.net./~xuelei/8020842/webrev.01/
>>
>> With this update, "com." is valid (return "com."); "." and
>> "example..com" are invalid. And IAE will be thrown for invalid
>IDN.
>>
>> Thanks,
>> Xuelei
>>
>>>
hg: jdk8/tl/jdk: 8021788: JarInputStream doesn't provide certificates for some file under META-INF
Changeset: 758e3117899c Author:weijun Date: 2013-08-09 11:41 +0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/758e3117899c 8021788: JarInputStream doesn't provide certificates for some file under META-INF Reviewed-by: chegar, sherman ! src/share/classes/java/util/jar/JarVerifier.java + test/java/util/jar/JarInputStream/ExtraFileInMetaInf.java
Re: Code review request, 8020842 IDN do not throw IAE when hostname ends with a trailing dot
On 8/9/2013 11:24 AM, Matthew Hall wrote:
> But, DNS considers "." as the valid root zone...
>
Good! Looks like that IDN.toASCII(".") should returns ".", so that a
general domain name can always use IDN.toASCII() conversion instead of
throwing runtime exception.
Xuelei
Re: Code review request, 8020842 IDN do not throw IAE when hostname ends with a trailing dot
Thanks for your feedback and suggestions.
Here is the new webrev:
http://cr.openjdk.java.net/~xuelei/8020842/webrev.02/
"." is regarded as valid IDN in this update.
Thanks,
Xuelei
On 8/9/2013 10:50 AM, Xuelei Fan wrote:
> On 8/9/2013 10:14 AM, Weijun Wang wrote:
>>
>>
>> On 8/9/13 9:37 AM, Xuelei Fan wrote:
>>> On 8/9/2013 9:22 AM, Weijun Wang wrote:
I tried nslookup. Those with ".." inside are illegal,
$ nslookup com..
nslookup: 'com..' is not a legal name (empty label)
but
$ nslookup .
Server:192.168.10.1
Address:192.168.10.1#53
Non-authoritative answer:
*** Can't find .: No answer
>>> Thanks for the testing. The behaviors are the same as this fix now.
>>
>> No exactly. It seems nslookup still regards "." legal but just cannot
>> find an IP for it.
>>
> I'm not sure whether a root domain name can be stand alone. Root label
> is not considered as a label in IDN. I think it is safe to regard that
> "." is not a valid IDN as it contains no label. Anyway, it is a corner
> case.
>
> There are many online IDN conversion web services, some of them can
> convert ".", some of the cannot. In the present implementation, we
> cannot recognize ".", and IDN.toASCII(".") throws
> StringIndexOutOfBoundsException. With this fix, I was wondering IAE is
> a better exception for IDN.toASCII(".").
>
>>>
>>> Learn something new today to use nslookup.
>>>
Also, since this bug was originally about SNIHostName, do you need to
add some extra restriction there to reject "oracle.com." things?
>>> No, we cannot restrict the format of IDN in SNIHostName more than in
>>> IDN. However, we may need to rethink about the comparing of two IDN, for
>>> example, "example.com." should equal to "example.com". I want to
>>> consider it in another bug.
>>
>> Not sure. Does the spec say IDN and SNIHostName are equivalent sets? And
>> it's not one is another's subset?
>>
> Per TLS specification, host name in SNI is an IDN. The spec of
> SNIHostname says, "hostname is not a valid Internationalized Domain Name
> (IDN) compliant with the RFC 3490 specification". The spec in
> SNIHostName has the same means as IDN. I won't want to add additional
> restrict beyond the specification of an IDN.
>
> Xuelei
>
>>>
>>> Can I push the changeset?
>>
>> I think it's better to ask someone in the networking team to make the
>> suggestion. From what I read Michael in this thread, he does not seem
>> totally agreed with your code changes (at least not the 00 version).
>>
>> Thanks
>> Max
>>
>>>
>>> Thanks,
>>> Xuelei
>>>
Thanks
Max
On 8/9/13 8:41 AM, Xuelei Fan wrote:
> Ping.
>
> Thanks,
> Xuelei
>
> On 8/7/2013 11:17 PM, Xuelei Fan wrote:
>> Please review the new update:
>>
>> http://cr.openjdk.java.net./~xuelei/8020842/webrev.01/
>>
>> With this update, "com." is valid (return "com."); "." and
>> "example..com" are invalid. And IAE will be thrown for invalid IDN.
>>
>> Thanks,
>> Xuelei
>>
>>>
>
Re: RFR [8022584] Memory leak in some NetworkInterface methods
Thumbs up!
Thanks,
David
On 9/08/2013 8:19 AM, Ivan Gerasimov wrote:
Thanks David!
On 09.08.2013 1:15, David Holmes wrote:
Main fix looks good to me.
Regression test may need some tweaking eg I think othervm will be needed.
Yes, it's a good point.
Since there may be a memory leak in the test, it'd better not interfere
with other tests in jtreg.
Also this:
System.out.println("WARNING: Cannot perform memory leak detection on
this OS");
should probably just say something like "Test skipped on this OS" -
there are other tests that do this so just check if there is common
terminology. (In the future we may have keyword tags to flag this as
linux only etc.)
OK, I changed the phrase to "Test only runs on Linux".
Updated webrev is here:
http://cr.openjdk.java.net/~igerasim/8022584/4/webrev/
Updated export is at the same place:
http://cr.openjdk.java.net/~igerasim/2commit/8022584-jdk8-Memleak-in-NetworkInterface.patch
Sincerely yours,
Ivan
Thanks,
David
On 9/08/2013 4:41 AM, Ivan Gerasimov wrote:
Chris, if it's not too late, I'd like to include a regtest in the fix.
Here's webrev that includes the test:
http://cr.openjdk.java.net/~igerasim/8022584/3/webrev/
The test gets past with the fixed jdk8 and fails with jdk8-b101 and jdk7
as expected.
Sincerely yours,
Ivan
On 08.08.2013 17:50, Chris Hegarty wrote:
Looks good to me. Thanks Ivan.
-Chris.
On 08/08/2013 02:33 PM, Ivan Gerasimov wrote:
Hello Chris!
Here's the update:
http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/
Thanks for offering the sponsorship!
Here's the hg-export
http://cr.openjdk.java.net/~igerasim/2commit/8022584-jdk8-Memleak-in-NetworkInterface.patch
Sincerely yours,
Ivan
On 08.08.2013 12:43, Chris Hegarty wrote:
Thanks for taking this Ivan.
Can you please make the changes suggested by both David and Alan (
simply return NULL/-1/JNI_FALSE, as appropriate, if GetStringUTFChars
fails ( returns NULL ), then I will sponsor this change into jdk8 for
you.
Please post an update webrev to cr.openjdk.java.net.
-Chris.
On 08/08/2013 06:01 AM, David Holmes wrote:
Ivan,
On 8/08/2013 2:05 PM, Ivan Gerasimov wrote:
David, Alan,
I added checking for NULL results and throwing OOMException if
necessary.
You don't need to throw it yourself:
JNU_ThrowOutOfMemoryError(env, NULL);
Assuming a correct VM implementation if NULL is returned then an
OOME
should already be pending and will be thrown as soon as your native
code
returns to Java.
Thanks,
David
I've also added some spaces along the code to improve indentation.
Would you please review the updated webrev?
http://washi.ru.oracle.com/~igerasim/webrevs/8022584/1/webrev/
Sincerely yours,
Ivan
On 08.08.2013 5:35, David Holmes wrote:
On 8/08/2013 11:20 AM, Ivan Gerasimov wrote:
Thanks Alan!
I've checked that and it turns out that GetStringUTFChars cannot
return
NULL.
For allocation of the result string it calls AllocateHeap() with
the
default EXIT_OOM fail strategy.
Thus, in case of being unable to allocate memory it simply stops
VM.
Ouch! That is a hotspot bug. GetStringUTFChars is supposed to
throw
OutOfMemoryError if it has memory issues, not abort the VM!
I recommend that you check for NULL and/or a pending exception.
David
Sincerely yours,
Ivan
On 08.08.2013 5:05, Alan Bateman wrote:
(cc'ing net-dev).
The change looks okay to me. One suggestion (while you are
there) is
to check the return from GetStringUTFChars so that the name
returns
when it fails with NULL.
-Alan.
On 07/08/2013 17:46, Ivan Gerasimov wrote:
Hello everybody!
Some methods of NetworkInterface class were reported to leak
memory.
http://bugs.sun.com/view_bug.do?bug_id=JDK-8022584 (not yet
available.)
Examples are isUp() and isLoopback().
Affected versions of jdk are 6, 7 and 8
Would you please review a simple fix that removes the
unnecessary
allocation?
http://cr.openjdk.java.net/~igerasim/8022584/0/webrev/
Sincerely yours,
Ivan
Re: RFR [8022584] Memory leak in some NetworkInterface methods
Sorry I messed this up. The JNI book says GetStringUTFChars will return
NULL and post OOME but the JNI spec (latest version 6.0) does not - it
only says it will return NULL on failure.
So your previous version was the more correct. Given we just failed to
allocate C-heap I think we are on thin ice anyway, but better to at
least attempt to do the right thing.
FYI I filed 8022683 to fix GetStringUTFChars.
David
-
On 9/08/2013 3:21 PM, David Holmes wrote:
Thumbs up!
Thanks,
David
On 9/08/2013 8:19 AM, Ivan Gerasimov wrote:
Thanks David!
On 09.08.2013 1:15, David Holmes wrote:
Main fix looks good to me.
Regression test may need some tweaking eg I think othervm will be
needed.
Yes, it's a good point.
Since there may be a memory leak in the test, it'd better not interfere
with other tests in jtreg.
Also this:
System.out.println("WARNING: Cannot perform memory leak detection on
this OS");
should probably just say something like "Test skipped on this OS" -
there are other tests that do this so just check if there is common
terminology. (In the future we may have keyword tags to flag this as
linux only etc.)
OK, I changed the phrase to "Test only runs on Linux".
Updated webrev is here:
http://cr.openjdk.java.net/~igerasim/8022584/4/webrev/
Updated export is at the same place:
http://cr.openjdk.java.net/~igerasim/2commit/8022584-jdk8-Memleak-in-NetworkInterface.patch
Sincerely yours,
Ivan
Thanks,
David
On 9/08/2013 4:41 AM, Ivan Gerasimov wrote:
Chris, if it's not too late, I'd like to include a regtest in the fix.
Here's webrev that includes the test:
http://cr.openjdk.java.net/~igerasim/8022584/3/webrev/
The test gets past with the fixed jdk8 and fails with jdk8-b101 and
jdk7
as expected.
Sincerely yours,
Ivan
On 08.08.2013 17:50, Chris Hegarty wrote:
Looks good to me. Thanks Ivan.
-Chris.
On 08/08/2013 02:33 PM, Ivan Gerasimov wrote:
Hello Chris!
Here's the update:
http://cr.openjdk.java.net/~igerasim/8022584/2/webrev/
Thanks for offering the sponsorship!
Here's the hg-export
http://cr.openjdk.java.net/~igerasim/2commit/8022584-jdk8-Memleak-in-NetworkInterface.patch
Sincerely yours,
Ivan
On 08.08.2013 12:43, Chris Hegarty wrote:
Thanks for taking this Ivan.
Can you please make the changes suggested by both David and Alan (
simply return NULL/-1/JNI_FALSE, as appropriate, if
GetStringUTFChars
fails ( returns NULL ), then I will sponsor this change into jdk8
for
you.
Please post an update webrev to cr.openjdk.java.net.
-Chris.
On 08/08/2013 06:01 AM, David Holmes wrote:
Ivan,
On 8/08/2013 2:05 PM, Ivan Gerasimov wrote:
David, Alan,
I added checking for NULL results and throwing OOMException if
necessary.
You don't need to throw it yourself:
JNU_ThrowOutOfMemoryError(env, NULL);
Assuming a correct VM implementation if NULL is returned then an
OOME
should already be pending and will be thrown as soon as your native
code
returns to Java.
Thanks,
David
I've also added some spaces along the code to improve indentation.
Would you please review the updated webrev?
http://washi.ru.oracle.com/~igerasim/webrevs/8022584/1/webrev/
Sincerely yours,
Ivan
On 08.08.2013 5:35, David Holmes wrote:
On 8/08/2013 11:20 AM, Ivan Gerasimov wrote:
Thanks Alan!
I've checked that and it turns out that GetStringUTFChars cannot
return
NULL.
For allocation of the result string it calls AllocateHeap() with
the
default EXIT_OOM fail strategy.
Thus, in case of being unable to allocate memory it simply stops
VM.
Ouch! That is a hotspot bug. GetStringUTFChars is supposed to
throw
OutOfMemoryError if it has memory issues, not abort the VM!
I recommend that you check for NULL and/or a pending exception.
David
Sincerely yours,
Ivan
On 08.08.2013 5:05, Alan Bateman wrote:
(cc'ing net-dev).
The change looks okay to me. One suggestion (while you are
there) is
to check the return from GetStringUTFChars so that the name
returns
when it fails with NULL.
-Alan.
On 07/08/2013 17:46, Ivan Gerasimov wrote:
Hello everybody!
Some methods of NetworkInterface class were reported to leak
memory.
http://bugs.sun.com/view_bug.do?bug_id=JDK-8022584 (not yet
available.)
Examples are isUp() and isLoopback().
Affected versions of jdk are 6, 7 and 8
Would you please review a simple fix that removes the
unnecessary
allocation?
http://cr.openjdk.java.net/~igerasim/8022584/0/webrev/
Sincerely yours,
Ivan
Re: Code review request, 8020842 IDN do not throw IAE when hostname ends with a trailing dot
Xuelei,
119 p = q + 1;
120 if (p < input.length() || q == (input.length() - 1)) {
Could be simplified to:
q <= input.length()-1
-Dmitry
On 2013-08-09 04:41, Xuelei Fan wrote:
> Ping.
>
> Thanks,
> Xuelei
>
> On 8/7/2013 11:17 PM, Xuelei Fan wrote:
>> Please review the new update:
>>
>> http://cr.openjdk.java.net./~xuelei/8020842/webrev.01/
>>
>> With this update, "com." is valid (return "com."); "." and
>> "example..com" are invalid. And IAE will be thrown for invalid IDN.
>>
>> Thanks,
>> Xuelei
>>
>> On 8/7/2013 10:18 PM, Michael McMahon wrote:
>>> On 07/08/13 15:13, Xuelei Fan wrote:
On 8/7/2013 10:05 PM, Michael McMahon wrote:
> Resolvers seem to accept queries using trailing dots.
>
> eg nslookup www.oracle.com.
>
> or InetAddress.getByName("www.oracle.com.");
>
> The part of RFC3490 quoted below seems to me to be saying
> that the empty label implied by the trailing dot is not regarded
> as a label so that you don't end up calling toAscii() or toUnicode()
> with an empty string. I don't think it's saying the trailing dot can't
> be there.
>
It makes sense.
What's your preference to return for IDN.toASCII("www.oracle.com."),
"www.oracle.com." or "www.oracle.com"? The current returned value is
"www.oracle.com". I would like to reserve the behavior in this update.
>>>
>>> My opinion is to keep it as at present ie. "www.oracle.com."
>>>
>>> Michael
>>>
I think we are on same page soon.
Thanks,
Xuelei
> Michael
>
> On 07/08/13 13:44, Xuelei Fan wrote:
>> On 8/7/2013 12:06 AM, Matthew Hall wrote:
>>> Trailing dots are allowed in plain DNS (thus almost surely in IDN),
>>> and the single dot represents the root zone. So you have to be
>>> careful making this sort of change to check the DNS RFCs first.
>> That's the first question we need to answer, whether IDN allow tailling
>> dots ("com."), zero-length root label ("."), and zero-length label ("",
>> for example ""example..com")?
>>
>> Per the specification of IDN.toASCII():
>> ===
>> "ToASCII operation can fail. ToASCII fails if any step of it fails. If
>> ToASCII operation fails, an IllegalArgumentException will be thrown. In
>> this case, the input string should not be used in an internationalized
>> domain name.
>>
>> A label is an individual part of a domain name. The original ToASCII
>> operation, as defined in RFC 3490, only operates on a single label.
>> This
>> method can handle both label and entire domain name, by assuming that
>> labels in a domain name are always separated by dots. ...
>>
>> Throws IllegalArgumentException - if the input string doesn't
>> conform to
>> RFC 3490 specification"
>>
>> Per the specification of RFC 3490:
>> ==
>> [section 2]
>> "A label is an individual part of a domain name. Labels are usually
>>shown separated by dots; for example, the domain name
>>"www.example.com" is composed of three labels: "www", "example", and
>>"com". (The zero-length root label described in [STD13], which can
>>be explicit as in "www.example.com." or implicit as in
>>"www.example.com", is not considered a label in this
>> specification.)"
>>
>> "An "internationalized label" is a label to which the ToASCII
>>operation (see section 4) can be applied without failing (with the
>>UseSTD3ASCIIRules flag unset). ...
>>Although most Unicode characters can appear in
>>internationalized labels, ToASCII will fail for some input strings,
>>and such strings are not valid internationalized labels."
>>
>> "An "internationalized domain name" (IDN) is a domain name in which
>>every label is an internationalized label."
>>
>> [Section 4.1]
>> "ToASCII consists of the following steps:
>>
>>...
>>
>>8. Verify that the number of code points is in the range 1 to 63
>> inclusive."
>>
>>
>> Here are the questions:
>> 1. whether "example..com" is an valid IDN?
>> As dot is used as label separators, there are three labels,
>> "example", "", "com". Per RFC 3490, "" is not a valid label. Hence,
>> "example..com" is not a valid IDN.
>>
>> We need to address the issue in IDN.
>>
>> 2. whether "xyz." is an valid IDN?
>> It's an gray area, I think. We can treat the trailing "." as root
>> label, or a label separator.
>> If the trailing "." is treated as label separator, "xyz." is
>> invalid
>> per RFC 3490.
>> if the trailing "." is treated as root label, what's the expected
>> return value of IDN.toASCII("xyz.")? I think the return value can be
>> either "xyz." or "xyz". The current
