Code Review 7041800: URI.equals may incorrectly return true with escaped octets
An embarrassing bug in the equal(String,String) private implementation method. The Strings compare equal when there is a % in one string, a different char in the same place in the other string, but the two chars following are the same. http://cr.openjdk.java.net/~chegar/7041800/webrev.00/webrev/ -Chris.
Re: Code Review 7041800: URI.equals may incorrectly return true with escaped octets
Chris Hegarty wrote: An embarrassing bug in the equal(String,String) private implementation method. The Strings compare equal when there is a % in one string, a different char in the same place in the other string, but the two chars following are the same. http://cr.openjdk.java.net/~chegar/7041800/webrev.00/webrev/ -Chris. The fix looks fine. For the test then I think it would be better to add it to the existing unit test rather than adding a new test. -Alan.
Re: Code Review 7041800: URI.equals may incorrectly return true with escaped octets
On 01/09/11 09:36, Chris Hegarty wrote: An embarrassing bug in the equal(String,String) private implementation method. The Strings compare equal when there is a % in one string, a different char in the same place in the other string, but the two chars following are the same. http://cr.openjdk.java.net/~chegar/7041800/webrev.00/webrev/ -Chris. Change looks fine. I just wonder if the test should go into the existing mega-test, rather than creating a second file just for this bug? - Michael.
Re: Code Review 7041800: URI.equals may incorrectly return true with escaped octets
On 09/ 1/11 11:40 AM, Michael McMahon wrote: On 01/09/11 09:36, Chris Hegarty wrote: ... Change looks fine. I just wonder if the test should go into the existing mega-test, rather than creating a second file just for this bug? Agreed, I added it to the mega-test. Updated webrev: http://cr.openjdk.java.net/~chegar/7041800/webrev.01/webrev/ -Chris. - Michael.
Re: Code Review 7041800: URI.equals may incorrectly return true with escaped octets
Chris Hegarty wrote: Agreed, I added it to the mega-test. Updated webrev: http://cr.openjdk.java.net/~chegar/7041800/webrev.01/webrev/ Thanks for putting it in the unit test. Looks fine now. -Alan.
hg: jdk8/tl/jdk: 7041800: URI.equals may incorrectly return true with escaped octets
Changeset: fcb33500b325 Author:chegar Date: 2011-09-01 13:53 +0100 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/fcb33500b325 7041800: URI.equals may incorrectly return true with escaped octets Reviewed-by: alanb, michaelm ! src/share/classes/java/net/URI.java ! test/java/net/URI/Test.java
Code Review Request: 7084032: test/java/net/Inet6Address/B6558853.java fails on Windows XP/2003 if IPv6 enabled
Hi,
test/java/net/Inet6Address/B6558853.java tests if the address obtained from
getHostAddress() on connections
using IPv6 link-local addresses contains the zone id. For
Inet6Address.getHostAddress() to return the zone id,
Inet6Address.scope_id_set needs to be set to true in addition to setting the
appropriate Inet6Address.scope_id.
In case of Windows XP/2003 with IPv6 enabled, the native socket implementation
of socketAccept() method in
src/windows/native/java/net/TwoStacksPlainSocketImpl.c does not set the
scope_id_set to true which causes
the zone id not to be returned, and B6558853 throws a RuntimeException.
The fix is to simply set the scopeidsetID in socketAccept() method to true if
the scope_id is greater than 0.
Submitting hg diff:
diff --git a/src/windows/native/java/net/TwoStacksPlainSocketImpl.c
b/src/windows/native/java/net/TwoStacksPlainSocketImpl.c
--- a/src/windows/native/java/net/TwoStacksPlainSocketImpl.c
+++ b/src/windows/native/java/net/TwoStacksPlainSocketImpl.c
@@ -576,6 +576,7 @@ Java_java_net_TwoStacksPlainSocketImpl_s
{
/* fields on this */
jint port;
+jint scope;
jint timeout = (*env)->GetIntField(env, this, psi_timeoutID);
jobject fdObj = (*env)->GetObjectField(env, this, psi_fdID);
jobject fd1Obj = (*env)->GetObjectField(env, this, psi_fd1ID);
@@ -755,7 +756,11 @@ Java_java_net_TwoStacksPlainSocketImpl_s
addr = (*env)->GetObjectField (env, socketAddressObj, ia6_ipaddressID);
(*env)->SetByteArrayRegion (env, addr, 0, 16, (const char
*)&him.him6.sin6_addr);
(*env)->SetIntField(env, socketAddressObj, ia_familyID, IPv6);
-(*env)->SetIntField(env, socketAddressObj, ia6_scopeidID,
him.him6.sin6_scope_id);
+scope=him.him6.sin6_scope_id;
+(*env)->SetIntField(env, socketAddressObj, ia6_scopeidID, scope);
+if(scope>0) {
+(*env)->SetBooleanField(env, socketAddressObj, ia6_scopeidsetID,
JNI_TRUE);
+}
}
/* fields common to AF_INET and AF_INET6 */
Thanks,
--
-Kurchi
Re: Datagram socket leak
Thomas,
Thanks for pointing this out. We overlooked this in the recent change in
this
area. One thing though, in the second change to DatagramSocket we can't
just check for isBound() since the socket might legitimately be unbound
(bindaddr is null). All I can think is that we catch the exception and
re throw
it, after closing, rather than use a finally() in that case. I have
created a bug report (7085981)
to track this. I'll post a webrev for it soon.
- Michael.
On 29/08/11 20:01, Salter, Thomas A wrote:
Here's what I changed. I'm working with the fcs source bundle for b147,
27_jun_2011, so I may not have the latest source base.
Left base folder: new
Right base folder: b147
File: src\share\classes\java\net\DatagramSocket.java
186,189d185
< finally {
< if( !isBound() )
< close();
< }
234d229
RE: Datagram socket leak
Maybe I posted a bad patch, but my intent was to do the try-finally after
checking for a non-null bind address.
public DatagramSocket(SocketAddress bindaddr) throws SocketException {
// create a datagram socket.
createImpl();
if (bindaddr != null) {
try {
bind(bindaddr);
} finally {
if( !isBound() )
close();
}
}
}
On a related note, I've also noticed that the Socket and ServerSocket
constructors can throw without closing the implementation, but this only
happens with IllegalArgumentException or other RuntimeExceptions. I'm not sure
what your policy is on cleaning up after runtime exceptions.
-Original Message-
From: Michael McMahon [mailto:michael.x.mcma...@oracle.com]
Sent: Thursday, September 01, 2011 12:47 PM
To: Salter, Thomas A
Cc: net-dev@openjdk.java.net
Subject: Re: Datagram socket leak
Thomas,
Thanks for pointing this out. We overlooked this in the recent change in
this
area. One thing though, in the second change to DatagramSocket we can't
just check for isBound() since the socket might legitimately be unbound
(bindaddr is null). All I can think is that we catch the exception and
re throw
it, after closing, rather than use a finally() in that case. I have
created a bug report (7085981)
to track this. I'll post a webrev for it soon.
- Michael.
On 29/08/11 20:01, Salter, Thomas A wrote:
> Here's what I changed. I'm working with the fcs source bundle for b147,
> 27_jun_2011, so I may not have the latest source base.
>
> Left base folder: new
> Right base folder: b147
>
> File: src\share\classes\java\net\DatagramSocket.java
> 186,189d185
> < finally {
> < if( !isBound() )
> < close();
> < }
> 234d229
> 236,239d230
> <} finally {
> <}
>
> File: src\share\classes\java\net\MulticastSocket.java
> 165d164
> 167,170d165
> <} finally {
> <}
>
>
>
>
> -Original Message-
> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
> Sent: Monday, August 29, 2011 2:33 PM
> To: Salter, Thomas A
> Cc: net-dev@openjdk.java.net
> Subject: Re: Datagram socket leak
>
> Ah ok. I finally get it.
>
> In which case I think you original changes should be fine. Do you want
> to make similar changes to MulticastSocket and post the diffs?
>
> Also, I think a testcase would be useful here. I know it's not strictly
> specified that the socket should be closed if the constructor throws,
> but it does seem desirable.
>
> -Chris.
>
> On 08/29/11 07:14 PM, Salter, Thomas A wrote:
>> I believe you're referring to the close() in the catch clause following the
>> call to getImpl().bind. The problem I encountered was when the
>> Datagram.bind threw an exception before it got that far. In my case, the
>> checkListen was throwing a SecurityException, but any of the earlier throws
>> would cause the same problem. The SecurityException wouldn't have been
>> caught by the catch addressed by the CR in any case. We encountered this
>> while running the TCK. One of its tests tries to create lots of sockets,
>> all of them getting security violations until we hit a limit on the number
>> of open sockets.
>>
>> public synchronized void bind(SocketAddress addr) throws
>> SocketException {
>> if (isClosed())
>> throw new SocketException("Socket is closed");
>> if (isBound())
>> throw new SocketException("already bound");
>> if (addr == null)
>> addr = new InetSocketAddress(0);
>> if (!(addr instanceof InetSocketAddress))
>> throw new IllegalArgumentException("Unsupported address
>> type!");
>> InetSocketAddress epoint = (InetSocketAddress) addr;
>> if (epoint.isUnresolved())
>> throw new SocketException("Unresolved address");
>> InetAddress iaddr = epoint.getAddress();
>> int port = epoint.getPort();
>> checkAddress(iaddr, "bind");
>> SecurityManager sec = System.getSecurityManager();
>> if (sec != null) {
>> sec.checkListen(port);<< This throws
>> a SecurityException
>> }
>> try {
>> getImpl().bind(port, iaddr);
>> } catch (SocketException e) {
>> getImpl().close();
>> throw e;
>> }
>> bound = true;
>> }
>>
>> Tom.
>>
>>
>> -Original Message-
>> From: Chris Hegarty [mailto:chris.hega...@oracle.com]
>> Sent: Monday, August 29, 2011 1:56 PM
>> To: Salter, Thomas A
>> Cc: net-dev@openjdk.java.net
>> Subject: Re: Datagram socket leak
>>
>> [take two!]
>>
>> Tom,
>>
>> This specific area of cod
Re: Datagram socket leak
Right. That would work assuming createImpl() doesn't leave the socket open
if it throws an exception, which seems to be the case.
On 01/09/11 18:05, Salter, Thomas A wrote:
Maybe I posted a bad patch, but my intent was to do the try-finally after
checking for a non-null bind address.
public DatagramSocket(SocketAddress bindaddr) throws SocketException {
// create a datagram socket.
createImpl();
if (bindaddr != null) {
try {
bind(bindaddr);
} finally {
if( !isBound() )
close();
}
}
}
On a related note, I've also noticed that the Socket and ServerSocket
constructors can throw without closing the implementation, but this only
happens with IllegalArgumentException or other RuntimeExceptions. I'm not sure
what your policy is on cleaning up after runtime exceptions.
The SocketImpl finalizer does eventually clean up. But, I agree we
shouldn't rely on that
especially in cases like the IAE explicitly thrown in the constructor.
We'll include those
cases in the fix for this bug as well.
Michael.
-Original Message-
From: Michael McMahon [mailto:michael.x.mcma...@oracle.com]
Sent: Thursday, September 01, 2011 12:47 PM
To: Salter, Thomas A
Cc: net-dev@openjdk.java.net
Subject: Re: Datagram socket leak
Thomas,
Thanks for pointing this out. We overlooked this in the recent change in
this
area. One thing though, in the second change to DatagramSocket we can't
just check for isBound() since the socket might legitimately be unbound
(bindaddr is null). All I can think is that we catch the exception and
re throw
it, after closing, rather than use a finally() in that case. I have
created a bug report (7085981)
to track this. I'll post a webrev for it soon.
- Michael.
On 29/08/11 20:01, Salter, Thomas A wrote:
Here's what I changed. I'm working with the fcs source bundle for b147,
27_jun_2011, so I may not have the latest source base.
Left base folder: new
Right base folder: b147
File: src\share\classes\java\net\DatagramSocket.java
186,189d185
< finally {
< if( !isBound() )
< close();
< }
234d229
< try {
236,239d230
< } finally {
< if( !isBound() )
< close();
< }
File: src\share\classes\java\net\MulticastSocket.java
165d164
< try {
167,170d165
< } finally {
< if( !isBound() )
< close();
< }
-Original Message-
From: Chris Hegarty [mailto:chris.hega...@oracle.com]
Sent: Monday, August 29, 2011 2:33 PM
To: Salter, Thomas A
Cc: net-dev@openjdk.java.net
Subject: Re: Datagram socket leak
Ah ok. I finally get it.
In which case I think you original changes should be fine. Do you want
to make similar changes to MulticastSocket and post the diffs?
Also, I think a testcase would be useful here. I know it's not strictly
specified that the socket should be closed if the constructor throws,
but it does seem desirable.
-Chris.
On 08/29/11 07:14 PM, Salter, Thomas A wrote:
I believe you're referring to the close() in the catch clause following the
call to getImpl().bind. The problem I encountered was when the Datagram.bind
threw an exception before it got that far. In my case, the checkListen was
throwing a SecurityException, but any of the earlier throws would cause the
same problem. The SecurityException wouldn't have been caught by the catch
addressed by the CR in any case. We encountered this while running the TCK.
One of its tests tries to create lots of sockets, all of them getting security
violations until we hit a limit on the number of open sockets.
public synchronized void bind(SocketAddress addr) throws SocketException
{
if (isClosed())
throw new SocketException("Socket is closed");
if (isBound())
throw new SocketException("already bound");
if (addr == null)
addr = new InetSocketAddress(0);
if (!(addr instanceof InetSocketAddress))
throw new IllegalArgumentException("Unsupported address type!");
InetSocketAddress epoint = (InetSocketAddress) addr;
if (epoint.isUnresolved())
throw new SocketException("Unresolved address");
InetAddress iaddr = epoint.getAddress();
int port = epoint.getPort();
checkAddress(iaddr, "bind");
SecurityManager sec = System.getSecurityManager();
if (sec != null) {
sec.checkListen(port);<
hg: jdk8/tl/langtools: 7086071: tools/javac/7079713/TestCircularClassfile.java fails on windows
Changeset: a45d78d26450 Author:jjh Date: 2011-09-01 14:35 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/langtools/rev/a45d78d26450 7086071: tools/javac/7079713/TestCircularClassfile.java fails on windows Summary: delete file before renaming another file to it Reviewed-by: jjg ! test/tools/javac/7079713/TestCircularClassfile.java
EHOSTUNREACH should not be considered as an exception when send on a loopback network interface.
Hi guys,
In some linuxes, when you bind on a loopback network interface, sendto
will be fail and errono will be set to EHOSTUNREACH.
In this situation, EHOSTUNREACH maybe need treat as INVAL: return false,
not throw an exception.
Patch is attached.
Does anyone interested in this issue?
--
Yours Charles
diff --git src/solaris/native/java/net/Inet4AddressImpl.c
src/solaris/native/java/net/Inet4AddressImpl.c
index 9a5b78e..52d18a8 100644
--- src/solaris/native/java/net/Inet4AddressImpl.c
+++ src/solaris/native/java/net/Inet4AddressImpl.c
@@ -391,10 +391,10 @@ ping4(JNIEnv *env, jint fd, struct sockaddr_in* him, jint
timeout,
sizeof(struct sockaddr));
if (n < 0 && errno != EINPROGRESS ) {
#ifdef __linux__
-if (errno != EINVAL)
+if (errno != EINVAL || errno != EHOSTUNREACH)
/*
* On some Linuxes, when bound to the loopback interface, sendto
- * will fail and errno will be set to EINVAL. When that happens,
+ * will fail and errno will be set to EINVAL or EHOSTUNREACH. When
that happens,
* don't throw an exception, just return false.
*/
#endif /*__linux__ */
@@ -549,9 +549,10 @@ Java_java_net_Inet4AddressImpl_isReachable0(JNIEnv *env,
jobject this,
case EADDRNOTAVAIL: /* address is not available on the remote
machine */
#ifdef __linux__
case EINVAL:
+case EHOSTUNREACH:
/*
* On some Linuxes, when bound to the loopback interface, connect
- * will fail and errno will be set to EINVAL. When that happens,
+ * will fail and errno will be set to EINVAL or EHOSTUNREACH. When
that happens,
* don't throw an exception, just return false.
*/
#endif /* __linux__ */
diff --git src/solaris/native/java/net/Inet6AddressImpl.c
src/solaris/native/java/net/Inet6AddressImpl.c
index bb5bae7..a71538d 100644
--- src/solaris/native/java/net/Inet6AddressImpl.c
+++ src/solaris/native/java/net/Inet6AddressImpl.c
@@ -509,10 +509,10 @@ ping6(JNIEnv *env, jint fd, struct sockaddr_in6* him,
jint timeout,
n = sendto(fd, sendbuf, plen, 0, (struct sockaddr*) him, sizeof(struct
sockaddr_in6));
if (n < 0 && errno != EINPROGRESS) {
#ifdef __linux__
-if (errno != EINVAL)
+if (errno != EINVAL || errno != EHOSTUNREACH)
/*
* On some Linuxes, when bound to the loopback interface, sendto
- * will fail and errno will be set to EINVAL. When that happens,
+ * will fail and errno will be set to EINVAL or EHOSTUNREACH. When
that happens,
* don't throw an exception, just return false.
*/
#endif /*__linux__ */
@@ -677,9 +677,10 @@ Java_java_net_Inet6AddressImpl_isReachable0(JNIEnv *env,
jobject this,
case EADDRNOTAVAIL: /* address is not available on the remote
machine */
#ifdef __linux__
case EINVAL:
+case EHOSTUNREACH:
/*
* On some Linuxes, when bound to the loopback interface, connect
- * will fail and errno will be set to EINVAL. When that happens,
+ * will fail and errno will be set to EINVAL or EHOSTUNREACH. When
that happens,
* don't throw an exception, just return false.
*/
#endif /* __linux__ */
Re: EHOSTUNREACH should not be considered as an exception when send on a loopback network interface.
On 09/02/2011 12:50 PM, Charles Lee wrote:
Hi guys,
In some linuxes, when you bind on a loopback network interface, sendto
will be fail and errono will be set to EHOSTUNREACH.
In this situation, EHOSTUNREACH maybe need treat as INVAL: return
false, not throw an exception.
Patch is attached.
Does anyone interested in this issue?
Sorry, I attached a wrong diff. Here is the right one.
--
Yours Charles
diff --git src/solaris/native/java/net/Inet4AddressImpl.c
src/solaris/native/java/net/Inet4AddressImpl.c
index 9a5b78e..d859cdc 100644
--- src/solaris/native/java/net/Inet4AddressImpl.c
+++ src/solaris/native/java/net/Inet4AddressImpl.c
@@ -391,10 +391,10 @@ ping4(JNIEnv *env, jint fd, struct sockaddr_in* him, jint
timeout,
sizeof(struct sockaddr));
if (n < 0 && errno != EINPROGRESS ) {
#ifdef __linux__
-if (errno != EINVAL)
+if (errno != EINVAL && errno != EHOSTUNREACH)
/*
* On some Linuxes, when bound to the loopback interface, sendto
- * will fail and errno will be set to EINVAL. When that happens,
+ * will fail and errno will be set to EINVAL or EHOSTUNREACH. When
that happens,
* don't throw an exception, just return false.
*/
#endif /*__linux__ */
@@ -549,9 +549,10 @@ Java_java_net_Inet4AddressImpl_isReachable0(JNIEnv *env,
jobject this,
case EADDRNOTAVAIL: /* address is not available on the remote
machine */
#ifdef __linux__
case EINVAL:
+case EHOSTUNREACH:
/*
* On some Linuxes, when bound to the loopback interface, connect
- * will fail and errno will be set to EINVAL. When that happens,
+ * will fail and errno will be set to EINVAL or EHOSTUNREACH. When
that happens,
* don't throw an exception, just return false.
*/
#endif /* __linux__ */
diff --git src/solaris/native/java/net/Inet6AddressImpl.c
src/solaris/native/java/net/Inet6AddressImpl.c
index bb5bae7..0ab5a3b 100644
--- src/solaris/native/java/net/Inet6AddressImpl.c
+++ src/solaris/native/java/net/Inet6AddressImpl.c
@@ -509,10 +509,10 @@ ping6(JNIEnv *env, jint fd, struct sockaddr_in6* him,
jint timeout,
n = sendto(fd, sendbuf, plen, 0, (struct sockaddr*) him, sizeof(struct
sockaddr_in6));
if (n < 0 && errno != EINPROGRESS) {
#ifdef __linux__
-if (errno != EINVAL)
+if (errno != EINVAL && errno != EHOSTUNREACH)
/*
* On some Linuxes, when bound to the loopback interface, sendto
- * will fail and errno will be set to EINVAL. When that happens,
+ * will fail and errno will be set to EINVAL or EHOSTUNREACH. When
that happens,
* don't throw an exception, just return false.
*/
#endif /*__linux__ */
@@ -677,9 +677,10 @@ Java_java_net_Inet6AddressImpl_isReachable0(JNIEnv *env,
jobject this,
case EADDRNOTAVAIL: /* address is not available on the remote
machine */
#ifdef __linux__
case EINVAL:
+case EHOSTUNREACH:
/*
* On some Linuxes, when bound to the loopback interface, connect
- * will fail and errno will be set to EINVAL. When that happens,
+ * will fail and errno will be set to EINVAL or EHOSTUNREACH. When
that happens,
* don't throw an exception, just return false.
*/
#endif /* __linux__ */
hg: jdk8/tl/jdk: 7082971: More performance tuning of BigDecimal and other java.math classes
Changeset: ffada2ce20e5 Author:darcy Date: 2011-09-01 23:00 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ffada2ce20e5 7082971: More performance tuning of BigDecimal and other java.math classes Reviewed-by: darcy Contributed-by: sergey.kukse...@oracle.com ! src/share/classes/java/math/BigDecimal.java ! src/share/classes/java/math/BigInteger.java ! src/share/classes/java/math/MutableBigInteger.java ! test/java/math/BigDecimal/DivideMcTests.java ! test/java/math/BigDecimal/FloatDoubleValueTests.java ! test/java/math/BigDecimal/RangeTests.java ! test/java/math/BigDecimal/StrippingZerosTest.java ! test/java/math/BigDecimal/ToPlainStringTests.java
