Re: BZ 66294 - optionally disable some SecurityManager checks
> 2022年11月8日 00:24，Mark Thomas 写道： > > Hi, > > BZ 66294 [1] highlights the performance impact in Tomcat of some additional > SecurityManager checks that were added to avoid AccessControlException when > using the EL API JAR outside of Tomcat. > > Details of the performance impact are in the bug report. > > I think we have a few options here. > > 1. Assume Tomcat 11 will remove the SecurityManager. No nothing for now and > advise the reporter to move to Tomcat 11 when available. > > 2. Do nothing. > > 3. Disable this check by default and an option (it will have to be a system > property) to enable it. > > 4. Something else. > > Thoughts? > > I am currently leaning towards 3 given that the performance impact is > noticeable and that the check isn't required in normal usage. +1 Han > > Mark > > > [1] https://bz.apache.org/bugzilla/show_bug.cgi?id=66294 > > - > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: BZ 66294 - optionally disable some SecurityManager checks
On 07/11/2022 21:08, Christopher Schultz wrote: Mark, On 11/7/22 11:24, Mark Thomas wrote: Hi, BZ 66294 [1] highlights the performance impact in Tomcat of some additional SecurityManager checks that were added to avoid AccessControlException when using the EL API JAR outside of Tomcat. Details of the performance impact are in the bug report. I think we have a few options here. 1. Assume Tomcat 11 will remove the SecurityManager. No nothing for now and advise the reporter to move to Tomcat 11 when available. 2. Do nothing. 3. Disable this check by default and an option (it will have to be a system property) to enable it. 4. Something else. Thoughts? I am currently leaning towards 3 given that the performance impact is noticeable and that the check isn't required in normal usage. I thought we only wrapped stuff in doPrivileged() when a SecurityManager was installed. Re-re-reading the bug report, it's clear that the reporter IS running under SM. If the reporter is running under SM and the code does not fail, doesn't that mean that the check isn't actually providing any benefit? The thread must already be running in a privileged context if making that call does not throw an exception at runtime. Can we just remove it entirely? Maybe I'm missing something... When used in Tomcat that code is always called from within another doPrivileged() call further up the stack and all the stack frames inbetween are for Tomcat provided code so the security checks pass. Hence you can skip this doPrivileged() if running in Tomcat. When used outside of Tomcat (EL is completely stand-alone) that isn't the case and you need to use doPrivileged() to avoid the exceptions. Mark - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: BZ 66294 - optionally disable some SecurityManager checks
Mark, On 11/7/22 11:24, Mark Thomas wrote: Hi, BZ 66294 [1] highlights the performance impact in Tomcat of some additional SecurityManager checks that were added to avoid AccessControlException when using the EL API JAR outside of Tomcat. Details of the performance impact are in the bug report. I think we have a few options here. 1. Assume Tomcat 11 will remove the SecurityManager. No nothing for now and advise the reporter to move to Tomcat 11 when available. 2. Do nothing. 3. Disable this check by default and an option (it will have to be a system property) to enable it. 4. Something else. Thoughts? I am currently leaning towards 3 given that the performance impact is noticeable and that the check isn't required in normal usage. I thought we only wrapped stuff in doPrivileged() when a SecurityManager was installed. Re-re-reading the bug report, it's clear that the reporter IS running under SM. If the reporter is running under SM and the code does not fail, doesn't that mean that the check isn't actually providing any benefit? The thread must already be running in a privileged context if making that call does not throw an exception at runtime. Can we just remove it entirely? Maybe I'm missing something... -chris - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: [VOTE] Release Apache Tomcat Native 2.0.2
Mark, On 11/2/22 16:57, Mark Thomas wrote: The key differences of version 2.0.2 compared to 2.0.1 are: - Update the minimum supported version of LibreSSL to 3.5.2. Based on a #13 provided by orbea. - The windows binaries in this release have been built with OpenSSL 3.0.7 The 2.0.x branch is primarily intended for use with Tomcat 10.1.x but can be used with earlier versions as long as the APR/native connector is not used. The proposed release artefacts can be found at [1], and the build was done using tag [2]. The Apache Tomcat Native 2.0.2 release is [X] Stable, go ahead and release [ ] Broken because of ... +1 (Sorry for ghosting after suggesting that we all move quickly!) -chris - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
BZ 66294 - optionally disable some SecurityManager checks
Hi, BZ 66294 [1] highlights the performance impact in Tomcat of some additional SecurityManager checks that were added to avoid AccessControlException when using the EL API JAR outside of Tomcat. Details of the performance impact are in the bug report. I think we have a few options here. 1. Assume Tomcat 11 will remove the SecurityManager. No nothing for now and advise the reporter to move to Tomcat 11 when available. 2. Do nothing. 3. Disable this check by default and an option (it will have to be a system property) to enable it. 4. Something else. Thoughts? I am currently leaning towards 3 given that the performance impact is noticeable and that the check isn't required in normal usage. Mark [1] https://bz.apache.org/bugzilla/show_bug.cgi?id=66294 - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 66331] Incorrect exception is being caught in SystemLogHandler
https://bz.apache.org/bugzilla/show_bug.cgi?id=66331 --- Comment #4 from John Carr --- In the end... just time, effort, and priorities. However, we didn't realize 10.0 was end of life. Now that we see that, we have added upgrading to 10.1.x into our backlog. Thanks again for getting the original issue fixed. -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 8.5.x updated: NIO writes don't return -1 so neither should CLOSED_NIO_CHANNEL
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new fef2d26ccf NIO writes don't return -1 so neither should
CLOSED_NIO_CHANNEL
fef2d26ccf is described below
commit fef2d26ccf35b6935c9adcedcfe3093a4f6c2e34
Author: Mark Thomas
AuthorDate: Mon Nov 7 13:32:20 2022 +
NIO writes don't return -1 so neither should CLOSED_NIO_CHANNEL
---
java/org/apache/catalina/tribes/transport/nio/NioSender.java | 4
java/org/apache/tomcat/util/net/NioChannel.java | 5 +++--
java/org/apache/tomcat/util/net/NioEndpoint.java | 7 +--
webapps/docs/changelog.xml | 5 +
4 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/java/org/apache/catalina/tribes/transport/nio/NioSender.java
b/java/org/apache/catalina/tribes/transport/nio/NioSender.java
index 252dc4d183..82e7149222 100644
--- a/java/org/apache/catalina/tribes/transport/nio/NioSender.java
+++ b/java/org/apache/catalina/tribes/transport/nio/NioSender.java
@@ -16,7 +16,6 @@
*/
package org.apache.catalina.tribes.transport.nio;
-import java.io.EOFException;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.nio.ByteBuffer;
@@ -206,9 +205,6 @@ public class NioSender extends AbstractSender {
//we have written everything, or we are starting a new package
//protect against buffer overwrite
int byteswritten = isUdpBased()?dataChannel.write(writebuf) :
socketChannel.write(writebuf);
-if (byteswritten == -1 ) {
-throw new EOFException();
-}
remaining -= byteswritten;
//if the entire message was written from the buffer
//reset the position counter
diff --git a/java/org/apache/tomcat/util/net/NioChannel.java
b/java/org/apache/tomcat/util/net/NioChannel.java
index ac46d76753..8e3cb4f0e1 100644
--- a/java/org/apache/tomcat/util/net/NioChannel.java
+++ b/java/org/apache/tomcat/util/net/NioChannel.java
@@ -19,6 +19,7 @@ package org.apache.tomcat.util.net;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.ByteChannel;
+import java.nio.channels.ClosedChannelException;
import java.nio.channels.GatheringByteChannel;
import java.nio.channels.ScatteringByteChannel;
import java.nio.channels.Selector;
@@ -278,12 +279,12 @@ public class NioChannel implements ByteChannel,
ScatteringByteChannel, Gathering
@Override
public int write(ByteBuffer src) throws IOException {
checkInterruptStatus();
-return -1;
+throw new ClosedChannelException();
}
@Override
public long write(ByteBuffer[] srcs, int offset, int length)
throws IOException {
-return -1L;
+throw new ClosedChannelException();
}
@Override
public String toString() {
diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java
b/java/org/apache/tomcat/util/net/NioEndpoint.java
index 2c4cde8bc9..74e75d744b 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -1322,9 +1322,7 @@ public class NioEndpoint extends
AbstractJsseEndpoint
}
}
n = getSocket().write(buffer);
-if (n == -1) {
-throw new EOFException();
-} else if (n == 0 && (buffer.hasRemaining() ||
getSocket().getOutboundRemaining() > 0)) {
+if (n == 0 && (buffer.hasRemaining() ||
getSocket().getOutboundRemaining() > 0)) {
// n == 0 could be an incomplete write but it could
also
// indicate that a previous incomplete write of the
// outbound buffer (for TLS) has now completed. Only
@@ -1355,9 +1353,6 @@ public class NioEndpoint extends
AbstractJsseEndpoint
} else {
do {
n = getSocket().write(buffer);
-if (n == -1) {
-throw new EOFException();
-}
} while (n > 0 && buffer.hasRemaining());
// If there is data left in the buffer the socket will be
registered for
// write further up the stack. This is to ensure the socket is
only
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index aea7fc4a18..f501abd8c4 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -137,6 +137,11 @@
stream is cancelled due to an attempt to write to the stream when it is
in a state that does not permit writes. (markt)
+
+NIO writes n
[tomcat] branch 9.0.x updated: NIO writes don't return -1 so neither should CLOSED_NIO_CHANNEL
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new 4917824b2e NIO writes don't return -1 so neither should
CLOSED_NIO_CHANNEL
4917824b2e is described below
commit 4917824b2e612031db395d918b8d19950cb0bd33
Author: Mark Thomas
AuthorDate: Mon Nov 7 13:32:20 2022 +
NIO writes don't return -1 so neither should CLOSED_NIO_CHANNEL
---
java/org/apache/catalina/tribes/transport/nio/NioSender.java | 4
java/org/apache/tomcat/util/net/NioChannel.java | 5 +++--
java/org/apache/tomcat/util/net/NioEndpoint.java | 7 +--
webapps/docs/changelog.xml | 5 +
4 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/java/org/apache/catalina/tribes/transport/nio/NioSender.java
b/java/org/apache/catalina/tribes/transport/nio/NioSender.java
index 252dc4d183..82e7149222 100644
--- a/java/org/apache/catalina/tribes/transport/nio/NioSender.java
+++ b/java/org/apache/catalina/tribes/transport/nio/NioSender.java
@@ -16,7 +16,6 @@
*/
package org.apache.catalina.tribes.transport.nio;
-import java.io.EOFException;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.nio.ByteBuffer;
@@ -206,9 +205,6 @@ public class NioSender extends AbstractSender {
//we have written everything, or we are starting a new package
//protect against buffer overwrite
int byteswritten = isUdpBased()?dataChannel.write(writebuf) :
socketChannel.write(writebuf);
-if (byteswritten == -1 ) {
-throw new EOFException();
-}
remaining -= byteswritten;
//if the entire message was written from the buffer
//reset the position counter
diff --git a/java/org/apache/tomcat/util/net/NioChannel.java
b/java/org/apache/tomcat/util/net/NioChannel.java
index ac46d76753..8e3cb4f0e1 100644
--- a/java/org/apache/tomcat/util/net/NioChannel.java
+++ b/java/org/apache/tomcat/util/net/NioChannel.java
@@ -19,6 +19,7 @@ package org.apache.tomcat.util.net;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.ByteChannel;
+import java.nio.channels.ClosedChannelException;
import java.nio.channels.GatheringByteChannel;
import java.nio.channels.ScatteringByteChannel;
import java.nio.channels.Selector;
@@ -278,12 +279,12 @@ public class NioChannel implements ByteChannel,
ScatteringByteChannel, Gathering
@Override
public int write(ByteBuffer src) throws IOException {
checkInterruptStatus();
-return -1;
+throw new ClosedChannelException();
}
@Override
public long write(ByteBuffer[] srcs, int offset, int length)
throws IOException {
-return -1L;
+throw new ClosedChannelException();
}
@Override
public String toString() {
diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java
b/java/org/apache/tomcat/util/net/NioEndpoint.java
index 0889e334fa..e30f4a7c2d 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -1422,9 +1422,7 @@ public class NioEndpoint extends
AbstractJsseEndpoint
}
}
n = getSocket().write(buffer);
-if (n == -1) {
-throw new EOFException();
-} else if (n == 0 && (buffer.hasRemaining() ||
getSocket().getOutboundRemaining() > 0)) {
+if (n == 0 && (buffer.hasRemaining() ||
getSocket().getOutboundRemaining() > 0)) {
// n == 0 could be an incomplete write but it could
also
// indicate that a previous incomplete write of the
// outbound buffer (for TLS) has now completed. Only
@@ -1455,9 +1453,6 @@ public class NioEndpoint extends
AbstractJsseEndpoint
} else {
do {
n = getSocket().write(buffer);
-if (n == -1) {
-throw new EOFException();
-}
} while (n > 0 && buffer.hasRemaining());
// If there is data left in the buffer the socket will be
registered for
// write further up the stack. This is to ensure the socket is
only
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index d6991ccbee..346f510c47 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -146,6 +146,11 @@
stream is cancelled due to an attempt to write to the stream when it is
in a state that does not permit writes. (markt)
+
+NIO writes n
[tomcat] branch 10.1.x updated: NIO writes don't return -1 so neither should CLOSED_NIO_CHANNEL
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new e25d13624e NIO writes don't return -1 so neither should
CLOSED_NIO_CHANNEL
e25d13624e is described below
commit e25d13624e4216685a5043f83300d3286dfc6e82
Author: Mark Thomas
AuthorDate: Mon Nov 7 13:32:20 2022 +
NIO writes don't return -1 so neither should CLOSED_NIO_CHANNEL
---
java/org/apache/catalina/tribes/transport/nio/NioSender.java | 4
java/org/apache/tomcat/util/net/NioChannel.java | 5 +++--
java/org/apache/tomcat/util/net/NioEndpoint.java | 7 +--
webapps/docs/changelog.xml | 5 +
4 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/java/org/apache/catalina/tribes/transport/nio/NioSender.java
b/java/org/apache/catalina/tribes/transport/nio/NioSender.java
index 252dc4d183..82e7149222 100644
--- a/java/org/apache/catalina/tribes/transport/nio/NioSender.java
+++ b/java/org/apache/catalina/tribes/transport/nio/NioSender.java
@@ -16,7 +16,6 @@
*/
package org.apache.catalina.tribes.transport.nio;
-import java.io.EOFException;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.nio.ByteBuffer;
@@ -206,9 +205,6 @@ public class NioSender extends AbstractSender {
//we have written everything, or we are starting a new package
//protect against buffer overwrite
int byteswritten = isUdpBased()?dataChannel.write(writebuf) :
socketChannel.write(writebuf);
-if (byteswritten == -1 ) {
-throw new EOFException();
-}
remaining -= byteswritten;
//if the entire message was written from the buffer
//reset the position counter
diff --git a/java/org/apache/tomcat/util/net/NioChannel.java
b/java/org/apache/tomcat/util/net/NioChannel.java
index 9a22906914..d263ce9ae6 100644
--- a/java/org/apache/tomcat/util/net/NioChannel.java
+++ b/java/org/apache/tomcat/util/net/NioChannel.java
@@ -19,6 +19,7 @@ package org.apache.tomcat.util.net;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.ByteChannel;
+import java.nio.channels.ClosedChannelException;
import java.nio.channels.GatheringByteChannel;
import java.nio.channels.ScatteringByteChannel;
import java.nio.channels.SocketChannel;
@@ -260,12 +261,12 @@ public class NioChannel implements ByteChannel,
ScatteringByteChannel, Gathering
@Override
public int write(ByteBuffer src) throws IOException {
checkInterruptStatus();
-return -1;
+throw new ClosedChannelException();
}
@Override
public long write(ByteBuffer[] srcs, int offset, int length)
throws IOException {
-return -1L;
+throw new ClosedChannelException();
}
@Override
public String toString() {
diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java
b/java/org/apache/tomcat/util/net/NioEndpoint.java
index a228715123..d1864aec5a 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -1372,9 +1372,7 @@ public class NioEndpoint extends
AbstractJsseEndpoint
}
}
n = getSocket().write(buffer);
-if (n == -1) {
-throw new EOFException();
-} else if (n == 0 && (buffer.hasRemaining() ||
getSocket().getOutboundRemaining() > 0)) {
+if (n == 0 && (buffer.hasRemaining() ||
getSocket().getOutboundRemaining() > 0)) {
// n == 0 could be an incomplete write but it could
also
// indicate that a previous incomplete write of the
// outbound buffer (for TLS) has now completed. Only
@@ -1405,9 +1403,6 @@ public class NioEndpoint extends
AbstractJsseEndpoint
} else {
do {
n = getSocket().write(buffer);
-if (n == -1) {
-throw new EOFException();
-}
} while (n > 0 && buffer.hasRemaining());
// If there is data left in the buffer the socket will be
registered for
// write further up the stack. This is to ensure the socket is
only
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 915f2c17c9..7ce54780b6 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -146,6 +146,11 @@
stream is cancelled due to an attempt to write to the stream when it is
in a state that does not permit writes. (markt)
+
+NIO w
[GitHub] [tomcat] markt-asf commented on pull request #562: Remove unnecessary -1 predicate because write will not return -1 unless NioChannel is CLOSED_NIO_CHANNEL
markt-asf commented on PR #562: URL: https://github.com/apache/tomcat/pull/562#issuecomment-1305623555 Thanks for the PR. I applied to it manually so I could use a slightly different fix and add a change log entry. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] markt-asf closed pull request #562: Remove unnecessary -1 predicate because write will not return -1 unless NioChannel is CLOSED_NIO_CHANNEL
markt-asf closed pull request #562: Remove unnecessary -1 predicate because write will not return -1 unless NioChannel is CLOSED_NIO_CHANNEL URL: https://github.com/apache/tomcat/pull/562 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch main updated: NIO writes don't return -1 so neither should CLOSED_NIO_CHANNEL
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new cc7c12993b NIO writes don't return -1 so neither should
CLOSED_NIO_CHANNEL
cc7c12993b is described below
commit cc7c12993bb43bafbdcc209d145e65e32025e3ab
Author: Mark Thomas
AuthorDate: Mon Nov 7 13:32:20 2022 +
NIO writes don't return -1 so neither should CLOSED_NIO_CHANNEL
---
java/org/apache/catalina/tribes/transport/nio/NioSender.java | 4
java/org/apache/tomcat/util/net/NioChannel.java | 5 +++--
java/org/apache/tomcat/util/net/NioEndpoint.java | 7 +--
webapps/docs/changelog.xml | 5 +
4 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/java/org/apache/catalina/tribes/transport/nio/NioSender.java
b/java/org/apache/catalina/tribes/transport/nio/NioSender.java
index 252dc4d183..82e7149222 100644
--- a/java/org/apache/catalina/tribes/transport/nio/NioSender.java
+++ b/java/org/apache/catalina/tribes/transport/nio/NioSender.java
@@ -16,7 +16,6 @@
*/
package org.apache.catalina.tribes.transport.nio;
-import java.io.EOFException;
import java.io.IOException;
import java.net.InetSocketAddress;
import java.nio.ByteBuffer;
@@ -206,9 +205,6 @@ public class NioSender extends AbstractSender {
//we have written everything, or we are starting a new package
//protect against buffer overwrite
int byteswritten = isUdpBased()?dataChannel.write(writebuf) :
socketChannel.write(writebuf);
-if (byteswritten == -1 ) {
-throw new EOFException();
-}
remaining -= byteswritten;
//if the entire message was written from the buffer
//reset the position counter
diff --git a/java/org/apache/tomcat/util/net/NioChannel.java
b/java/org/apache/tomcat/util/net/NioChannel.java
index 9a22906914..d263ce9ae6 100644
--- a/java/org/apache/tomcat/util/net/NioChannel.java
+++ b/java/org/apache/tomcat/util/net/NioChannel.java
@@ -19,6 +19,7 @@ package org.apache.tomcat.util.net;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.ByteChannel;
+import java.nio.channels.ClosedChannelException;
import java.nio.channels.GatheringByteChannel;
import java.nio.channels.ScatteringByteChannel;
import java.nio.channels.SocketChannel;
@@ -260,12 +261,12 @@ public class NioChannel implements ByteChannel,
ScatteringByteChannel, Gathering
@Override
public int write(ByteBuffer src) throws IOException {
checkInterruptStatus();
-return -1;
+throw new ClosedChannelException();
}
@Override
public long write(ByteBuffer[] srcs, int offset, int length)
throws IOException {
-return -1L;
+throw new ClosedChannelException();
}
@Override
public String toString() {
diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java
b/java/org/apache/tomcat/util/net/NioEndpoint.java
index bdc534263e..7bad146bcf 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -1372,9 +1372,7 @@ public class NioEndpoint extends
AbstractNetworkChannelEndpoint 0)) {
+if (n == 0 && (buffer.hasRemaining() ||
getSocket().getOutboundRemaining() > 0)) {
// n == 0 could be an incomplete write but it could
also
// indicate that a previous incomplete write of the
// outbound buffer (for TLS) has now completed. Only
@@ -1405,9 +1403,6 @@ public class NioEndpoint extends
AbstractNetworkChannelEndpoint 0 && buffer.hasRemaining());
// If there is data left in the buffer the socket will be
registered for
// write further up the stack. This is to ensure the socket is
only
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 2c6bde9208..da5668d3f5 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -154,6 +154,11 @@
stream is cancelled due to an attempt to write to the stream when it is
in a state that does not permit writes. (markt)
+
+NIO writes never return -1 so refactor CLOSED_NIO_CHANNEL
+not to do so and remove checks for this return value. Based on
+562 by tianshuang. (markt)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 8.5.x updated: Improve error message when cancelling stream due to an invalid write
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new a26f1734e3 Improve error message when cancelling stream due to an
invalid write
a26f1734e3 is described below
commit a26f1734e364d85abc61dd56a49455ffc89fc402
Author: Mark Thomas
AuthorDate: Mon Nov 7 13:00:36 2022 +
Improve error message when cancelling stream due to an invalid write
---
java/org/apache/coyote/http2/Http2UpgradeHandler.java | 3 ++-
java/org/apache/coyote/http2/LocalStrings.properties | 2 +-
java/org/apache/coyote/http2/StreamStateMachine.java | 3 +++
webapps/docs/changelog.xml| 5 +
4 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index e9d982b95c..ce4bbc8ec8 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -912,7 +912,8 @@ class Http2UpgradeHandler extends AbstractStream implements
InternalHttpUpgradeH
synchronized (this) {
if (!stream.canWrite()) {
stream.doStreamCancel(sm.getString("upgradeHandler.stream.notWritable",
-stream.getConnectionId(), stream.getIdAsString()),
Http2Error.STREAM_CLOSED);
+stream.getConnectionId(), stream.getIdAsString(),
stream.state.getCurrentStateName() ),
+Http2Error.STREAM_CLOSED);
}
long windowSize = getWindowSize();
if (stream.getConnectionAllocationMade() > 0) {
diff --git a/java/org/apache/coyote/http2/LocalStrings.properties
b/java/org/apache/coyote/http2/LocalStrings.properties
index 05f2b125bf..563d624a34 100644
--- a/java/org/apache/coyote/http2/LocalStrings.properties
+++ b/java/org/apache/coyote/http2/LocalStrings.properties
@@ -153,7 +153,7 @@ upgradeHandler.startRequestBodyFrame.result=Connection
[{0}], Stream [{1}] start
upgradeHandler.stream.closed=Stream [{0}] has been closed for some time
upgradeHandler.stream.error=Connection [{0}], Stream [{1}] Closed due to error
upgradeHandler.stream.even=A new remote stream ID of [{0}] was requested but
all remote streams must use odd identifiers
-upgradeHandler.stream.notWritable=Connection [{0}], Stream [{1}], This stream
is not writable
+upgradeHandler.stream.notWritable=Connection [{0}], Stream [{1}], This stream
is in state [{2}] and is not writable
upgradeHandler.stream.old=A new remote stream ID of [{0}] was requested but
the most recent stream was [{1}]
upgradeHandler.tooManyRemoteStreams=The client attempted to use more than
[{0}] active streams
upgradeHandler.tooMuchOverhead=Connection [{0}], Too much overhead so the
connection will be closed
diff --git a/java/org/apache/coyote/http2/StreamStateMachine.java
b/java/org/apache/coyote/http2/StreamStateMachine.java
index 72028fd4a4..6ec79ff81c 100644
--- a/java/org/apache/coyote/http2/StreamStateMachine.java
+++ b/java/org/apache/coyote/http2/StreamStateMachine.java
@@ -164,6 +164,9 @@ class StreamStateMachine {
stateChange(State.IDLE, State.CLOSED_FINAL);
}
+final synchronized String getCurrentStateName() {
+return state.name();
+}
private enum State {
IDLE (false, false, false, true,
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 3d49ec6b9b..aea7fc4a18 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -132,6 +132,11 @@
A single space rather than a single dash should be used to separate the
day, month and year components to be compliant with RFC 6265. (markt)
+
+Include the name of the current stream state in the error message when
a
+stream is cancelled due to an attempt to write to the stream when it is
+in a state that does not permit writes. (markt)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 9.0.x updated: Improve error message when cancelling stream due to an invalid write
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/9.0.x by this push:
new ae0b58a426 Improve error message when cancelling stream due to an
invalid write
ae0b58a426 is described below
commit ae0b58a426ec2d42eee3e0593e83d38eb2e1b573
Author: Mark Thomas
AuthorDate: Mon Nov 7 13:00:36 2022 +
Improve error message when cancelling stream due to an invalid write
---
java/org/apache/coyote/http2/Http2UpgradeHandler.java | 3 ++-
java/org/apache/coyote/http2/LocalStrings.properties | 2 +-
java/org/apache/coyote/http2/StreamStateMachine.java | 3 +++
webapps/docs/changelog.xml| 5 +
4 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index d45a67d8a9..a3c0c54b15 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -916,7 +916,8 @@ class Http2UpgradeHandler extends AbstractStream implements
InternalHttpUpgradeH
synchronized (this) {
if (!stream.canWrite()) {
stream.doStreamCancel(sm.getString("upgradeHandler.stream.notWritable",
-stream.getConnectionId(), stream.getIdAsString()),
Http2Error.STREAM_CLOSED);
+stream.getConnectionId(), stream.getIdAsString(),
stream.state.getCurrentStateName() ),
+Http2Error.STREAM_CLOSED);
}
long windowSize = getWindowSize();
if (stream.getConnectionAllocationMade() > 0) {
diff --git a/java/org/apache/coyote/http2/LocalStrings.properties
b/java/org/apache/coyote/http2/LocalStrings.properties
index 615e6413dc..8972c4fb6e 100644
--- a/java/org/apache/coyote/http2/LocalStrings.properties
+++ b/java/org/apache/coyote/http2/LocalStrings.properties
@@ -155,7 +155,7 @@ upgradeHandler.startRequestBodyFrame.result=Connection
[{0}], Stream [{1}] start
upgradeHandler.stream.closed=Stream [{0}] has been closed for some time
upgradeHandler.stream.error=Connection [{0}], Stream [{1}] Closed due to error
upgradeHandler.stream.even=A new remote stream ID of [{0}] was requested but
all remote streams must use odd identifiers
-upgradeHandler.stream.notWritable=Connection [{0}], Stream [{1}], This stream
is not writable
+upgradeHandler.stream.notWritable=Connection [{0}], Stream [{1}], This stream
is in state [{2}] and is not writable
upgradeHandler.stream.old=A new remote stream ID of [{0}] was requested but
the most recent stream was [{1}]
upgradeHandler.tooManyRemoteStreams=The client attempted to use more than
[{0}] active streams
upgradeHandler.tooMuchOverhead=Connection [{0}], Too much overhead so the
connection will be closed
diff --git a/java/org/apache/coyote/http2/StreamStateMachine.java
b/java/org/apache/coyote/http2/StreamStateMachine.java
index 72028fd4a4..6ec79ff81c 100644
--- a/java/org/apache/coyote/http2/StreamStateMachine.java
+++ b/java/org/apache/coyote/http2/StreamStateMachine.java
@@ -164,6 +164,9 @@ class StreamStateMachine {
stateChange(State.IDLE, State.CLOSED_FINAL);
}
+final synchronized String getCurrentStateName() {
+return state.name();
+}
private enum State {
IDLE (false, false, false, true,
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index c9a6fda495..d6991ccbee 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -141,6 +141,11 @@
A single space rather than a single dash should be used to separate the
day, month and year components to be compliant with RFC 6265. (markt)
+
+Include the name of the current stream state in the error message when
a
+stream is cancelled due to an attempt to write to the stream when it is
+in a state that does not permit writes. (markt)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch 10.1.x updated: Improve error message when cancelling stream due to an invalid write
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new ec7c412141 Improve error message when cancelling stream due to an
invalid write
ec7c412141 is described below
commit ec7c412141d0e174558a54c67f771cad507335e4
Author: Mark Thomas
AuthorDate: Mon Nov 7 13:00:36 2022 +
Improve error message when cancelling stream due to an invalid write
---
java/org/apache/coyote/http2/Http2UpgradeHandler.java | 3 ++-
java/org/apache/coyote/http2/LocalStrings.properties | 2 +-
java/org/apache/coyote/http2/StreamStateMachine.java | 3 +++
webapps/docs/changelog.xml| 5 +
4 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 8ff3bd59d0..8ead0b378d 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -937,7 +937,8 @@ class Http2UpgradeHandler extends AbstractStream implements
InternalHttpUpgradeH
synchronized (this) {
if (!stream.canWrite()) {
stream.doStreamCancel(sm.getString("upgradeHandler.stream.notWritable",
-stream.getConnectionId(), stream.getIdAsString()),
Http2Error.STREAM_CLOSED);
+stream.getConnectionId(), stream.getIdAsString(),
stream.state.getCurrentStateName() ),
+Http2Error.STREAM_CLOSED);
}
long windowSize = getWindowSize();
if (stream.getConnectionAllocationMade() > 0) {
diff --git a/java/org/apache/coyote/http2/LocalStrings.properties
b/java/org/apache/coyote/http2/LocalStrings.properties
index 615e6413dc..8972c4fb6e 100644
--- a/java/org/apache/coyote/http2/LocalStrings.properties
+++ b/java/org/apache/coyote/http2/LocalStrings.properties
@@ -155,7 +155,7 @@ upgradeHandler.startRequestBodyFrame.result=Connection
[{0}], Stream [{1}] start
upgradeHandler.stream.closed=Stream [{0}] has been closed for some time
upgradeHandler.stream.error=Connection [{0}], Stream [{1}] Closed due to error
upgradeHandler.stream.even=A new remote stream ID of [{0}] was requested but
all remote streams must use odd identifiers
-upgradeHandler.stream.notWritable=Connection [{0}], Stream [{1}], This stream
is not writable
+upgradeHandler.stream.notWritable=Connection [{0}], Stream [{1}], This stream
is in state [{2}] and is not writable
upgradeHandler.stream.old=A new remote stream ID of [{0}] was requested but
the most recent stream was [{1}]
upgradeHandler.tooManyRemoteStreams=The client attempted to use more than
[{0}] active streams
upgradeHandler.tooMuchOverhead=Connection [{0}], Too much overhead so the
connection will be closed
diff --git a/java/org/apache/coyote/http2/StreamStateMachine.java
b/java/org/apache/coyote/http2/StreamStateMachine.java
index 72028fd4a4..6ec79ff81c 100644
--- a/java/org/apache/coyote/http2/StreamStateMachine.java
+++ b/java/org/apache/coyote/http2/StreamStateMachine.java
@@ -164,6 +164,9 @@ class StreamStateMachine {
stateChange(State.IDLE, State.CLOSED_FINAL);
}
+final synchronized String getCurrentStateName() {
+return state.name();
+}
private enum State {
IDLE (false, false, false, true,
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 9ecaaff4d3..915f2c17c9 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -141,6 +141,11 @@
A single space rather than a single dash should be used to separate the
day, month and year components to be compliant with RFC 6265. (markt)
+
+Include the name of the current stream state in the error message when
a
+stream is cancelled due to an attempt to write to the stream when it is
+in a state that does not permit writes. (markt)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[tomcat] branch main updated: Improve error message when cancelling stream due to an invalid write
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 8172a94123 Improve error message when cancelling stream due to an
invalid write
8172a94123 is described below
commit 8172a941232a14aaad9ec73e2ca14eb03ca82453
Author: Mark Thomas
AuthorDate: Mon Nov 7 13:00:36 2022 +
Improve error message when cancelling stream due to an invalid write
---
java/org/apache/coyote/http2/Http2UpgradeHandler.java | 3 ++-
java/org/apache/coyote/http2/LocalStrings.properties | 2 +-
java/org/apache/coyote/http2/StreamStateMachine.java | 3 +++
webapps/docs/changelog.xml| 5 +
4 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 8ff3bd59d0..8ead0b378d 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -937,7 +937,8 @@ class Http2UpgradeHandler extends AbstractStream implements
InternalHttpUpgradeH
synchronized (this) {
if (!stream.canWrite()) {
stream.doStreamCancel(sm.getString("upgradeHandler.stream.notWritable",
-stream.getConnectionId(), stream.getIdAsString()),
Http2Error.STREAM_CLOSED);
+stream.getConnectionId(), stream.getIdAsString(),
stream.state.getCurrentStateName() ),
+Http2Error.STREAM_CLOSED);
}
long windowSize = getWindowSize();
if (stream.getConnectionAllocationMade() > 0) {
diff --git a/java/org/apache/coyote/http2/LocalStrings.properties
b/java/org/apache/coyote/http2/LocalStrings.properties
index 615e6413dc..8972c4fb6e 100644
--- a/java/org/apache/coyote/http2/LocalStrings.properties
+++ b/java/org/apache/coyote/http2/LocalStrings.properties
@@ -155,7 +155,7 @@ upgradeHandler.startRequestBodyFrame.result=Connection
[{0}], Stream [{1}] start
upgradeHandler.stream.closed=Stream [{0}] has been closed for some time
upgradeHandler.stream.error=Connection [{0}], Stream [{1}] Closed due to error
upgradeHandler.stream.even=A new remote stream ID of [{0}] was requested but
all remote streams must use odd identifiers
-upgradeHandler.stream.notWritable=Connection [{0}], Stream [{1}], This stream
is not writable
+upgradeHandler.stream.notWritable=Connection [{0}], Stream [{1}], This stream
is in state [{2}] and is not writable
upgradeHandler.stream.old=A new remote stream ID of [{0}] was requested but
the most recent stream was [{1}]
upgradeHandler.tooManyRemoteStreams=The client attempted to use more than
[{0}] active streams
upgradeHandler.tooMuchOverhead=Connection [{0}], Too much overhead so the
connection will be closed
diff --git a/java/org/apache/coyote/http2/StreamStateMachine.java
b/java/org/apache/coyote/http2/StreamStateMachine.java
index 72028fd4a4..6ec79ff81c 100644
--- a/java/org/apache/coyote/http2/StreamStateMachine.java
+++ b/java/org/apache/coyote/http2/StreamStateMachine.java
@@ -164,6 +164,9 @@ class StreamStateMachine {
stateChange(State.IDLE, State.CLOSED_FINAL);
}
+final synchronized String getCurrentStateName() {
+return state.name();
+}
private enum State {
IDLE (false, false, false, true,
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 870cbc8c91..2c6bde9208 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -149,6 +149,11 @@
A single space rather than a single dash should be used to separate the
day, month and year components to be compliant with RFC 6265. (markt)
+
+Include the name of the current stream state in the error message when
a
+stream is cancelled due to an attempt to write to the stream when it is
+in a state that does not permit writes. (markt)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
JDK 20 EAb22, ZenGC EA builds, JavaFX 20 EAb5 and several heads-ups!
Greetings, With JavaOne in Las Vegas, last month was epically busy! It was great to finally have the ability to meet and discuss the Quality Outreach program with some of you... face-to-face! This installment of the newsletter is packed as we have several heads-ups, including new Early-Access builds being made available. The JDK 20 schedule has been proposed [1]. The next major milestone is Rampdown Phase One which should happen in just a month on December 8! The next few weeks will be particularly interesting as we will see which from the candidate JEPs recently announced (see 'Topics of Interest' section below) will be proposed to target JDK 20 [2]. And given that JDK 20 is getting closer, we are eagerly waiting for your test feedback on your projects running with the latest JDK 20 EA builds. [1] https://mail.openjdk.org/pipermail/jdk-dev/2022-October/007108.html [2] https://openjdk.org/projects/jdk/20/ ### Heads-up - JDK 20: `java.net.URL` parsing fix & behavior change Before JDK 20, some of the parsing/validation performed by the JDK built-in `URLStreamHander` implementations were delayed until `URL::openConnection` or `URLConnection::connect` was called. Starting JDK 20, some of these parsing/validations are now performed early, i.e. within URL constructors. An exception caused by a malformed URL that would have been delayed until the connection was opened or connected may starting JDK 20, throw a `MalformedURLException` at URL construction time. We suggest testing your project(s) against this change. And for those who want to rely on the old behavior, a new system property has been introduced to revert, on the command line, to the previous behavior. For more details, please see JBS-8293590 [3] and the release notes [4]. [3] https://bugs.openjdk.org/browse/JDK-8293590 [4] https://bugs.openjdk.org/browse/JDK-8295750 ### Heads-up - JDK 20: Thread.stop(), Thread.suspend() and Thread.resume() degradation The ability to stop, suspend, or resume a thread with the corresponding Thread.stop(), Thread.suspend() or Thread.resume() methods have been removed in JDK 20. Those methods have been degraded to throw a UOE exception (UnsupportedOperationException). Using those methods was inherently unsafe. That is also why they were deprecated since JDK 1.2 (1998!) and were flagged 'forRemoval' in previous features release. We do not expect this behavior change to cause issues on well-maintained codebase. For more details please check JDK-8289610 [5], JDK-8249627 [6], and the Java Thread Primitive Deprecation FAQ [7]. [5] https://bugs.openjdk.org/browse/JDK-8289610 [6] https://bugs.openjdk.org/browse/JDK-8249627 [7] https://docs.oracle.com/en/java/javase/19/docs/api/java.base/java/lang/doc-files/threadPrimitiveDeprecation.html ### Heads-up - JDK 20: Deprecate and disable the legacy parallel class loading workaround for non-parallel-capable class loaders. Prior to JDK 7, custom class loaders using non-hierarchical class delegation model were prone to deadlock. A workaround was added in the HotSpot VM (JDK 6) to allow parallel class loading for non-parallel-capable class loaders to avoid deadlocks. Parallel-capable class loaders were introduced in Java SE 7 [8] to support parallel class loading to implement a deadlock-free class loader using a non-hierarchical class delegation model. [8] and [9] describe how to migrate those class loaders depending on this workaround to be multi-threaded parallel-capable class loaders. This workaround was intended to allow those developers to migrate to the new mechanism. JDK 7 was released 11 years ago so it is now expected that those deadlock-prone custom class loaders have been migrated to the parallel-capable class loaders. As a consequence, this workaround is removed in JDK 20 as it impedes eliminating the object monitors from pinning for virtual threads. We suggest confirming that your codebase is not relying on this legacy workaround. If it still is, you should migrate away from it ASAP. Please note that the legacy behavior can be temporary re-enabled using a special flag. For additional details, please check [10] and [11]. [8] https://docs.oracle.com/javase/7/docs/technotes/guides/lang/cl-mt.html [9] https://openjdk.org/groups/core-libs/ClassLoaderProposal.html [10] https://bugs.openjdk.org/browse/JDK-8295848 [11] https://bugs.openjdk.org/browse/JDK-8296446 ### Heads-up - JavaFX builds Oracle is now publishing JavaFX builds, starting with early access builds of JavaFX 20, at jdk.java.net/javafx20 [12]. Developers are now able to download JavaFX and JDK builds from the same place, and use jlink to create a custom JDK that includes the JavaFX modules. The latest JavaFX 20 EA builds (b5-2022/10/28) are now available [12] along with the related javadoc [13]. These early-access builds are provided under the GNU General Public License, version 2, with the Classpath Exception. Feedback should
