You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2026/10/16 15:25:15 UTC
(tomcat) branch 11.0.x updated (8e1f20ff43 -> 93dea1dc4d)
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a change to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
from 8e1f20ff43 Fix BZ 70269 - StatementCache regression with drivers that report statements as not poolable (#1083)
new d90aebbbee Account for TLS 1.3 in the enabled protocol management of the JNI OpenSSL SSLEngine
new 661eb9923e Make the lazily parsed wrapped certificate field volatile in OpenSSLX509Certificate
new 62b4792a4c Match OpenSSL semantics for AND cipher expressions with unknown terms
new 4e813accaa Exclude unimplemented SSLv2 ciphers when converting OpenSSL cipher names to JSSE names
new bd550d39bf Correct the cipher authentication method computation of the OpenSSL FFM implementation
new 28177b3c97 Account for the array offset when copying between OpenSSL FFM engine buffers and heap ByteBuffers
new e5cb820aaa Validate the protocol of OCSP URLs from a client certificate AIA extension
new b973929ab1 Correct the FILE check for SSL_CONF commands in the OpenSSL FFM code
new fc8faea170 Check for NULL after SSL_CTX_new() and SSL_new() in the OpenSSL FFM code
new fcea01e031 Correct the OpenSSL 3 version boundary in the FFM compatibility layer
new 5aa34339b8 Bind the nonce value parameter of OCSP_request_add1_nonce as a pointer
new 2e3743f17b Harmonize resource bundle processing in ProxyErrorReportValve
new a2725acb46 Correct the selection of multi-release JAR entries in JarFileUrlJar
new 679b7107fd Keep streams returned from ReferenceCountedJar open
new 959efc91f2 Ignore extra close() calls in ReferenceCountedJar
new 79826e4a79 Resolve nested JAR entries in JarFileUrlNestedJar using the JarURLConnection
new 3eaebe9d49 Report the resource path when a WEB-INF/lib JAR cannot be scanned
new 5090bdfa2d Require at least one iteration when computing message digests
new 9cb44cd42f Avoid busy spin in TaskQueue.take() when the keep-alive time is zero
new 3d1d4ce3b2 Keep the acquisition count of LimitLatch stable across releaseAll()
new 017c6dedcf Fix premature termination of InlineExecutorService
new 3d92ff3440 Fix submitted task count drift in ThreadPoolExecutor
new 93dea1dc4d Shut down the internal virtual thread executor when the endpoint stops
The 23 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../realm/DigestCredentialHandlerBase.java | 6 +
.../catalina/valves/ProxyErrorReportValve.java | 23 +-
.../apache/tomcat/util/net/AbstractEndpoint.java | 6 +
.../tomcat/util/net/openssl/OpenSSLEngine.java | 15 +-
.../util/net/openssl/OpenSSLX509Certificate.java | 2 +-
.../ciphers/OpenSSLCipherConfigurationParser.java | 50 ++++-
.../net/openssl/panama/LocalStrings.properties | 2 +
.../util/net/openssl/panama/OpenSSLContext.java | 78 +++----
.../util/net/openssl/panama/OpenSSLEngine.java | 26 ++-
.../util/net/openssl/panama/OpenSSLLibrary.java | 17 +-
.../net/openssl/panama/OpenSSLX509Certificate.java | 2 +-
java/org/apache/tomcat/util/openssl/openssl_h.java | 4 +-
.../util/openssl/openssl_h_Compatibility.java | 4 +-
java/org/apache/tomcat/util/res/StringManager.java | 2 +-
.../org/apache/tomcat/util/scan/JarFileUrlJar.java | 81 ++++++-
.../tomcat/util/scan/JarFileUrlNestedJar.java | 12 +-
.../tomcat/util/scan/ReferenceCountedJar.java | 54 ++++-
.../tomcat/util/scan/StandardJarScanner.java | 2 +-
.../util/security/ConcurrentMessageDigest.java | 7 +
.../tomcat/util/security/LocalStrings.properties | 1 +
.../tomcat/util/threads/InlineExecutorService.java | 13 +-
.../org/apache/tomcat/util/threads/LimitLatch.java | 21 +-
java/org/apache/tomcat/util/threads/TaskQueue.java | 11 +-
.../tomcat/util/threads/ThreadPoolExecutor.java | 61 ++++-
.../realm/TestMessageDigestCredentialHandler.java | 23 ++
.../tomcat/util/net/TestAbstractEndpoint.java | 44 ++++
.../util/net/openssl/ciphers/TestCipher.java | 38 +++-
.../TestOpenSSLCipherConfigurationParser.java | 10 +
.../apache/tomcat/util/scan/TestJarFileUrlJar.java | 226 +++++++++++++++++++
.../tomcat/util/scan/TestJarFileUrlNestedJar.java | 164 ++++++++++++++
.../tomcat/util/scan/TestReferenceCountedJar.java | 213 ++++++++++++++++++
.../util/security/TestConcurrentMessageDigest.java | 70 +++++-
.../util/threads/TestInlineExecutorService.java | 147 ++++++++++++
.../apache/tomcat/util/threads/TestLimitLatch.java | 52 +++++
.../apache/tomcat/util/threads/TestTaskQueue.java | 92 ++++++++
.../util/threads/TestThreadPoolExecutor.java | 246 +++++++++++++++++++++
webapps/docs/changelog.xml | 47 ++++
37 files changed, 1750 insertions(+), 122 deletions(-)
create mode 100644 test/org/apache/tomcat/util/scan/TestJarFileUrlJar.java
create mode 100644 test/org/apache/tomcat/util/scan/TestJarFileUrlNestedJar.java
create mode 100644 test/org/apache/tomcat/util/scan/TestReferenceCountedJar.java
create mode 100644 test/org/apache/tomcat/util/threads/TestInlineExecutorService.java
create mode 100644 test/org/apache/tomcat/util/threads/TestTaskQueue.java
create mode 100644 test/org/apache/tomcat/util/threads/TestThreadPoolExecutor.java
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 02/23: Make the lazily parsed wrapped certificate field volatile in OpenSSLX509Certificate
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 661eb9923e44dd1412dec9d854d0a7f2f342c67a
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 07:59:20 2026 +0200
Make the lazily parsed wrapped certificate field volatile in OpenSSLX509Certificate
OpenSSLX509Certificate lazily parses its final DER bytes into a JDK
X509Certificate in the unsynchronized unwrap() method. Instances are
cached in the engine peer-certificate array which is handed out
repeatedly to potentially concurrent application threads through
SSLSession.getPeerCertificates() and getPeerPrincipal().
Without volatile there is no happens-before edge from the parse in one
thread to a read in another, so a thread may never observe the parsed
certificate and re-parse the DER bytes on every call, and in principle
could observe a partially initialized certificate object. Marking the
field volatile makes the lazy publication safe; the remaining benign
check-then-act race is an idempotent redundant parse, matching the
existing lock-free design. Applied to both the JNI and the Panama
implementations.
---
java/org/apache/tomcat/util/net/openssl/OpenSSLX509Certificate.java | 2 +-
.../apache/tomcat/util/net/openssl/panama/OpenSSLX509Certificate.java | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLX509Certificate.java b/java/org/apache/tomcat/util/net/openssl/OpenSSLX509Certificate.java
index 81d1aa3f88..c7887507ec 100644
--- a/java/org/apache/tomcat/util/net/openssl/OpenSSLX509Certificate.java
+++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLX509Certificate.java
@@ -35,7 +35,7 @@ import java.util.Set;
final class OpenSSLX509Certificate extends X509Certificate {
private final byte[] bytes;
- private X509Certificate wrapped;
+ private volatile X509Certificate wrapped;
OpenSSLX509Certificate(byte[] bytes) {
this.bytes = bytes;
diff --git a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLX509Certificate.java b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLX509Certificate.java
index 0011c1be54..3f9aa69d81 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLX509Certificate.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLX509Certificate.java
@@ -35,7 +35,7 @@ import java.util.Set;
final class OpenSSLX509Certificate extends X509Certificate {
private final byte[] bytes;
- private X509Certificate wrapped;
+ private volatile X509Certificate wrapped;
OpenSSLX509Certificate(byte[] bytes) {
this.bytes = bytes;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 09/23: Check for NULL after SSL_CTX_new() and SSL_new() in the OpenSSL FFM code
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit fc8faea170d623ab6d49b8f6d13b8b8e1a9a94f1
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 10:59:05 2026 +0200
Check for NULL after SSL_CTX_new() and SSL_new() in the OpenSSL FFM code
The SSL context and SSL object returned by SSL_CTX_new() and SSL_new()
are passed directly to subsequent OpenSSL calls. Those calls dereference
their arguments natively, so if an allocation failed, for example under
memory exhaustion, the JVM would crash with a SIGSEGV instead of
reporting an error.
Check the result of each call in the panama OpenSSLContext constructor,
the panama OpenSSLEngine constructor and OpenSSLLibrary.findCiphers()
and throw an exception carrying the OpenSSL error queue content when
NULL is returned. The findCiphers() failures are reported by the
existing handling for that method: a logged warning and an empty cipher
list. In addition, reject a NULL SSL context segment in the engine
constructor, matching the check the JNI implementation performs.
The JNI/tomcat-native implementation effectively performs the same
checks: its native allocation helpers throw a Java exception when the
underlying OpenSSL allocation returns NULL.
---
.../apache/tomcat/util/net/openssl/panama/LocalStrings.properties | 2 ++
java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java | 3 +++
java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java | 5 ++++-
java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java | 6 ++++++
4 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/tomcat/util/net/openssl/panama/LocalStrings.properties b/java/org/apache/tomcat/util/net/openssl/panama/LocalStrings.properties
index bd73d69ba2..76ea57c54a 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/net/openssl/panama/LocalStrings.properties
@@ -27,6 +27,7 @@ engine.invalidBufferArray=offset: [{0}], length: [{1}] (expected: offset <= offs
engine.invalidDestinationBuffersState=The state of the destination buffers changed concurrently while unwrapping bytes
engine.invalidOCSPURL=The OCSP URL is invalid: [{0}]
engine.noRestrictSessionCreation=OpenSslEngine does not permit restricting the engine to only resuming existing sessions
+engine.noSSL=Could not create the SSL object [{0}]
engine.noSSLContext=No SSL context
engine.noSSLState=No state was found in the SSL Engine for the given SSL instance [{0}]
engine.noSession=SSL session ID not available
@@ -54,6 +55,7 @@ openssl.doubleInit=SSL context already initialized, ignoring
openssl.errApplyConf=Could not apply OpenSSLConf to SSL context
openssl.errCheckConf=Error during OpenSSLConf check
openssl.errMakeConf=Could not create OpenSSLConf context [{0}]
+openssl.errMakeContext=Could not create SSL context [{0}]
openssl.errorAddingCertificate=Error adding certificate to chain: [{0}]
openssl.errorConfiguringLocations=Error configuring CA certificate locations: [{0}]
openssl.errorInit=Non fatal error initializing certificates or configuration: [{0}]
diff --git a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
index 88a99ce4ae..83ba29c94c 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
@@ -189,6 +189,9 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext {
// SSL protocol
sslCtx = SSL_CTX_new(TLS_server_method());
+ if (MemorySegment.NULL.equals(sslCtx)) {
+ throw new SSLException(sm.getString("openssl.errMakeContext", OpenSSLLibrary.getLastError()));
+ }
int protocol = SSL_PROTOCOL_NONE;
for (String enabledProtocol : sslHostConfig.getEnabledProtocols()) {
diff --git a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
index da255ab0b2..96ab226282 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
@@ -188,13 +188,16 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
OpenSSLSessionContext sessionContext, boolean alpn, boolean initialized, int certificateVerificationDepth,
boolean certificateVerificationOptionalNoCA, boolean noOcspCheck, boolean ocspSoftFail, int ocspTimeout,
int ocspVerifyFlags) {
- if (sslCtx == null) {
+ if (sslCtx == null || MemorySegment.NULL.equals(sslCtx)) {
throw new IllegalArgumentException(sm.getString("engine.noSSLContext"));
}
engineArena = Arena.ofAuto();
bufSegment = engineArena.allocate(MAX_ENCRYPTED_PACKET_LENGTH);
session = new OpenSSLSession();
var ssl = SSL_new(sslCtx);
+ if (MemorySegment.NULL.equals(ssl)) {
+ throw new IllegalStateException(sm.getString("engine.noSSL", OpenSSLLibrary.getLastError()));
+ }
// Set ssl_info_callback
SSL_set_info_callback(ssl, SSL_set_info_callback$cb.allocate(new InfoCallback(), engineArena));
if (clientMode) {
diff --git a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java
index cac10c2a89..c969385848 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java
@@ -445,10 +445,16 @@ public class OpenSSLLibrary {
try (var localArena = Arena.ofConfined()) {
initLibrary();
var sslCtx = SSL_CTX_new(TLS_server_method());
+ if (MemorySegment.NULL.equals(sslCtx)) {
+ throw new IllegalStateException(sm.getString("openssl.errMakeContext", getLastError()));
+ }
try {
openssl_h_Compatibility.SSL_CTX_set_options(sslCtx, SSL_OP_ALL());
SSL_CTX_set_cipher_list(sslCtx, localArena.allocateFrom(ciphers));
var ssl = SSL_new(sslCtx);
+ if (MemorySegment.NULL.equals(ssl)) {
+ throw new IllegalStateException(sm.getString("engine.noSSL", getLastError()));
+ }
SSL_set_accept_state(ssl);
try {
for (String c : getCiphers(ssl)) {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 20/23: Keep the acquisition count of LimitLatch stable across releaseAll()
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 3d1d4ce3b281e2e1a0af1027e41ca425ff581cc0
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 14:28:58 2026 +0200
Keep the acquisition count of LimitLatch stable across releaseAll()
releaseAll() wakes the queued threads by releasing the synchronizer.
The release path also decremented the acquisition counter by one, a
decrement that corresponded to no thread returning a share, leaving the
counter below the number of shares actually held. Callers that inspect
the counter after releaseAll(), or that continue to use the latch
rather than discarding it, saw fewer acquisitions counted than there
were, so the limit could be exceeded once enforced again.
releaseAll() now wakes the waiting threads without changing the
counter. Threads released in this way hold a share until they call
countDown(), like any other acquisition. The documentation of reset()
now describes the consequence of resetting while shares are still held
and that queued threads are not woken by a reset, and the documented
return value of countDown() now matches the value it has always
returned.
Add testReleaseAllKeepsCount which pins the counter through a
releaseAll() cycle with waiting threads.
---
.../org/apache/tomcat/util/threads/LimitLatch.java | 21 ++++++---
.../apache/tomcat/util/threads/TestLimitLatch.java | 52 ++++++++++++++++++++++
2 files changed, 66 insertions(+), 7 deletions(-)
diff --git a/java/org/apache/tomcat/util/threads/LimitLatch.java b/java/org/apache/tomcat/util/threads/LimitLatch.java
index dd08dbdb58..04c36ff41c 100644
--- a/java/org/apache/tomcat/util/threads/LimitLatch.java
+++ b/java/org/apache/tomcat/util/threads/LimitLatch.java
@@ -58,7 +58,10 @@ public class LimitLatch {
@Override
protected boolean tryReleaseShared(int arg) {
- count.decrementAndGet();
+ // An argument of zero is used for releaseAll() which must not change the acquisition count.
+ if (arg != 0) {
+ count.decrementAndGet();
+ }
return true;
}
}
@@ -133,10 +136,10 @@ public class LimitLatch {
/**
* Releases a shared latch, making it available for another thread to use.
*
- * @return the previous counter value
+ * @return the counter value after the decrement
*/
public long countDown() {
- sync.releaseShared(0);
+ sync.releaseShared(1);
long result = getCount();
if (log.isTraceEnabled()) {
log.trace("Counting down[" + Thread.currentThread().getName() + "] latch=" + result);
@@ -145,9 +148,9 @@ public class LimitLatch {
}
/**
- * Releases all waiting threads and causes the {@link #limit} to be ignored until {@link #reset()} is called. Note
- * that this method also decrements the acquisition counter by one as a side effect, so {@link #reset()} should be
- * called (or a new {@link LimitLatch} used) before the limit is enforced again.
+ * Releases all waiting threads and causes the {@link #limit} to be ignored until {@link #reset()} is called. This
+ * method does not change the acquisition counter. Threads that this method releases are counted as holding a share
+ * and each must still call {@link #countDown()} once it has finished, as with any other acquisition.
*
* @return <code>true</code> if release was done
*/
@@ -157,7 +160,11 @@ public class LimitLatch {
}
/**
- * Resets the latch and initializes the shared acquisition counter to zero.
+ * Resets the latch and initializes the shared acquisition counter to zero. Any shares currently held are not
+ * accounted for. When the holders of those shares later call {@link #countDown()}, the counter goes negative which
+ * effectively allows that many additional acquisitions before the limit is reached again. Reset the latch only when
+ * no shares are held (for example, after using a dedicated latch for a period during which the limit is ignored),
+ * or accept that the limit is not reliably enforced until the counter has returned to zero.
*
* @see #releaseAll()
*/
diff --git a/test/org/apache/tomcat/util/threads/TestLimitLatch.java b/test/org/apache/tomcat/util/threads/TestLimitLatch.java
index 4538f51aed..e7cba8c1fb 100644
--- a/test/org/apache/tomcat/util/threads/TestLimitLatch.java
+++ b/test/org/apache/tomcat/util/threads/TestLimitLatch.java
@@ -85,6 +85,58 @@ public class TestLimitLatch {
checkWaitingThreadCount(latch, 0);
}
+ @Test
+ public void testReleaseAllKeepsCount() throws Exception {
+ LimitLatch latch = new LimitLatch(2);
+ Object lock = new Object();
+
+ // Two threads hold a share of the latch
+ latch.countUpOrAwait();
+ latch.countUpOrAwait();
+
+ // Five threads wait for a share of the latch
+ TestThread[] testThreads = new TestThread[5];
+ for (int i = 0; i < testThreads.length; i++) {
+ testThreads[i] = new TestThread(latch, lock);
+ testThreads[i].start();
+ }
+ for (int i = 0; i < testThreads.length; i++) {
+ if (!waitForThreadToStart(testThreads[i])) {
+ Assert.fail("Test thread [" + i + "] did not start");
+ }
+ }
+ checkWaitingThreadCount(latch, 5);
+
+ // Releasing all waiting threads must not change the count. The released threads acquire a share which they
+ // return below.
+ latch.releaseAll();
+
+ if (!waitForThreadsToReachStage(testThreads, 0, 5, 0)) {
+ Assert.fail("Threads were not released");
+ }
+ // Two shares held by this thread plus one acquired by each of the released threads
+ Assert.assertEquals(7, latch.getCount());
+
+ synchronized (lock) {
+ lock.notifyAll();
+ }
+
+ if (!waitForThreadsToReachStage(testThreads, 0, 0, 5)) {
+ Assert.fail("Threads did not complete");
+ }
+ Assert.assertEquals(2, latch.getCount());
+
+ // Return the shares held by this thread
+ latch.countDown();
+ latch.countDown();
+ Assert.assertEquals(0, latch.getCount());
+
+ latch.reset();
+ latch.countUpOrAwait();
+ Assert.assertEquals(1, latch.getCount());
+ }
+
+
@Test
public void testTenWait() throws Exception {
LimitLatch latch = new LimitLatch(10);
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 06/23: Account for the array offset when copying between OpenSSL FFM engine buffers and heap ByteBuffers
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 28177b3c97794348a580af017f63e1653cc10a1a
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 10:12:00 2026 +0200
Account for the array offset when copying between OpenSSL FFM engine buffers and heap ByteBuffers
The write and read helper methods of the FFM based OpenSSL SSLEngine
used the buffer position directly as the index into the array returned
by ByteBuffer.array(). For a heap buffer that is a view of a larger
array, for example the result of slicing the buffer at a non-zero
position, the index into the backing array is the array offset plus
the position. The unadjusted position caused the engine to read from
and write to the wrong offset, silently corrupting the application
data of any wrap or unwrap call that used such a buffer.
The buffers Tomcat itself passes to the engine are all direct or have
a zero array offset, so this went unnoticed internally. It is a
regression against the JNI based implementation, which uses ByteBuffer
get and put methods that take the array offset into account
implicitly.
---
.../apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
index 8c9aa52cf7..84e13bc83f 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
@@ -251,7 +251,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
final int len = Math.min(src.remaining(), MAX_PLAINTEXT_LENGTH);
MemorySegment srcSegment = src.isDirect() ? MemorySegment.ofBuffer(src) : bufSegment;
if (!src.isDirect()) {
- MemorySegment.copy(src.array(), pos, bufSegment, ValueLayout.JAVA_BYTE, 0, len);
+ MemorySegment.copy(src.array(), src.arrayOffset() + pos, bufSegment, ValueLayout.JAVA_BYTE, 0, len);
}
final int sslWrote = SSL_write(ssl, srcSegment, len);
if (sslWrote > 0) {
@@ -274,7 +274,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
final int len = src.remaining();
MemorySegment srcSegment = src.isDirect() ? MemorySegment.ofBuffer(src) : bufSegment;
if (!src.isDirect()) {
- MemorySegment.copy(src.array(), pos, bufSegment, ValueLayout.JAVA_BYTE, 0, len);
+ MemorySegment.copy(src.array(), src.arrayOffset() + pos, bufSegment, ValueLayout.JAVA_BYTE, 0, len);
}
final int netWrote = BIO_write(networkBIO, srcSegment, len);
if (netWrote > 0) {
@@ -299,7 +299,8 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
final int sslRead = SSL_read(ssl, dstSegment, len);
if (sslRead > 0) {
if (!dst.isDirect()) {
- MemorySegment.copy(dstSegment, ValueLayout.JAVA_BYTE, 0, dst.array(), pos, sslRead);
+ MemorySegment.copy(dstSegment, ValueLayout.JAVA_BYTE, 0, dst.array(), dst.arrayOffset() + pos,
+ sslRead);
}
dst.position(pos + sslRead);
return sslRead;
@@ -322,7 +323,8 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
final int bioRead = BIO_read(networkBIO, dstSegment, pending);
if (bioRead > 0) {
if (!dst.isDirect()) {
- MemorySegment.copy(dstSegment, ValueLayout.JAVA_BYTE, 0, dst.array(), pos, bioRead);
+ MemorySegment.copy(dstSegment, ValueLayout.JAVA_BYTE, 0, dst.array(), dst.arrayOffset() + pos,
+ bioRead);
}
dst.position(pos + bioRead);
return bioRead;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 01/23: Account for TLS 1.3 in the enabled protocol management of the JNI OpenSSL SSLEngine
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit d90aebbbeed2ac9bfebfdd0a5ffb3b60982ac158
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 07:56:28 2026 +0200
Account for TLS 1.3 in the enabled protocol management of the JNI OpenSSL SSLEngine
The Tomcat Native based SSLEngine implementation advertised TLS 1.3 as
an implemented protocol (OpenSSL 1.1.1+) but never accounted for it in
its enabled-protocol bookkeeping:
- getEnabledProtocols() only inspected the SSL_OP_NO_SSLv3, NO_TLSv1,
NO_TLSv1_1 and NO_TLSv1_2 options, so it always omitted TLSv1.3 from
the returned list even when TLS 1.3 was enabled. Since the base
SSLEngine.getSSLParameters() is built from getEnabledProtocols(), web
applications using the SSLEngine request attribute saw an incomplete
list.
- setEnabledProtocols() had no TLSv1.3 case and never set the
SSL_OP_NO_TLSv1_3 option, so a requested protocol list excluding
TLS 1.3 silently left TLS 1.3 enabled.
Fix both by mirroring the Panama implementation which already handles
TLS 1.3 in these places, using the existing SSL.SSL_OP_NO_TLSv1_3
constant. This complements the equivalent fix recently applied to the
OpenSSLContext init reconfigure block.
---
java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java
index 2d16b64f38..fa75c1b43f 100644
--- a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java
+++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java
@@ -824,6 +824,9 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
if ((opts & SSL.SSL_OP_NO_TLSv1_2) == 0) {
enabled.add(Constants.SSL_PROTO_TLSv1_2);
}
+ if ((opts & SSL.SSL_OP_NO_TLSv1_3) == 0) {
+ enabled.add(Constants.SSL_PROTO_TLSv1_3);
+ }
if ((opts & SSL.SSL_OP_NO_SSLv3) == 0) {
enabled.add(Constants.SSL_PROTO_SSLv3);
}
@@ -846,6 +849,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
boolean tlsv1 = false;
boolean tlsv1_1 = false;
boolean tlsv1_2 = false;
+ boolean tlsv1_3 = false;
for (String p : protocols) {
if (!IMPLEMENTED_PROTOCOLS_SET.contains(p)) {
throw new IllegalArgumentException(sm.getString("engine.unsupportedProtocol", p));
@@ -855,6 +859,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
case Constants.SSL_PROTO_TLSv1 -> tlsv1 = true;
case Constants.SSL_PROTO_TLSv1_1 -> tlsv1_1 = true;
case Constants.SSL_PROTO_TLSv1_2 -> tlsv1_2 = true;
+ case Constants.SSL_PROTO_TLSv1_3 -> tlsv1_3 = true;
}
}
// Enable all and then disable what we not want
@@ -871,6 +876,9 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
if (!tlsv1_2) {
SSL.setOptionsLong(state.ssl, SSL.SSL_OP_NO_TLSv1_2);
}
+ if (!tlsv1_3) {
+ SSL.setOptionsLong(state.ssl, SSL.SSL_OP_NO_TLSv1_3);
+ }
}
@Override
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 21/23: Fix premature termination of InlineExecutorService
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 017c6dedcf47f6badead30ba3f65c0ea4c3f1a92
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 14:48:24 2026 +0200
Fix premature termination of InlineExecutorService
Track the number of running tasks with a counter instead of a single
boolean flag. The executor now reports itself terminated only when a
shutdown has been requested and no task is still running, which keeps
awaitTermination() from returning while a task is in progress and
allows shutdown() to run tasks queued by a running task. shutdown()
also notifies waiting threads when it declares termination.
---
.../tomcat/util/threads/InlineExecutorService.java | 13 +-
.../util/threads/TestInlineExecutorService.java | 147 +++++++++++++++++++++
2 files changed, 155 insertions(+), 5 deletions(-)
diff --git a/java/org/apache/tomcat/util/threads/InlineExecutorService.java b/java/org/apache/tomcat/util/threads/InlineExecutorService.java
index 999e2384ea..52ca30bf79 100644
--- a/java/org/apache/tomcat/util/threads/InlineExecutorService.java
+++ b/java/org/apache/tomcat/util/threads/InlineExecutorService.java
@@ -34,7 +34,7 @@ public class InlineExecutorService extends AbstractExecutorService {
}
private volatile boolean shutdown;
- private volatile boolean taskRunning;
+ private volatile int runningTasks;
private volatile boolean terminated;
private final Object lock = new Object();
@@ -43,7 +43,10 @@ public class InlineExecutorService extends AbstractExecutorService {
public void shutdown() {
shutdown = true;
synchronized (lock) {
- terminated = !taskRunning;
+ terminated = runningTasks == 0;
+ if (terminated) {
+ lock.notifyAll();
+ }
}
}
@@ -86,14 +89,14 @@ public class InlineExecutorService extends AbstractExecutorService {
if (shutdown) {
throw new RejectedExecutionException();
}
- taskRunning = true;
+ runningTasks++;
}
try {
command.run();
} finally {
synchronized (lock) {
- taskRunning = false;
- if (shutdown) {
+ runningTasks--;
+ if (shutdown && runningTasks == 0) {
terminated = true;
lock.notifyAll();
}
diff --git a/test/org/apache/tomcat/util/threads/TestInlineExecutorService.java b/test/org/apache/tomcat/util/threads/TestInlineExecutorService.java
new file mode 100644
index 0000000000..a1b3b5c7a0
--- /dev/null
+++ b/test/org/apache/tomcat/util/threads/TestInlineExecutorService.java
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.util.threads;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.RejectedExecutionException;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestInlineExecutorService {
+
+ /*
+ * Tasks run by multiple threads can be running at the same time. The service must not report termination until
+ * every running task has completed.
+ */
+ @Test
+ public void testTerminationWaitsForAllRunningTasks() throws Exception {
+ InlineExecutorService executor = new InlineExecutorService();
+
+ CountDownLatch tasksStarted = new CountDownLatch(2);
+ CountDownLatch firstTaskDone = new CountDownLatch(1);
+ CountDownLatch releaseSecondTask = new CountDownLatch(1);
+ CountDownLatch secondTaskDone = new CountDownLatch(1);
+
+ Runnable firstTask = () -> {
+ tasksStarted.countDown();
+ try {
+ Thread.sleep(300);
+ } catch (InterruptedException e) {
+ // Ignore
+ }
+ firstTaskDone.countDown();
+ };
+ Runnable secondTask = () -> {
+ tasksStarted.countDown();
+ try {
+ releaseSecondTask.await(10, TimeUnit.SECONDS);
+ } catch (InterruptedException e) {
+ // Ignore
+ }
+ secondTaskDone.countDown();
+ };
+
+ Thread firstThread = new Thread(() -> executor.execute(firstTask));
+ Thread secondThread = new Thread(() -> executor.execute(secondTask));
+
+ firstThread.start();
+ secondThread.start();
+
+ Assert.assertTrue("Tasks did not start", tasksStarted.await(10, TimeUnit.SECONDS));
+ Assert.assertTrue("First task did not complete", firstTaskDone.await(10, TimeUnit.SECONDS));
+
+ // The second task is still running at this point
+ executor.shutdown();
+ Assert.assertFalse("Terminated while a task was still running", executor.isTerminated());
+ Assert.assertFalse("awaitTermination returned true while a task was still running",
+ executor.awaitTermination(100, TimeUnit.MILLISECONDS));
+
+ releaseSecondTask.countDown();
+ firstThread.join(TimeUnit.SECONDS.toMillis(10));
+ secondThread.join(TimeUnit.SECONDS.toMillis(10));
+ Assert.assertFalse("Second thread did not complete", secondThread.isAlive());
+
+ Assert.assertTrue("Not terminated after all tasks completed", executor.isTerminated());
+ Assert.assertTrue("awaitTermination returned false after termination",
+ executor.awaitTermination(10, TimeUnit.SECONDS));
+ }
+
+
+ /*
+ * A task that submits another task (which runs inline on the same thread) must leave the service in a state in
+ * which it can terminate normally once both tasks have completed.
+ */
+ @Test
+ public void testReentrantExecute() throws Exception {
+ InlineExecutorService executor = new InlineExecutorService();
+
+ executor.execute(() -> executor.execute(() -> {
+ // Inner task. Does nothing.
+ }));
+
+ executor.shutdown();
+ Assert.assertTrue("Not terminated after nested tasks completed", executor.isTerminated());
+ }
+
+
+ /*
+ * Shutting down a service with no running tasks terminates it. Threads already waiting for termination must be
+ * woken rather than only noticing the termination when their timeout expires.
+ */
+ @Test
+ public void testAwaitTerminationWakesOnShutdown() throws Exception {
+ InlineExecutorService executor = new InlineExecutorService();
+
+ CountDownLatch waiterReturned = new CountDownLatch(1);
+ Boolean[] result = new Boolean[1];
+ Thread waiter = new Thread(() -> {
+ try {
+ result[0] = executor.awaitTermination(60, TimeUnit.SECONDS);
+ } catch (InterruptedException e) {
+ // Ignore
+ }
+ waiterReturned.countDown();
+ });
+ waiter.start();
+
+ // Give the waiter thread time to start waiting
+ Thread.sleep(100);
+
+ executor.shutdown();
+
+ Assert.assertTrue("Waiting thread was not woken", waiterReturned.await(10, TimeUnit.SECONDS));
+ Assert.assertEquals(Boolean.TRUE, result[0]);
+ waiter.join(TimeUnit.SECONDS.toMillis(10));
+ }
+
+
+ /*
+ * Tasks must be rejected once the service has been shut down.
+ */
+ @Test
+ public void testRejectAfterShutdown() throws Exception {
+ InlineExecutorService executor = new InlineExecutorService();
+
+ executor.shutdown();
+
+ Assert.assertThrows(RejectedExecutionException.class, () -> executor.execute(() -> {
+ // Does nothing.
+ }));
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 13/23: Correct the selection of multi-release JAR entries in JarFileUrlJar
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit a2725acb46f1ad6fe3f6542ec273e8f67dcefa29
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 12:58:13 2026 +0200
Correct the selection of multi-release JAR entries in JarFileUrlJar
The entry selected for a class present with multiple versions depended
on the order in which entries appeared in the JAR: a versioned entry
encountered first pinned the selection, so an older version could be
used, and an entry that only exists for a later runtime version could
be selected although the runtime would never use it. The selection now
builds a map of the highest applicable versioned entry for each base
class name, mirroring the stream-based implementation, and resolves
every first-seen base name through it, so the scanned entry matches the
entry the runtime would use regardless of entry order. Versioned
non-class entries are resolved to the base entry, as the stream-based
implementations already do, and entries only applicable to a later
runtime are skipped.
---
.../org/apache/tomcat/util/scan/JarFileUrlJar.java | 81 +++++++-
.../apache/tomcat/util/scan/TestJarFileUrlJar.java | 226 +++++++++++++++++++++
webapps/docs/changelog.xml | 11 +
3 files changed, 315 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/tomcat/util/scan/JarFileUrlJar.java b/java/org/apache/tomcat/util/scan/JarFileUrlJar.java
index 901e366004..b5a89464ae 100644
--- a/java/org/apache/tomcat/util/scan/JarFileUrlJar.java
+++ b/java/org/apache/tomcat/util/scan/JarFileUrlJar.java
@@ -23,7 +23,9 @@ import java.net.JarURLConnection;
import java.net.URISyntaxException;
import java.net.URL;
import java.util.Enumeration;
+import java.util.HashMap;
import java.util.HashSet;
+import java.util.Map;
import java.util.Set;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
@@ -44,6 +46,7 @@ public class JarFileUrlJar implements Jar {
private final boolean multiRelease;
private Enumeration<JarEntry> entries;
private Set<String> entryNamesSeen;
+ private Map<String,JarEntry> multiReleaseMap;
private JarEntry entry = null;
/**
@@ -157,6 +160,14 @@ public class JarFileUrlJar implements Jar {
// - the case where an entry appears in the versions location
// but not in the base location is handled correctly
+ // The look-up of the entry to use is based on the 'base' name of
+ // each entry and the highest applicable version of each class, so
+ // that the correct entry is selected independently of the order of
+ // the entries in the JAR.
+ if (multiReleaseMap == null) {
+ multiReleaseMap = populateMultiReleaseMap();
+ }
+
// Enumerate the entries until one is reached that represents an
// entry that has not been seen before.
String name;
@@ -175,11 +186,25 @@ public class JarFileUrlJar implements Jar {
if (name.isEmpty() || entryNamesSeen.contains(name)) {
continue;
}
-
entryNamesSeen.add(name);
- // JarFile.getJarEntry is version aware so use it
- entry = jarFile.getJarEntry(entry.getName());
+ JarEntry versionedEntry = multiReleaseMap.get(name);
+ if (versionedEntry != null) {
+ // Use the highest applicable version of this class
+ entry = versionedEntry;
+ } else {
+ // Use the base entry. Classes with a version that
+ // applies to this runtime were resolved from the
+ // multi-release map above.
+ entry = jarFile.getJarEntry(name);
+ }
+
+ if (entry == null) {
+ // There is no entry that applies to this runtime, for
+ // example the entry only exists in the versions
+ // location with a version later than this runtime
+ continue;
+ }
} else {
entry = null;
}
@@ -222,6 +247,56 @@ public class JarFileUrlJar implements Jar {
public void reset() throws IOException {
entries = null;
entryNamesSeen = null;
+ multiReleaseMap = null;
entry = null;
}
+
+
+ /**
+ * Identifies, for each class that appears in the versions location of the JAR, the entry for the highest version
+ * that applies to the current runtime. Keyed by 'base' name, which is the name with the
+ * <code>META-INF/versions/<version>/</code> prefix removed.
+ *
+ * @return the map of 'base' name to the entry to use for that name
+ */
+ private Map<String,JarEntry> populateMultiReleaseMap() {
+ int targetVersion = Runtime.version().feature();
+
+ Map<String,Integer> mrVersions = new HashMap<>();
+ Map<String,JarEntry> result = new HashMap<>();
+
+ // Tracking the base name and the latest valid version found is
+ // sufficient to be able to create the map of entries to use
+ Enumeration<JarEntry> jarEntries = jarFile.entries();
+ while (jarEntries.hasMoreElements()) {
+ JarEntry jarEntry = jarEntries.nextElement();
+ String name = jarEntry.getName();
+ if (name.startsWith("META-INF/versions/") && name.endsWith(".class")) {
+ int i = name.indexOf('/', 18);
+ if (i > 0) {
+ String baseName = name.substring(i + 1);
+ int version;
+ try {
+ version = Integer.parseInt(name, 18, i, 10);
+ } catch (NumberFormatException e) {
+ continue;
+ }
+
+ // Ignore any entries targeting for a later version than
+ // the target for this runtime
+ if (version <= targetVersion) {
+ Integer mappedVersion = mrVersions.get(baseName);
+ // Ignore any entry for which a later version has
+ // already been found
+ if (mappedVersion == null || version > mappedVersion.intValue()) {
+ mrVersions.put(baseName, Integer.valueOf(version));
+ result.put(baseName, jarEntry);
+ }
+ }
+ }
+ }
+ }
+
+ return result;
+ }
}
diff --git a/test/org/apache/tomcat/util/scan/TestJarFileUrlJar.java b/test/org/apache/tomcat/util/scan/TestJarFileUrlJar.java
new file mode 100644
index 0000000000..eacef7ee04
--- /dev/null
+++ b/test/org/apache/tomcat/util/scan/TestJarFileUrlJar.java
@@ -0,0 +1,226 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.util.scan;
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URI;
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.jar.Attributes;
+import java.util.jar.JarEntry;
+import java.util.jar.JarOutputStream;
+import java.util.jar.Manifest;
+
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import org.apache.tomcat.Jar;
+
+public class TestJarFileUrlJar {
+
+ @Rule
+ public TemporaryFolder testDir = new TemporaryFolder();
+
+ private static final String VERSIONS = "META-INF/versions/";
+ private static final String BASE_CLASS = "test/Foo.class";
+
+ private File createMultiReleaseJar(String... entryNames) throws IOException {
+ Manifest manifest = new Manifest();
+ manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0");
+ manifest.getMainAttributes().putValue("Multi-Release", "true");
+
+ File file = new File(testDir.getRoot(), "mr.jar");
+ try (JarOutputStream jos = new JarOutputStream(new FileOutputStream(file), manifest)) {
+ for (String entryName : entryNames) {
+ jos.putNextEntry(new JarEntry(entryName));
+ jos.write(new byte[] { 1, 2, 3 });
+ jos.closeEntry();
+ }
+ }
+ return file;
+ }
+
+ private static String readEntry(Jar jar) throws IOException {
+ try (InputStream is = jar.getEntryInputStream()) {
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ byte[] buffer = new byte[1024];
+ int read = is.read(buffer);
+ while (read > 0) {
+ baos.write(buffer, 0, read);
+ read = is.read(buffer);
+ }
+ return baos.toString("ISO-8859-1");
+ }
+ }
+
+ private static List<String> entryNames(Jar jar) throws IOException {
+ List<String> names = new ArrayList<>();
+ jar.nextEntry();
+ while (jar.getEntryName() != null) {
+ names.add(jar.getEntryName());
+ jar.nextEntry();
+ }
+ return names;
+ }
+
+ /*
+ * The entry selected for a class that is present with multiple versions must be the highest version that applies
+ * to the current runtime, regardless of the order in which the entries appear in the JAR.
+ */
+ @Test
+ public void testMultiReleaseVersionSelectionOrderIndependent() throws Exception {
+ // The lower version first
+ File file = createMultiReleaseJar(VERSIONS + "9/" + BASE_CLASS, VERSIONS + "11/" + BASE_CLASS, BASE_CLASS);
+ try (Jar jar = new JarFileUrlJar(file.toURI().toURL(), false)) {
+ Assert.assertTrue(entryNames(jar).contains(VERSIONS + "11/" + BASE_CLASS));
+ }
+
+ // The higher version first
+ file = createMultiReleaseJar(VERSIONS + "11/" + BASE_CLASS, VERSIONS + "9/" + BASE_CLASS, BASE_CLASS);
+ try (Jar jar = new JarFileUrlJar(file.toURI().toURL(), false)) {
+ Assert.assertTrue(entryNames(jar).contains(VERSIONS + "11/" + BASE_CLASS));
+ }
+
+ // The base entry first
+ file = createMultiReleaseJar(BASE_CLASS, VERSIONS + "9/" + BASE_CLASS, VERSIONS + "11/" + BASE_CLASS);
+ try (Jar jar = new JarFileUrlJar(file.toURI().toURL(), false)) {
+ Assert.assertTrue(entryNames(jar).contains(VERSIONS + "11/" + BASE_CLASS));
+ }
+
+ // Versioned entries only
+ file = createMultiReleaseJar(VERSIONS + "9/" + BASE_CLASS, VERSIONS + "11/" + BASE_CLASS);
+ try (Jar jar = new JarFileUrlJar(file.toURI().toURL(), false)) {
+ Assert.assertTrue(entryNames(jar).contains(VERSIONS + "11/" + BASE_CLASS));
+ }
+ }
+
+
+ /*
+ * Entries that are only available for a later version than the current runtime must not be selected. The base
+ * entry is used where one exists.
+ */
+ @Test
+ public void testMultiReleaseFutureVersion() throws Exception {
+ // The name is unlikely to ever be a valid class but it is certain to be later than the current runtime
+ String futureClass = VERSIONS + "999999/" + BASE_CLASS;
+
+ // Future versioned entry first, base entry available
+ File file = createMultiReleaseJar(futureClass, BASE_CLASS);
+ try (Jar jar = new JarFileUrlJar(file.toURI().toURL(), false)) {
+ List<String> names = entryNames(jar);
+ Assert.assertTrue(names.contains(BASE_CLASS));
+ Assert.assertFalse(names.contains(futureClass));
+ }
+
+ // Future versioned entry only
+ file = createMultiReleaseJar(futureClass);
+ try (Jar jar = new JarFileUrlJar(file.toURI().toURL(), false)) {
+ List<String> names = entryNames(jar);
+ Assert.assertFalse(names.contains(futureClass));
+ Assert.assertFalse(names.contains(BASE_CLASS));
+ }
+ }
+
+
+ /*
+ * The entry selected must be the entry the current runtime would use. The content of the selected entry is used
+ * here to make the selection visible.
+ */
+ @Test
+ public void testMultiReleaseSelectedEntryContent() throws Exception {
+ Manifest manifest = new Manifest();
+ manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0");
+ manifest.getMainAttributes().putValue("Multi-Release", "true");
+
+ File file = new File(testDir.getRoot(), "content.jar");
+ try (JarOutputStream jos = new JarOutputStream(new FileOutputStream(file), manifest)) {
+ jos.putNextEntry(new JarEntry(VERSIONS + "9/" + BASE_CLASS));
+ jos.write("nine".getBytes("ISO-8859-1"));
+ jos.closeEntry();
+ jos.putNextEntry(new JarEntry(VERSIONS + "11/" + BASE_CLASS));
+ jos.write("eleven".getBytes("ISO-8859-1"));
+ jos.closeEntry();
+ jos.putNextEntry(new JarEntry(BASE_CLASS));
+ jos.write("base".getBytes("ISO-8859-1"));
+ jos.closeEntry();
+ }
+
+ try (Jar jar = new JarFileUrlJar(file.toURI().toURL(), false)) {
+ String selected = null;
+ jar.nextEntry();
+ while (jar.getEntryName() != null) {
+ if (jar.getEntryName().endsWith("Foo.class")) {
+ selected = readEntry(jar);
+ }
+ jar.nextEntry();
+ }
+ Assert.assertEquals("eleven", selected);
+ }
+ }
+
+
+ /*
+ * Re-enumeration after reset() must produce the same selection. Entries under a version directory that cannot be
+ * parsed must not be returned. Versioned entries for resources (non-class entries) are handled the same way as by
+ * the stream based implementations: the base entry is used where one exists and the versioned entries are
+ * ignored.
+ */
+ @Test
+ public void testMultiReleaseResetAndUnusableEntries() throws Exception {
+ String unparseable = VERSIONS + "abc/" + BASE_CLASS;
+ String versionedResource = VERSIONS + "9/test/resource.txt";
+ String resource = "test/resource.txt";
+
+ File file = createMultiReleaseJar(unparseable, versionedResource, VERSIONS + "9/" + BASE_CLASS,
+ VERSIONS + "11/" + BASE_CLASS, BASE_CLASS, resource);
+ try (Jar jar = new JarFileUrlJar(file.toURI().toURL(), false)) {
+ List<String> names = entryNames(jar);
+ Assert.assertFalse(names.contains(unparseable));
+ Assert.assertFalse(names.contains(VERSIONS + "9/" + BASE_CLASS));
+ Assert.assertTrue(names.contains(VERSIONS + "11/" + BASE_CLASS));
+ Assert.assertFalse(names.contains(BASE_CLASS));
+ Assert.assertFalse(names.contains(versionedResource));
+ Assert.assertTrue(names.contains(resource));
+
+ jar.reset();
+ Assert.assertEquals(names, entryNames(jar));
+ }
+ }
+
+
+ /*
+ * The version selection must work the same way for a JAR opened via a jar: URL.
+ */
+ @Test
+ public void testMultiReleaseVersionSelectionViaJarUrl() throws Exception {
+ File file = createMultiReleaseJar(VERSIONS + "9/" + BASE_CLASS, VERSIONS + "11/" + BASE_CLASS, BASE_CLASS);
+ URL url = URI.create("jar:" + file.toURI().toURL().toExternalForm() + "!/").toURL();
+ try (Jar jar = new JarFileUrlJar(url, true)) {
+ List<String> names = entryNames(jar);
+ Assert.assertTrue(names.contains(VERSIONS + "11/" + BASE_CLASS));
+ Assert.assertFalse(names.contains(VERSIONS + "9/" + BASE_CLASS));
+ Assert.assertFalse(names.contains(BASE_CLASS));
+ }
+ }
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index e900eea853..519a4a3ae0 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -371,6 +371,17 @@
authentication type for almost every cipher suite. The computation now
matches the one performed by the JNI based implementation. (remm)
</fix>
+ <fix>
+ Correct the selection of multi-release JAR entries by the
+ <code>jar:file:</code> and <code>file:</code> based JAR
+ implementation used when scanning. The entry selected for a class
+ present with multiple versions depended on the order of the entries
+ in the JAR, so an older version, or a version intended for a later
+ runtime, could be scanned instead of the version the runtime would
+ use. The highest applicable version is now selected, independently
+ of the entry order, matching the behaviour of the stream based JAR
+ implementation. (remm)
+ </fix>
</changelog>
</subsection>
<subsection name="Web applications">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 22/23: Fix submitted task count drift in ThreadPoolExecutor
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 3d92ff34401048496384e38390a5f0aac7cd406c
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 15:25:50 2026 +0200
Fix submitted task count drift in ThreadPoolExecutor
Adjust the submitted task counter on every path where a counted task
can no longer reach afterExecute(): a null command, a rejection
handler that does not throw, removal of a queued task via remove() or
purge(), and a beforeExecute override that throws. The counter feeds
TaskQueue's queue-or-create-thread decision, so a drifted value makes
idle pools create unnecessary threads.
---
.../tomcat/util/threads/ThreadPoolExecutor.java | 61 ++++-
.../util/threads/TestThreadPoolExecutor.java | 246 +++++++++++++++++++++
webapps/docs/changelog.xml | 9 +
3 files changed, 309 insertions(+), 7 deletions(-)
diff --git a/java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java b/java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java
index 3ce64d281f..5c31dfa737 100644
--- a/java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java
+++ b/java/org/apache/tomcat/util/threads/ThreadPoolExecutor.java
@@ -660,7 +660,20 @@ public class ThreadPoolExecutor extends AbstractExecutorService {
* ScheduledThreadPoolExecutor.
*/
final void reject(Runnable command) {
- handler.rejectedExecution(command, this);
+ try {
+ handler.rejectedExecution(command, this);
+ } catch (RejectedExecutionException rx) {
+ // The caller in execute() decrements the submitted count, or, when a retryable queue is used, re-queues
+ // the task which will then be counted by afterExecute() once it completes.
+ throw rx;
+ } catch (RuntimeException | Error e) {
+ // The task will not be executed by a worker, so afterExecute() will not decrement the submitted count.
+ submittedCount.decrementAndGet();
+ throw e;
+ }
+ // The handler completed without throwing. The task was discarded or run directly by the handler. It will not
+ // pass through afterExecute(), so decrement the submitted count here.
+ submittedCount.decrementAndGet();
}
/**
@@ -942,7 +955,15 @@ public class ThreadPoolExecutor extends AbstractExecutorService {
wt.interrupt();
}
try {
- beforeExecute(wt, task);
+ try {
+ beforeExecute(wt, task);
+ } catch (Throwable ex) {
+ // beforeExecute() threw. The task will not be executed
+ // and afterExecute() will not be called, so decrement
+ // the submitted count here.
+ submittedCount.decrementAndGet();
+ throw ex;
+ }
try {
task.run();
afterExecute(task, null);
@@ -1090,6 +1111,11 @@ public class ThreadPoolExecutor extends AbstractExecutorService {
@Override
public void execute(Runnable command) {
+ // Check for null before counting the task, so a rejected null
+ // argument does not leave the submitted count inflated.
+ if (command == null) {
+ throw new NullPointerException();
+ }
submittedCount.incrementAndGet();
try {
executeInternal(command);
@@ -1150,7 +1176,11 @@ public class ThreadPoolExecutor extends AbstractExecutorService {
}
if (isRunning(c) && workQueue.offer(command)) {
int recheck = ctl.get();
- if (!isRunning(recheck) && remove(command)) {
+ // Remove directly from the queue here. The public remove(Runnable)
+ // method also adjusts the submitted count, which would double the
+ // adjustment for this task, since the reject() call below and the
+ // catch block in execute() take care of that.
+ if (!isRunning(recheck) && workQueue.remove(command)) {
reject(command);
} else if (workerCountOf(recheck) == 0) {
addWorker(null, false);
@@ -1564,6 +1594,10 @@ public class ThreadPoolExecutor extends AbstractExecutorService {
*/
public boolean remove(Runnable task) {
boolean removed = workQueue.remove(task);
+ if (removed) {
+ // The removed task will never be executed, so afterExecute() will not decrement the submitted count.
+ submittedCount.decrementAndGet();
+ }
tryTerminate(); // In case SHUTDOWN and now empty
return removed;
}
@@ -1577,19 +1611,32 @@ public class ThreadPoolExecutor extends AbstractExecutorService {
*/
public void purge() {
final BlockingQueue<Runnable> q = workQueue;
+ // Count the cancelled tasks that are removed. These will never be
+ // executed, so afterExecute() will not decrement the submitted count.
+ final AtomicInteger purged = new AtomicInteger(0);
try {
- q.removeIf(r -> r instanceof Future<?> && ((Future<?>) r).isCancelled());
+ q.removeIf(r -> {
+ if (r instanceof Future<?> future && future.isCancelled()) {
+ purged.incrementAndGet();
+ return true;
+ }
+ return false;
+ });
} catch (ConcurrentModificationException fallThrough) {
// Take slow path if we encounter interference during traversal.
// Make copy for traversal and call remove for cancelled entries.
- // The slow path is more likely to be O(N*N).
+ // The slow path is more likely to be O(N*N). Any entries removed
+ // before the interference occurred are already accounted for in
+ // the purged count and will not appear in the copy taken below.
for (Object r : q.toArray()) {
- if (r instanceof Future<?> && ((Future<?>) r).isCancelled()) {
- q.remove(r);
+ if (r instanceof Future<?> future && future.isCancelled() && q.remove(r)) {
+ purged.incrementAndGet();
}
}
}
+ submittedCount.addAndGet(-purged.get());
+
tryTerminate(); // In case SHUTDOWN and now empty
}
diff --git a/test/org/apache/tomcat/util/threads/TestThreadPoolExecutor.java b/test/org/apache/tomcat/util/threads/TestThreadPoolExecutor.java
new file mode 100644
index 0000000000..0c299bf6fb
--- /dev/null
+++ b/test/org/apache/tomcat/util/threads/TestThreadPoolExecutor.java
@@ -0,0 +1,246 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.util.threads;
+
+import java.util.concurrent.ArrayBlockingQueue;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestThreadPoolExecutor {
+
+ /*
+ * The submitted task count is used by TaskQueue.offer() to decide if a task should be queued or a new thread
+ * should be created. A task that is counted when submitted but never reaches afterExecute() leaves the count
+ * permanently inflated.
+ */
+
+ private static ThreadPoolExecutor createPool(ThreadPoolExecutor.RejectedExecutionHandler handler) {
+ // Single thread with room for a single queued task. The task following
+ // the queued one is rejected.
+ return new ThreadPoolExecutor(1, 1, 60, TimeUnit.SECONDS, new ArrayBlockingQueue<>(1),
+ Executors.defaultThreadFactory(), handler);
+ }
+
+ private static Runnable blockingTask(String name, CountDownLatch release) {
+ return blockingTask(name, null, release);
+ }
+
+ private static Runnable blockingTask(String name, CountDownLatch started, CountDownLatch release) {
+ return new Runnable() {
+ @Override
+ public void run() {
+ if (started != null) {
+ started.countDown();
+ }
+ try {
+ release.await();
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ }
+ }
+
+ @Override
+ public String toString() {
+ return name;
+ }
+ };
+ }
+
+ /*
+ * Waits until the running task has started. getActiveCount() is not usable for this: it is approximate, and a
+ * worker that the pool constructor prestarted counts as active before it has taken its first task.
+ */
+ private static void awaitStarted(CountDownLatch started) throws InterruptedException {
+ Assert.assertTrue(started.await(5, TimeUnit.SECONDS));
+ }
+
+ private static void awaitSubmittedCount(ThreadPoolExecutor pool, int expected) throws InterruptedException {
+ long end = System.currentTimeMillis() + 5000;
+ while (pool.getSubmittedCount() != expected && System.currentTimeMillis() < end) {
+ Thread.sleep(20);
+ }
+ Assert.assertEquals(expected, pool.getSubmittedCount());
+ }
+
+ /*
+ * A rejected execution handler that does not throw discards the task or runs it in the calling thread. Either way
+ * the task never reaches afterExecute(), so the submitted count must be decremented when the handler returns.
+ */
+ @Test
+ public void testNonThrowingRejectionHandlerKeepsSubmittedCount() throws Exception {
+ // Test both handlers that do not throw.
+ for (ThreadPoolExecutor.RejectedExecutionHandler handler :
+ new ThreadPoolExecutor.RejectedExecutionHandler[] { new ThreadPoolExecutor.DiscardPolicy(),
+ new ThreadPoolExecutor.CallerRunsPolicy() }) {
+ CountDownLatch release = new CountDownLatch(1);
+ boolean expectRunInCaller = handler instanceof ThreadPoolExecutor.CallerRunsPolicy;
+ ThreadPoolExecutor pool = createPool(handler);
+ try {
+ CountDownLatch started = new CountDownLatch(1);
+ pool.execute(blockingTask("running", started, release));
+ awaitStarted(started);
+ pool.execute(blockingTask("queued", release));
+ AtomicBoolean ran = new AtomicBoolean();
+ pool.execute(() -> ran.set(true)); // rejected: discarded or run by this thread
+ // Only the running and the queued task are still outstanding.
+ Assert.assertEquals(handler.getClass().getName(), 2, pool.getSubmittedCount());
+ Assert.assertEquals(handler.getClass().getName(), expectRunInCaller, ran.get());
+ release.countDown();
+ awaitSubmittedCount(pool, 0);
+ } finally {
+ pool.shutdownNow();
+ }
+ }
+ }
+
+ /*
+ * A null command is rejected with a NullPointerException before it can be counted.
+ */
+ @Test
+ public void testNullCommandKeepsSubmittedCount() throws Exception {
+ ThreadPoolExecutor pool = createPool(new ThreadPoolExecutor.AbortPolicy());
+ try {
+ try {
+ pool.execute(null);
+ Assert.fail("Expected NullPointerException");
+ } catch (NullPointerException e) {
+ // Expected.
+ }
+ Assert.assertEquals(0, pool.getSubmittedCount());
+ } finally {
+ pool.shutdownNow();
+ }
+ }
+
+ /*
+ * A task removed from the queue is never executed and never reaches afterExecute(), so the removal must decrement
+ * the submitted count.
+ */
+ @Test
+ public void testRemoveKeepsSubmittedCount() throws Exception {
+ CountDownLatch release = new CountDownLatch(1);
+ ThreadPoolExecutor pool = createPool(new ThreadPoolExecutor.AbortPolicy());
+ try {
+ Runnable queued = blockingTask("queued", release);
+ CountDownLatch started = new CountDownLatch(1);
+ pool.execute(blockingTask("running", started, release));
+ awaitStarted(started);
+ pool.execute(queued);
+ Assert.assertTrue(pool.remove(queued));
+ // Only the running task is still outstanding.
+ Assert.assertEquals(1, pool.getSubmittedCount());
+ release.countDown();
+ awaitSubmittedCount(pool, 0);
+ } finally {
+ pool.shutdownNow();
+ }
+ }
+
+ /*
+ * A cancelled future removed by purge() is never executed and never reaches afterExecute(), so the removal must
+ * decrement the submitted count.
+ */
+ @Test
+ public void testPurgeKeepsSubmittedCount() throws Exception {
+ CountDownLatch release = new CountDownLatch(1);
+ ThreadPoolExecutor pool = createPool(new ThreadPoolExecutor.AbortPolicy());
+ try {
+ CountDownLatch started = new CountDownLatch(1);
+ pool.execute(blockingTask("running", started, release));
+ awaitStarted(started);
+ Future<?> queued = pool.submit(blockingTask("queued", release));
+ // The submitted future is in the queue since the only worker is busy.
+ Assert.assertTrue(queued.cancel(false));
+ pool.purge();
+ // Only the running task is still outstanding.
+ Assert.assertEquals(1, pool.getSubmittedCount());
+ release.countDown();
+ awaitSubmittedCount(pool, 0);
+ } finally {
+ pool.shutdownNow();
+ }
+ }
+
+ /*
+ * If beforeExecute() throws, the worker dies without processing the task and without calling afterExecute(), so
+ * the submitted count must be decremented for the lost task.
+ */
+ @Test
+ public void testBeforeExecuteThrowKeepsSubmittedCount() throws Exception {
+ CountDownLatch threw = new CountDownLatch(1);
+ CountDownLatch release = new CountDownLatch(1);
+ class BeforeExecutorThrows extends ThreadPoolExecutor {
+ BeforeExecutorThrows() {
+ super(1, 1, 60, TimeUnit.SECONDS, new ArrayBlockingQueue<>(1), Executors.defaultThreadFactory(),
+ new AbortPolicy());
+ }
+
+ @Override
+ protected void beforeExecute(Thread t, Runnable r) {
+ if ("marker".equals(r.toString())) {
+ threw.countDown();
+ throw new IllegalStateException("Test beforeExecute failure");
+ }
+ }
+ }
+
+ BeforeExecutorThrows pool = new BeforeExecutorThrows();
+ try {
+ pool.execute(blockingTask("marker", release));
+ // The worker should die without executing the task.
+ Assert.assertTrue(threw.await(5, TimeUnit.SECONDS));
+ // The task is lost with the worker, so nothing is outstanding.
+ awaitSubmittedCount(pool, 0);
+ } finally {
+ release.countDown();
+ pool.shutdownNow();
+ }
+ }
+
+ /*
+ * Tasks that complete normally or that throw must each decrement the submitted count exactly once.
+ */
+ @Test
+ public void testTaskCompletionDecrementsOnce() throws Exception {
+ ThreadPoolExecutor pool = new ThreadPoolExecutor(2, 2, 60, TimeUnit.SECONDS, new ArrayBlockingQueue<>(20),
+ Executors.defaultThreadFactory(), new ThreadPoolExecutor.AbortPolicy());
+ try {
+ int taskCount = 10;
+ CountDownLatch done = new CountDownLatch(taskCount);
+ for (int i = 0; i < taskCount; i++) {
+ final int index = i;
+ pool.execute(() -> {
+ done.countDown();
+ if (index % 2 == 0) {
+ throw new IllegalStateException("Test task failure " + index);
+ }
+ });
+ }
+ Assert.assertTrue(done.await(5, TimeUnit.SECONDS));
+ awaitSubmittedCount(pool, 0);
+ Assert.assertTrue(pool.getSubmittedCount() >= 0);
+ } finally {
+ pool.shutdownNow();
+ }
+ }
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 519a4a3ae0..83101c9a1d 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -382,6 +382,15 @@
of the entry order, matching the behaviour of the stream based JAR
implementation. (remm)
</fix>
+ <fix>
+ Correct the accounting of the count of submitted tasks that
+ <code>ThreadPoolExecutor</code> maintains for <code>TaskQueue</code>.
+ The count was not always adjusted when a submitted task could no
+ longer complete, such as when a rejected execution handler did not
+ throw, or when a queued task was removed. A drifted value caused
+ thread pools with idle threads to create unnecessary threads.
+ (remm)
+ </fix>
</changelog>
</subsection>
<subsection name="Web applications">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 18/23: Require at least one iteration when computing message digests
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 5090bdfa2d82c15fc044ee06f7d91aa5d666033e
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 14:08:26 2026 +0200
Require at least one iteration when computing message digests
ConcurrentMessageDigest.digest(String, int, byte[]...) documents the
iterations argument as the number of times the digest is applied, but
the value was not checked. A count of zero or less still produced the
digest of the input, because the first round runs unconditionally and
the additional rounds only run when the count is greater than one.
Callers that passed such a count silently received a result that did
not match the documented semantics.
The method now throws IllegalArgumentException when fewer than one
iteration is requested, which is consistent with how the JDK validates
the iteration count of PBEKeySpec.
Stored credentials used by the message digest credential handler carry
their own iteration count. A stored count below one was previously
accepted and silently verified as a single iteration. Such credentials
are now reported as invalid, matching the existing handling of other
malformed stored credentials, so verification returns false instead of
using an iteration count the digest computation rejects.
Add regression tests covering the rejected counts, repeated iteration
semantics and stored credentials with an invalid iteration count.
---
.../realm/DigestCredentialHandlerBase.java | 6 ++
.../util/security/ConcurrentMessageDigest.java | 7 +++
.../tomcat/util/security/LocalStrings.properties | 1 +
.../realm/TestMessageDigestCredentialHandler.java | 23 +++++++
.../util/security/TestConcurrentMessageDigest.java | 70 ++++++++++++++++++----
5 files changed, 96 insertions(+), 11 deletions(-)
diff --git a/java/org/apache/catalina/realm/DigestCredentialHandlerBase.java b/java/org/apache/catalina/realm/DigestCredentialHandlerBase.java
index 6c4b216e64..ced174e5ce 100644
--- a/java/org/apache/catalina/realm/DigestCredentialHandlerBase.java
+++ b/java/org/apache/catalina/realm/DigestCredentialHandlerBase.java
@@ -198,6 +198,12 @@ public abstract class DigestCredentialHandlerBase implements CredentialHandler {
return false;
}
+ if (iterations < 1) {
+ // Stored credentials must specify at least one iteration
+ logInvalidStoredCredentials(storedCredentials);
+ return false;
+ }
+
String storedHexEncoded = storedCredentials.substring(sep2 + 1);
byte[] salt;
try {
diff --git a/java/org/apache/tomcat/util/security/ConcurrentMessageDigest.java b/java/org/apache/tomcat/util/security/ConcurrentMessageDigest.java
index ec785a2246..06e0bd4600 100644
--- a/java/org/apache/tomcat/util/security/ConcurrentMessageDigest.java
+++ b/java/org/apache/tomcat/util/security/ConcurrentMessageDigest.java
@@ -117,9 +117,16 @@ public class ConcurrentMessageDigest {
* @param iterations the number of times to apply the digest
* @param input the byte arrays to digest
* @return the digest
+ *
+ * @throws IllegalArgumentException if iterations is less than one
*/
public static byte[] digest(String algorithm, int iterations, byte[]... input) {
+ if (iterations < 1) {
+ throw new IllegalArgumentException(
+ sm.getString("concurrentMessageDigest.invalidIterations", Integer.toString(iterations)));
+ }
+
Queue<MessageDigest> queue = queues.get(algorithm);
if (queue == null) {
throw new IllegalStateException(sm.getString("concurrentMessageDigest.noDigest"));
diff --git a/java/org/apache/tomcat/util/security/LocalStrings.properties b/java/org/apache/tomcat/util/security/LocalStrings.properties
index 310e51c13b..6b9a406e91 100644
--- a/java/org/apache/tomcat/util/security/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/security/LocalStrings.properties
@@ -16,4 +16,5 @@
# Do not edit this file directly.
# To edit translations see: https://tomcat.apache.org/getinvolved.html#Translations
+concurrentMessageDigest.invalidIterations=At least one iteration is required. [{0}]
concurrentMessageDigest.noDigest=Digest algorithm not initialized or unavailable
diff --git a/test/org/apache/catalina/realm/TestMessageDigestCredentialHandler.java b/test/org/apache/catalina/realm/TestMessageDigestCredentialHandler.java
index 2020da3e7d..5b79d71f63 100644
--- a/test/org/apache/catalina/realm/TestMessageDigestCredentialHandler.java
+++ b/test/org/apache/catalina/realm/TestMessageDigestCredentialHandler.java
@@ -48,6 +48,29 @@ public class TestMessageDigestCredentialHandler {
}
}
+ /*
+ * A stored credential that specifies fewer than one iteration is invalid. It must not be treated as valid and it
+ * must not trigger an exception during verification.
+ */
+ @Test
+ public void testZeroIterationsStoredCredential() throws Exception {
+ MessageDigestCredentialHandler mdch = new MessageDigestCredentialHandler();
+ mdch.setAlgorithm("SHA-512");
+ mdch.setIterations(1);
+ mdch.setSaltLength(8);
+ String storedCredential = mdch.mutate(PWD);
+
+ int sep1 = storedCredential.indexOf('$');
+ int sep2 = storedCredential.indexOf('$', sep1 + 1);
+ Assert.assertTrue(sep1 > 0 && sep2 > sep1);
+ String tamperedCredential = storedCredential.substring(0, sep1 + 1) + "0" + storedCredential.substring(sep2);
+
+ MessageDigestCredentialHandler verifier = new MessageDigestCredentialHandler();
+ verifier.setAlgorithm("SHA-512");
+ Assert.assertFalse(verifier.matches(PWD, tamperedCredential));
+ }
+
+
private void doTest(String digest, int saltLength, int iterations) throws NoSuchAlgorithmException {
MessageDigestCredentialHandler mdch = new MessageDigestCredentialHandler();
MessageDigestCredentialHandler verifier = new MessageDigestCredentialHandler();
diff --git a/test/org/apache/tomcat/util/security/TestConcurrentMessageDigest.java b/test/org/apache/tomcat/util/security/TestConcurrentMessageDigest.java
index 2ce4c136c9..b92c749b5b 100644
--- a/test/org/apache/tomcat/util/security/TestConcurrentMessageDigest.java
+++ b/test/org/apache/tomcat/util/security/TestConcurrentMessageDigest.java
@@ -1,21 +1,22 @@
/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
*/
package org.apache.tomcat.util.security;
+import org.junit.Assert;
import org.junit.Test;
public class TestConcurrentMessageDigest {
@@ -37,4 +38,51 @@ public class TestConcurrentMessageDigest {
ConcurrentMessageDigest.init(SHA256);
ConcurrentMessageDigest.digest(SHA256, new byte[8]);
}
+
+
+ /*
+ * A zero iteration count is not a valid number of times to apply the digest. It must fail rather than silently
+ * applying the digest once.
+ */
+ @Test
+ public void testZeroIterations() throws Exception {
+ ConcurrentMessageDigest.init(SHA256);
+
+ Assert.assertThrows(IllegalArgumentException.class,
+ () -> ConcurrentMessageDigest.digest(SHA256, 0, new byte[8]));
+ }
+
+
+ /*
+ * A negative iteration count is not a valid number of times to apply the digest. It must fail rather than
+ * silently applying the digest once.
+ */
+ @Test
+ public void testNegativeIterations() throws Exception {
+ ConcurrentMessageDigest.init(SHA256);
+
+ Assert.assertThrows(IllegalArgumentException.class,
+ () -> ConcurrentMessageDigest.digest(SHA256, -5, new byte[8]));
+ }
+
+
+ /*
+ * Multiple iterations must repeatedly digest the result of the previous iteration.
+ */
+ @Test
+ public void testMultipleIterations() throws Exception {
+ ConcurrentMessageDigest.init(SHA256);
+
+ byte[] input = "input".getBytes("UTF-8");
+
+ byte[] twice = ConcurrentMessageDigest.digest(SHA256, 2, input);
+ byte[] expected = ConcurrentMessageDigest.digest(SHA256, ConcurrentMessageDigest.digest(SHA256, input));
+
+ Assert.assertArrayEquals(expected, twice);
+
+ byte[] threeTimes = ConcurrentMessageDigest.digest(SHA256, 3, input);
+ expected = ConcurrentMessageDigest.digest(SHA256, expected);
+
+ Assert.assertArrayEquals(expected, threeTimes);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 04/23: Exclude unimplemented SSLv2 ciphers when converting OpenSSL cipher names to JSSE names
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 4e813accaa7dfc4593124cd82490f960db97905c
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 08:54:06 2026 +0200
Exclude unimplemented SSLv2 ciphers when converting OpenSSL cipher names
to JSSE names
openSSLToJsse() resolved an OpenSSL cipher name only when its alias
mapped to exactly one Cipher constant, and returned that constant's
first JSSE name otherwise. Two consequences of including the
unimplemented SSLv2 constants of the Cipher enumeration in that
resolution:
- Genuine OpenSSL names shared with an SSLv2 constant (RC4-MD5,
EXP-RC4-MD5, EXP-RC2-CBC-MD5) were ambiguous and resolved to
null. The JNI SSLEngine implementation then added that null to the
set returned by getSupportedCipherSuites(), violating the SSLEngine
contract, and the Panama findCiphers() method could add null to the
list later parsed by the cipher configuration parser, causing a null
pointer exception when a PROFILE= expression resolved to one of these
ciphers.
- Aliases matching only an SSLv2 constant (RC2-CBC-MD5,
DES-CBC3-MD5, the SSL_CK_*/SSL2_* constants) resolved to
non-standard pseudo JSSE names derived from the enumeration.
The conversion now excludes SSLv2 ciphers uniformly. Names shared with
an SSLv2 constant resolve to the JSSE name of the modern cipher, names
matching only SSLv2 ciphers, genuinely ambiguous names and unknown
names resolve to null, and the two engine call sites that previously
could propagate null now filter unmapped names.
---
.../tomcat/util/net/openssl/OpenSSLEngine.java | 7 +++++-
.../ciphers/OpenSSLCipherConfigurationParser.java | 29 ++++++++++++++++++----
.../util/net/openssl/panama/OpenSSLLibrary.java | 7 +++++-
.../util/net/openssl/ciphers/TestCipher.java | 27 ++++++++++++++++++++
webapps/docs/changelog.xml | 11 ++++++++
5 files changed, 74 insertions(+), 7 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java
index fa75c1b43f..4fc7a1a56a 100644
--- a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java
+++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java
@@ -90,7 +90,12 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
if (c == null || c.isEmpty() || availableCipherSuites.contains(c)) {
continue;
}
- availableCipherSuites.add(OpenSSLCipherConfigurationParser.openSSLToJsse(c));
+ String jsseName = OpenSSLCipherConfigurationParser.openSSLToJsse(c);
+ // Ignore ciphers with no JSSE equivalent.
+ if (jsseName == null) {
+ continue;
+ }
+ availableCipherSuites.add(jsseName);
}
} finally {
SSL.freeSSL(ssl);
diff --git a/java/org/apache/tomcat/util/net/openssl/ciphers/OpenSSLCipherConfigurationParser.java b/java/org/apache/tomcat/util/net/openssl/ciphers/OpenSSLCipherConfigurationParser.java
index 157d716a47..cfeb7bced3 100644
--- a/java/org/apache/tomcat/util/net/openssl/ciphers/OpenSSLCipherConfigurationParser.java
+++ b/java/org/apache/tomcat/util/net/openssl/ciphers/OpenSSLCipherConfigurationParser.java
@@ -898,21 +898,40 @@ public class OpenSSLCipherConfigurationParser {
*
* @param opensslCipherName The OpenSSL name for a cipher
*
- * @return The first JSSE name for the specified OpenSSL cipher, or {@code null} if the name is not a known,
- * unambiguous OpenSSL cipher name
+ * @return The first JSSE name for the specified OpenSSL cipher, or {@code null} if the name is not a known
+ * OpenSSL cipher name, if it remains ambiguous once unimplemented SSLv2 ciphers are excluded or if
+ * only SSLv2 ciphers match
*/
public static String openSSLToJsse(String opensslCipherName) {
if (!initialized) {
init();
}
List<Cipher> ciphers = aliases.get(opensslCipherName);
- if (ciphers == null || ciphers.size() != 1) {
+ if (ciphers == null) {
// Not an OpenSSL cipher name
return null;
}
- Cipher cipher = ciphers.get(0);
+ // OpenSSL has not supported SSLv2 for many years and the SSLv2
+ // ciphers here are unimplemented. Excluding them resolves the
+ // ambiguity for the OpenSSL names that an SSLv2 cipher shares with
+ // the equivalent modern cipher.
+ Cipher candidate = null;
+ for (Cipher cipher : ciphers) {
+ if (cipher.getProtocol() == Protocol.SSLv2) {
+ continue;
+ }
+ if (candidate != null) {
+ // Ambiguous name
+ return null;
+ }
+ candidate = cipher;
+ }
+ if (candidate == null) {
+ // Only SSLv2 ciphers matched
+ return null;
+ }
// Each Cipher always has at least one JSSE name
- return cipher.getJsseNames().iterator().next();
+ return candidate.getJsseNames().iterator().next();
}
diff --git a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java
index b8691992d2..cac10c2a89 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java
@@ -456,7 +456,12 @@ public class OpenSSLLibrary {
if (c == null || c.isEmpty() || ciphersList.contains(c)) {
continue;
}
- ciphersList.add(OpenSSLCipherConfigurationParser.openSSLToJsse(c));
+ String jsseName = OpenSSLCipherConfigurationParser.openSSLToJsse(c);
+ // Ignore ciphers with no JSSE equivalent.
+ if (jsseName == null) {
+ continue;
+ }
+ ciphersList.add(jsseName);
}
} finally {
SSL_free(ssl);
diff --git a/test/org/apache/tomcat/util/net/openssl/ciphers/TestCipher.java b/test/org/apache/tomcat/util/net/openssl/ciphers/TestCipher.java
index 46309bf581..3f224bacef 100644
--- a/test/org/apache/tomcat/util/net/openssl/ciphers/TestCipher.java
+++ b/test/org/apache/tomcat/util/net/openssl/ciphers/TestCipher.java
@@ -116,6 +116,33 @@ public class TestCipher {
}
+ /**
+ * Check that OpenSSL aliases that are shared with unimplemented SSLv2
+ * ciphers resolve to the JSSE name of the modern cipher, while genuinely
+ * ambiguous aliases, aliases that only match SSLv2 ciphers and unknown
+ * names resolve to null.
+ */
+ @Test
+ public void testOpenSSLToJsseAmbiguousNames() {
+ Assert.assertEquals("SSL_RSA_WITH_RC4_128_MD5",
+ OpenSSLCipherConfigurationParser.openSSLToJsse("RC4-MD5"));
+ Assert.assertEquals("SSL_RSA_EXPORT_WITH_RC4_40_MD5",
+ OpenSSLCipherConfigurationParser.openSSLToJsse("EXP-RC4-MD5"));
+ Assert.assertEquals("SSL_RSA_EXPORT_WITH_RC2_CBC_40_MD5",
+ OpenSSLCipherConfigurationParser.openSSLToJsse("EXP-RC2-CBC-MD5"));
+ // Unambiguous name
+ Assert.assertEquals("TLS_RSA_WITH_AES_128_CBC_SHA",
+ OpenSSLCipherConfigurationParser.openSSLToJsse("AES128-SHA"));
+ // Unknown name
+ Assert.assertNull(OpenSSLCipherConfigurationParser.openSSLToJsse("Nonsense"));
+ // Alias shared by more than one modern cipher
+ Assert.assertNull(OpenSSLCipherConfigurationParser.openSSLToJsse("SHA1"));
+ // Aliases that only match an unimplemented SSLv2 cipher
+ Assert.assertNull(OpenSSLCipherConfigurationParser.openSSLToJsse("RC2-CBC-MD5"));
+ Assert.assertNull(OpenSSLCipherConfigurationParser.openSSLToJsse("SSL_CK_RC4_128_WITH_MD5"));
+ }
+
+
/**
* Check that the elements of the Cipher enumeration are all using standard
* names from the TLS registry or are known exceptions.
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index e0d3739586..1dcad89624 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -351,6 +351,17 @@
using only the known terms. This matches OpenSSL behavior, where such
an expression results in no cipher match. (remm)
</fix>
+ <fix>
+ In the OpenSSL cipher name conversion, ignore the unimplemented SSLv2
+ ciphers so that names shared with a modern cipher, for example
+ <code>RC4-MD5</code>, resolve to the JSSE name of the modern cipher
+ rather than returning <code>null</code>, and so that names matching
+ only an SSLv2 cipher no longer resolve to a non-standard name.
+ Ambiguous and unknown names still resolve to <code>null</code>; the
+ OpenSSL engine implementations now filter such unmapped names rather
+ than adding <code>null</code> to the lists of cipher suites they
+ expose. (remm)
+ </fix>
</changelog>
</subsection>
<subsection name="Web applications">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 12/23: Harmonize resource bundle processing in ProxyErrorReportValve
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 2e3743f17b063c88c646d6c13120997d3954d001
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 11:55:30 2026 +0200
Harmonize resource bundle processing in ProxyErrorReportValve
The redirect url lookup of ProxyErrorReportValve passed the response
locale straight to ResourceBundle.getBundle(). The StringManager locale
processing introduced by commit c45c9b47f3 handles this differently: a
bundle is only loaded for a locale for which a bundle actually exists,
checked with a cache free probe.
Align the valve with that processing. Resolve the response locale
through the candidate locales of the default control, check each
candidate with StringManager.bundleExists() and only then load the
bundle for the first matching candidate. Locales that match no
candidate fall back to the default error report as before. To allow the
reuse, make bundleExists() in StringManager accessible outside the
class. For the documented layout, a properties file at the root of the
class path, the resolution is unchanged for every locale.
---
.../catalina/valves/ProxyErrorReportValve.java | 23 +++++++++++++++++++++-
java/org/apache/tomcat/util/res/StringManager.java | 2 +-
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/catalina/valves/ProxyErrorReportValve.java b/java/org/apache/catalina/valves/ProxyErrorReportValve.java
index 14b891834a..ed40596203 100644
--- a/java/org/apache/catalina/valves/ProxyErrorReportValve.java
+++ b/java/org/apache/catalina/valves/ProxyErrorReportValve.java
@@ -25,6 +25,7 @@ import java.net.URISyntaxException;
import java.net.URL;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
+import java.util.Locale;
import java.util.MissingResourceException;
import java.util.ResourceBundle;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -57,6 +58,13 @@ public class ProxyErrorReportValve extends ErrorReportValve {
private static final int PROXY_CONNECT_TIMEOUT_MS = 5000;
private static final int PROXY_READ_TIMEOUT_MS = 10000;
+ /*
+ * Control used to resolve the candidate locales of the optional properties file bundle. The default control is the
+ * one getBundle() uses, so the candidate chain matches the one getBundle() would walk.
+ */
+ private static final ResourceBundle.Control BUNDLE_CONTROL =
+ ResourceBundle.Control.getControl(ResourceBundle.Control.FORMAT_PROPERTIES);
+
/**
* Default constructor.
*/
@@ -111,9 +119,22 @@ public class ProxyErrorReportValve extends ErrorReportValve {
}
private String getRedirectUrl(Response response) {
+ String bundleName = this.getClass().getSimpleName();
+ // Resolve the bundle for the first candidate locale for which a bundle exists.
+ Locale bundleLocale = null;
+ for (Locale candidate : BUNDLE_CONTROL.getCandidateLocales(bundleName, response.getLocale())) {
+ if (StringManager.bundleExists(bundleName, candidate)) {
+ bundleLocale = candidate;
+ break;
+ }
+ }
+ if (bundleLocale == null) {
+ // No bundle available. Fall back to the default error report.
+ return null;
+ }
ResourceBundle resourceBundle;
try {
- resourceBundle = ResourceBundle.getBundle(this.getClass().getSimpleName(), response.getLocale());
+ resourceBundle = ResourceBundle.getBundle(bundleName, bundleLocale);
} catch (MissingResourceException e) {
// No bundle available. Fall back to the default error report.
return null;
diff --git a/java/org/apache/tomcat/util/res/StringManager.java b/java/org/apache/tomcat/util/res/StringManager.java
index 7724748ee2..e5a7ee52f6 100644
--- a/java/org/apache/tomcat/util/res/StringManager.java
+++ b/java/org/apache/tomcat/util/res/StringManager.java
@@ -302,7 +302,7 @@ public class StringManager {
*
* @return true if a bundle exists for the given locale
*/
- private static boolean bundleExists(String bundleName, Locale locale) {
+ public static boolean bundleExists(String bundleName, Locale locale) {
ClassLoader classLoader = StringManager.class.getClassLoader();
if (bundleExists(bundleName, locale, classLoader)) {
return true;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 15/23: Ignore extra close() calls in ReferenceCountedJar
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 959efc91f2093e92a6759ef5df69236d443a6579
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 13:22:07 2026 +0200
Ignore extra close() calls in ReferenceCountedJar
An extra close() call (more closes than opens) drove the reference count
below zero. Since the wrapped Jar is only closed when a balanced close
returns the count to exactly zero, once the count went negative no
subsequent open()/close() cycle would close the wrapped Jar again,
leaking it and its file handle until the garbage collector ran the
cleaner. Extra close() calls are now ignored, saturating the count at
zero. Current callers all balance their references, so this hardens the
public API rather than fixing an active leak.
---
.../tomcat/util/scan/ReferenceCountedJar.java | 6 +++++
.../tomcat/util/scan/TestReferenceCountedJar.java | 31 ++++++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/java/org/apache/tomcat/util/scan/ReferenceCountedJar.java b/java/org/apache/tomcat/util/scan/ReferenceCountedJar.java
index 83f3dc1bce..6330ae3c24 100644
--- a/java/org/apache/tomcat/util/scan/ReferenceCountedJar.java
+++ b/java/org/apache/tomcat/util/scan/ReferenceCountedJar.java
@@ -61,6 +61,12 @@ public class ReferenceCountedJar implements Jar {
@Override
public synchronized void close() {
+ if (referenceCount <= 0) {
+ // More closes than opens have been requested. Ignore the extra
+ // close rather than letting the count go negative, which would
+ // prevent the wrapped Jar from ever being closed again.
+ return;
+ }
referenceCount--;
if (referenceCount == 0) {
wrappedJar.close();
diff --git a/test/org/apache/tomcat/util/scan/TestReferenceCountedJar.java b/test/org/apache/tomcat/util/scan/TestReferenceCountedJar.java
index 77a6c8a117..33775f1815 100644
--- a/test/org/apache/tomcat/util/scan/TestReferenceCountedJar.java
+++ b/test/org/apache/tomcat/util/scan/TestReferenceCountedJar.java
@@ -21,6 +21,7 @@ import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.lang.reflect.Field;
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.util.jar.Attributes;
@@ -179,4 +180,34 @@ public class TestReferenceCountedJar {
Assert.assertEquals(CONTENT, readFully(is));
}
}
+
+
+ /*
+ * Extra close() calls (more closes than opens) must not drive the reference count below zero. If the count were to
+ * go negative, subsequent open()/close() cycles would never return it to zero and the wrapped Jar would never be
+ * closed again, leaking it.
+ */
+ @Test
+ public void testExtraCloseDoesNotGoNegative() throws Exception {
+ File file = createJar();
+ URI jarUri = URI.create("jar:" + file.toURI().toURL().toExternalForm() + "!/");
+
+ ReferenceCountedJar jar = new ReferenceCountedJar(jarUri.toURL());
+ Field countField = ReferenceCountedJar.class.getDeclaredField("referenceCount");
+ countField.setAccessible(true);
+
+ jar.close();
+ Assert.assertEquals(0, countField.getInt(jar));
+
+ // Extra closes must be ignored rather than decrementing below zero.
+ jar.close();
+ jar.close();
+ Assert.assertEquals(0, countField.getInt(jar));
+
+ // A later open()/close() cycle must still be able to return to zero and release the wrapped Jar.
+ try (InputStream is = jar.getInputStream(ENTRY_NAME)) {
+ Assert.assertEquals(CONTENT, readFully(is));
+ }
+ Assert.assertEquals(0, countField.getInt(jar));
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 11/23: Bind the nonce value parameter of OCSP_request_add1_nonce as a pointer
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 5aa34339b8bc6646cbbb25024776fd8b04dd4bb5
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 11:28:13 2026 +0200
Bind the nonce value parameter of OCSP_request_add1_nonce as a pointer
The C prototype is int OCSP_request_add1_nonce(OCSP_REQUEST *req,
unsigned char *val, int len) but the FFM binding declared the second
parameter with the JAVA_CHAR layout and the wrapper method accepted a
Java char. A scalar is placed where the native code reads a pointer. The
only call site passed (char) 0 which currently works out to a NULL
pointer through zero-extension, but the binding was formally incorrect,
a non-zero value would have produced an invalid pointer for OpenSSL to
copy from, and the documented use of passing an explicit nonce value was
impossible with this signature.
Bind the parameter as a pointer, accept a MemorySegment in the wrapper
and pass MemorySegment.NULL at the call site, as intended for generating
a random nonce. The previous incorrect layout stemmed from hand-editing
of the generated file; the corrected binding is what jextract itself
generates for this prototype. No behavior change on the existing call
path.
---
java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java | 2 +-
java/org/apache/tomcat/util/openssl/openssl_h.java | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
index 96ab226282..1b9ebbe642 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
@@ -1408,7 +1408,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
X509_STORE_CTX_set_error(x509ctx, X509_V_ERR_UNABLE_TO_GET_CRL());
return V_OCSP_CERTSTATUS_UNKNOWN();
}
- OCSP_request_add1_nonce(ocspRequest, (char) 0, -1);
+ OCSP_request_add1_nonce(ocspRequest, MemorySegment.NULL, -1);
MemorySegment ocspRequestDerPointer = localArena.allocateFrom(ValueLayout.ADDRESS, MemorySegment.NULL);
int requestLength = i2d_OCSP_REQUEST(ocspRequest, ocspRequestDerPointer);
if (requestLength <= 0) {
diff --git a/java/org/apache/tomcat/util/openssl/openssl_h.java b/java/org/apache/tomcat/util/openssl/openssl_h.java
index 518bd7fb58..66b1a908c4 100644
--- a/java/org/apache/tomcat/util/openssl/openssl_h.java
+++ b/java/org/apache/tomcat/util/openssl/openssl_h.java
@@ -9132,7 +9132,7 @@ public class openssl_h {
public static final FunctionDescriptor DESC = FunctionDescriptor.of(
openssl_h.C_INT,
openssl_h.C_POINTER,
- ValueLayout.JAVA_CHAR,
+ openssl_h.C_POINTER,
openssl_h.C_INT
);
@@ -9176,7 +9176,7 @@ public class openssl_h {
* int OCSP_request_add1_nonce(OCSP_REQUEST *req, unsigned char *val, int len)
* }
*/
- public static int OCSP_request_add1_nonce(MemorySegment req, char val, int len) {
+ public static int OCSP_request_add1_nonce(MemorySegment req, MemorySegment val, int len) {
var mh$ = OCSP_request_add1_nonce.HANDLE;
try {
if (TRACE_DOWNCALLS) {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 16/23: Resolve nested JAR entries in JarFileUrlNestedJar using the JarURLConnection
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 79826e4a79eff089a3d0f50e2f9289831d55635c
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 13:43:25 2026 +0200
Resolve nested JAR entries in JarFileUrlNestedJar using the JarURLConnection
The constructor previously derived the entry name for the nested JAR by
manually extracting the text after the "!/" separator from the URL
string. Percent-encoding in the entry name is not visible in the URL
string in decoded form, so for URLs that named the entry using percent
encoding (for example a nested JAR whose file name contains a space),
the manual lookup did not find the entry the JDK itself resolves the
URL to. The Jar was then constructed with no entry, and the problem
only surfaced later as a NullPointerException when the content of the
Jar was first accessed.
The entry is now resolved with JarURLConnection.getJarEntry(), which
applies the same decoding the JDK applies when it resolves the URL.
This is the same approach JarFileUrlJar already uses. If the URL does
not name an entry at all (it ends with "!/"), a FileNotFoundException
is now thrown when the Jar is constructed rather than leaving the Jar
in a state that fails unexpectedly later.
Add TestJarFileUrlNestedJar to cover plain and percent-encoded entry
names as well as the missing entry, empty entry name and missing
separator cases.
---
.../tomcat/util/scan/JarFileUrlNestedJar.java | 12 +-
.../tomcat/util/scan/TestJarFileUrlNestedJar.java | 164 +++++++++++++++++++++
2 files changed, 172 insertions(+), 4 deletions(-)
diff --git a/java/org/apache/tomcat/util/scan/JarFileUrlNestedJar.java b/java/org/apache/tomcat/util/scan/JarFileUrlNestedJar.java
index a189aa843a..c0583b5897 100644
--- a/java/org/apache/tomcat/util/scan/JarFileUrlNestedJar.java
+++ b/java/org/apache/tomcat/util/scan/JarFileUrlNestedJar.java
@@ -16,6 +16,7 @@
*/
package org.apache.tomcat.util.scan;
+import java.io.FileNotFoundException;
import java.io.IOException;
import java.net.JarURLConnection;
import java.net.URL;
@@ -43,10 +44,13 @@ public class JarFileUrlNestedJar extends AbstractInputStreamJar {
jarConn.setUseCaches(false);
warFile = jarConn.getJarFile();
- String urlAsString = url.toString();
- int pathStart = urlAsString.indexOf("!/") + 2;
- String jarPath = urlAsString.substring(pathStart);
- jarEntry = warFile.getJarEntry(jarPath);
+ // Use the connection to resolve the entry rather than parsing the URL string manually so that any
+ // percent-encoding in the entry name is handled the same way as the JDK handles it. A missing entry
+ // causes jarConn.getJarFile() above to throw.
+ jarEntry = jarConn.getJarEntry();
+ if (jarEntry == null) {
+ throw new FileNotFoundException(url.toExternalForm());
+ }
}
diff --git a/test/org/apache/tomcat/util/scan/TestJarFileUrlNestedJar.java b/test/org/apache/tomcat/util/scan/TestJarFileUrlNestedJar.java
new file mode 100644
index 0000000000..0eceb7614a
--- /dev/null
+++ b/test/org/apache/tomcat/util/scan/TestJarFileUrlNestedJar.java
@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.util.scan;
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URL;
+import java.nio.charset.StandardCharsets;
+import java.util.jar.Attributes;
+import java.util.jar.JarEntry;
+import java.util.jar.JarOutputStream;
+import java.util.jar.Manifest;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.tomcat.Jar;
+
+public class TestJarFileUrlNestedJar {
+
+ private static final String TEST_ENTRY = "Test.class";
+ private static final byte[] TEST_CONTENT = "test content".getBytes(StandardCharsets.UTF_8);
+
+ /*
+ * Creates a WAR that contains a nested JAR at the given path. The nested JAR contains a single entry named
+ * TEST_ENTRY with the content TEST_CONTENT.
+ */
+ private File createWar(String nestedJarPath) throws IOException {
+ File file = File.createTempFile("nested-", ".war");
+ file.deleteOnExit();
+
+ Manifest manifest = new Manifest();
+ manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0");
+
+ try (FileOutputStream fos = new FileOutputStream(file);
+ JarOutputStream war = new JarOutputStream(fos, manifest)) {
+
+ war.putNextEntry(new JarEntry(nestedJarPath));
+ war.write(createNestedJar());
+ war.closeEntry();
+ }
+
+ return file;
+ }
+
+
+ private static byte[] createNestedJar() throws IOException {
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+
+ Manifest manifest = new Manifest();
+ manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0");
+
+ try (JarOutputStream jar = new JarOutputStream(baos, manifest)) {
+ jar.putNextEntry(new JarEntry(TEST_ENTRY));
+ jar.write(TEST_CONTENT);
+ jar.closeEntry();
+ }
+
+ return baos.toByteArray();
+ }
+
+
+ private static byte[] readJar(Jar jar) throws IOException {
+ try (InputStream is = jar.getInputStream(TEST_ENTRY)) {
+ ByteArrayOutputStream buffer = new ByteArrayOutputStream();
+ byte[] data = new byte[1024];
+ int len;
+ while ((len = is.read(data)) > 0) {
+ buffer.write(data, 0, len);
+ }
+ return buffer.toByteArray();
+ }
+ }
+
+
+ /*
+ * A nested JAR referenced with a plain entry name must be readable.
+ */
+ @Test
+ public void testPlainEntryName() throws Exception {
+ String nestedPath = "WEB-INF/lib/nested.jar";
+ File war = createWar(nestedPath);
+
+ try (Jar jar = new JarFileUrlNestedJar(new URL("jar:file:" + war.getAbsolutePath() + "!/" + nestedPath))) {
+ Assert.assertTrue(jar.exists(TEST_ENTRY));
+ Assert.assertArrayEquals(TEST_CONTENT, readJar(jar));
+ }
+ }
+
+
+ /*
+ * A percent-encoded entry name must resolve to the same entry that the JDK resolves it to, rather than
+ * resulting in a JAR with no usable content.
+ */
+ @Test
+ public void testEncodedEntryName() throws Exception {
+ File war = createWar("WEB-INF/lib/my jar.jar");
+
+ try (Jar jar = new JarFileUrlNestedJar(
+ new URL("jar:file:" + war.getAbsolutePath() + "!/WEB-INF/lib/my%20jar.jar"))) {
+ Assert.assertTrue(jar.exists(TEST_ENTRY));
+ Assert.assertArrayEquals(TEST_CONTENT, readJar(jar));
+ }
+ }
+
+
+ /*
+ * A URL that does not refer to an existing entry must fail when the JAR is constructed rather than later when
+ * the content is accessed.
+ */
+ @Test
+ public void testMissingEntryThrows() throws Exception {
+ File war = createWar("WEB-INF/lib/nested.jar");
+
+ Assert.assertThrows(IOException.class, () -> new JarFileUrlNestedJar(
+ new URL("jar:file:" + war.getAbsolutePath() + "!/WEB-INF/lib/missing.jar")));
+ }
+
+
+ /*
+ * A URL that does not name an entry (it refers to the whole archive) must fail when the JAR is constructed
+ * rather than later when the content is accessed. JarFactory routes such URLs elsewhere. This covers direct
+ * construction of JarFileUrlNestedJar with such a URL.
+ */
+ @Test
+ public void testEmptyEntryNameThrows() throws Exception {
+ File war = createWar("WEB-INF/lib/nested.jar");
+
+ Assert.assertThrows(FileNotFoundException.class,
+ () -> new JarFileUrlNestedJar(new URL("jar:file:" + war.getAbsolutePath() + "!/")));
+ }
+
+
+ /*
+ * A URL that does not contain the JAR entry separator must fail with an IOException before the JAR can be used.
+ * In practice the JDK's jar URL handler rejects such a spec ("no !/ in spec") with a MalformedURLException when
+ * the URL is created, so a URL of this form never reaches JarFileUrlNestedJar via JarFactory.
+ */
+ @Test
+ public void testMissingSeparatorThrows() throws Exception {
+ File war = createWar("WEB-INF/lib/nested.jar");
+
+ Assert.assertThrows(IOException.class,
+ () -> new JarFileUrlNestedJar(new URL("jar:file:" + war.getAbsolutePath() + "!x")));
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 07/23: Validate the protocol of OCSP URLs from a client certificate AIA extension
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit e5cb820aaa2766417efeccf13414806f65c59349
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 10:43:03 2026 +0200
Validate the protocol of OCSP URLs from a client certificate AIA extension
The FFM OpenSSL implementation accepted any protocol that has a URL
stream handler registered in the JVM (for example file: or jar:) when
building OCSP requests from the URLs found in the OCSP No Check AIA
extension of a client certificate. Setting requestAttempted to true
before the URLConnection was cast to HttpURLConnection meant that the
resulting ClassCastException was reported as a non-soft-failable
X509_V_ERR_APPLICATION_VERIFICATION error, causing the handshake to
fail even when ocspSoftFail was enabled (the default).
Now the protocol is checked before a request is attempted. A URL with
a protocol other than http or https is logged as invalid and skipped,
matching the handling of syntactically invalid URLs at the same place
and the behaviour of the JNI/tomcat-native implementation, where
OSSL_HTTP_parse_url() rejects non-HTTP URLs and no request is made.
The other elements of the original report were rejected: the DER
length claims used by the OCSP URL parser are bounded by
Asn1Parser.parseLength() against the real remaining data before any
array is allocated, so no oversized allocation is possible; and
bounding the number of responder URLs or the aggregate wait time would
change the behaviour of the opt-in OCSP feature beyond what the JNI
implementation offers for an exposure limited to certificates signed
by trusted CAs.
---
.../org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
index 84e13bc83f..da255ab0b2 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
@@ -1277,6 +1277,15 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
for (String urlString : urls) {
try {
URL url = (new URI(urlString)).toURL();
+ String protocol = url.getProtocol();
+ if (!"http".equals(protocol) && !"https".equals(protocol)) {
+ // OpenSSLEngine can only talk HTTP/HTTPS to OCSP responders. Other
+ // protocol handlers (e.g. file) trigger a ClassCastException below so
+ // treat them as invalid and skip to the next URL, as the JNI
+ // implementation does when OCSP URL parsing fails.
+ log.warn(sm.getString("engine.invalidOCSPURL", urlString));
+ continue;
+ }
requestAttempted = true;
ocspResponse =
processOCSPRequest(state, url, issuer, x509, x509ctx, localArena);
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 03/23: Match OpenSSL semantics for AND cipher expressions with unknown terms
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 62b4792a4cf0c414dc96d0ab6fde9efdd6812013
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 08:32:57 2026 +0200
Match OpenSSL semantics for AND cipher expressions with unknown terms
In the OpenSSL cipher configuration parser, an AND expression (X+Y+Z)
that contains an unknown term is only evaluated when every term is a
known alias. OpenSSL rejects such an element entirely (the equivalent
cipher list string produces no cipher match).
Previously, the Java parser only required the first term to be a known
alias and silently skipped any subsequent unknown term. This made the
result position dependent and silently widened the configured cipher
set, e.g. parse("SHA1+Nonsense") returned the full SHA1 set while
parse("Nonsense+SHA1") returned no ciphers at all. A typo in the
second or later term of an AND expression therefore loosened the
configured cipher set with no indication in the logs.
The parser now logs the unknown element as a warning and discards the
whole element, matching OpenSSL behavior regardless of the position of
the unknown term.
TestCipher builds synthetic NAME+PROTOCOL tokens from the columns of
openssl ciphers -v output for its mapping checks. The SSLv3 protocol
suffix is no longer a keyword the parser supports (removed earlier to
align with the OpenSSL development branch) and previously relied on the
lenient AND handling. The protocol suffix is informational for these
tokens, so the test now strips a trailing +SSLv3 suffix before parsing
while continuing to use the full token for the unmapped-cipher
bookkeeping. A new test asserts an AND expression with an unknown term
parses to an empty set for both term orderings.
---
.../ciphers/OpenSSLCipherConfigurationParser.java | 21 +++++++++++++++------
.../tomcat/util/net/openssl/ciphers/TestCipher.java | 11 ++++++++++-
.../TestOpenSSLCipherConfigurationParser.java | 10 ++++++++++
webapps/docs/changelog.xml | 7 +++++++
4 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/openssl/ciphers/OpenSSLCipherConfigurationParser.java b/java/org/apache/tomcat/util/net/openssl/ciphers/OpenSSLCipherConfigurationParser.java
index 0f85779ea9..157d716a47 100644
--- a/java/org/apache/tomcat/util/net/openssl/ciphers/OpenSSLCipherConfigurationParser.java
+++ b/java/org/apache/tomcat/util/net/openssl/ciphers/OpenSSLCipherConfigurationParser.java
@@ -810,13 +810,22 @@ public class OpenSSLCipherConfigurationParser {
add(ciphers, element, orderedAllAddedCiphers);
} else if (element.contains(AND)) {
String[] intersections = element.split("\\" + AND);
- if (intersections.length > 0 && aliases.containsKey(intersections[0])) {
- List<Cipher> result = new ArrayList<>(aliases.get(intersections[0]));
- for (int i = 1; i < intersections.length; i++) {
- if (aliases.containsKey(intersections[i])) {
- result.retainAll(aliases.get(intersections[i]));
- }
+ List<Cipher> result = null;
+ for (String intersection : intersections) {
+ // As in OpenSSL, any unknown term makes the whole AND
+ // element match nothing
+ if (!aliases.containsKey(intersection)) {
+ log.warn(sm.getString("opensslCipherConfigurationParser.unknownElement", intersection));
+ result = null;
+ break;
}
+ if (result == null) {
+ result = new ArrayList<>(aliases.get(intersection));
+ } else {
+ result.retainAll(aliases.get(intersection));
+ }
+ }
+ if (result != null) {
add(ciphers, result, orderedAllAddedCiphers);
}
}
diff --git a/test/org/apache/tomcat/util/net/openssl/ciphers/TestCipher.java b/test/org/apache/tomcat/util/net/openssl/ciphers/TestCipher.java
index 4753a4814e..46309bf581 100644
--- a/test/org/apache/tomcat/util/net/openssl/ciphers/TestCipher.java
+++ b/test/org/apache/tomcat/util/net/openssl/ciphers/TestCipher.java
@@ -39,8 +39,17 @@ public class TestCipher {
StringBuilder errors = new StringBuilder();
for (String openSSLCipherSuite : openSSLCipherSuites) {
+ // The protocol suffix identifies the version OpenSSL reports as
+ // the minimum protocol for the cipher suite. SSLv3 is no longer a
+ // supported alias in cipher configuration strings (removed from
+ // OpenSSL and from this parser) so strip that suffix before
+ // parsing. The suffix is not used to determine the mapping.
+ String specification = openSSLCipherSuite;
+ if (specification.endsWith("+SSLv3")) {
+ specification = specification.substring(0, specification.length() - "+SSLv3".length());
+ }
List<String> jsseCipherSuites =
- OpenSSLCipherConfigurationParser.parseExpression(openSSLCipherSuite);
+ OpenSSLCipherConfigurationParser.parseExpression(specification);
for (JsseImpl jsseImpl : JSSE_IMPLS) {
boolean found = false;
diff --git a/test/org/apache/tomcat/util/net/openssl/ciphers/TestOpenSSLCipherConfigurationParser.java b/test/org/apache/tomcat/util/net/openssl/ciphers/TestOpenSSLCipherConfigurationParser.java
index a01e8ba384..7f8cf44f65 100644
--- a/test/org/apache/tomcat/util/net/openssl/ciphers/TestOpenSSLCipherConfigurationParser.java
+++ b/test/org/apache/tomcat/util/net/openssl/ciphers/TestOpenSSLCipherConfigurationParser.java
@@ -537,6 +537,16 @@ public class TestOpenSSLCipherConfigurationParser {
}
+ @Test
+ public void testIntersectionWithUnknownTermIsEmpty() throws Exception {
+ // OpenSSL treats an AND element that contains an unknown term as
+ // matching no ciphers, regardless of the position of the unknown term
+ testSpecificationIsEmpty("SHA1+Nonsense");
+ Assert.assertEquals(0, OpenSSLCipherConfigurationParser.parse("SHA1+Nonsense").size());
+ Assert.assertEquals(0, OpenSSLCipherConfigurationParser.parse("Nonsense+SHA1").size());
+ }
+
+
private void testSpecificationIsEmpty(String specification) throws Exception {
String openSSLCipherList = TesterOpenSSL.getOpenSSLCiphersAsExpression(specification);
Assert.assertEquals(TesterOpenSSL.VERSION_STRING + " with specification [" + specification + "] returned [" +
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index a436abc9ae..e0d3739586 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -344,6 +344,13 @@
drop TLS 1.3 when rebuilding the list of enabled protocols during
initialization. (dsoumis)
</fix>
+ <fix>
+ In the OpenSSL cipher configuration parser, treat an AND expression
+ (<code>+</code>) that contains an unknown term as matching no ciphers
+ and log a warning, rather than silently ignoring the unknown term and
+ using only the known terms. This matches OpenSSL behavior, where such
+ an expression results in no cipher match. (remm)
+ </fix>
</changelog>
</subsection>
<subsection name="Web applications">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 10/23: Correct the OpenSSL 3 version boundary in the FFM compatibility layer
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit fcea01e031d4626cab47f3f23311dc1e3a8b9257
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 11:14:59 2026 +0200
Correct the OpenSSL 3 version boundary in the FFM compatibility layer
OpenSSL 3.0.0 (and its pre-releases) report a version number of
0x30000000L, since every release of the 3.x and 4.x series uses a zero
status nibble in the synthesised 0xMNN00PP0L version number. The
compatibility code used a boundary of 0x3000000fL, a value based on the
OpenSSL 1.x convention of released versions ending in 0xf, so exactly
OpenSSL 3.0.0 was classified as an OpenSSL 1.x library.
On such a system the FIPS handling called FIPS_mode(), a symbol no
OpenSSL 3.x library exports, failing library initialisation with an
UnsatisfiedLinkError; the same applied to FIPS_mode_set() during
destroy(). The peer certificate retrieval used SSL_get_peer_certificate,
which is not an exported symbol in 3.x, so retrieving client
certificates failed as well. Engines were also set up and the
implementation was reported as OpenSSL 1.x.
Use 0x30000000L as the boundary between the 1.x and 3.x behaviour at all
three places the constant appeared. The affected code branches are the
ones every OpenSSL version from 3.0.1 onwards already uses, and all
symbols those branches reference are available in OpenSSL 3.0.0. No
other version falls between the old and the new boundary.
---
java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java | 4 ++--
java/org/apache/tomcat/util/openssl/openssl_h_Compatibility.java | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java
index c969385848..30d03d3611 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java
@@ -83,7 +83,7 @@ public class OpenSSLLibrary {
if (OpenSSLStatus.isLibraryInitialized()) {
return;
}
- long initParam = (OpenSSL_version_num() >= 0x3000000fL) ? 0 : OPENSSL_INIT_ENGINE_ALL_BUILTIN();
+ long initParam = (OpenSSL_version_num() >= 0x30000000L) ? 0 : OPENSSL_INIT_ENGINE_ALL_BUILTIN();
OPENSSL_init_ssl(initParam, MemorySegment.NULL);
OpenSSLStatus.setLibraryInitialized(true);
}
@@ -362,7 +362,7 @@ public class OpenSSLLibrary {
OpenSSLStatus.setAvailable(false);
try {
- if (OpenSSL_version_num() < 0x3000000fL) {
+ if (OpenSSL_version_num() < 0x30000000L) {
// There could be unreferenced SSL_CTX still waiting for GC
System.gc();
freeDHParameters();
diff --git a/java/org/apache/tomcat/util/openssl/openssl_h_Compatibility.java b/java/org/apache/tomcat/util/openssl/openssl_h_Compatibility.java
index dfb231836d..fc0a9c8689 100644
--- a/java/org/apache/tomcat/util/openssl/openssl_h_Compatibility.java
+++ b/java/org/apache/tomcat/util/openssl/openssl_h_Compatibility.java
@@ -41,8 +41,8 @@ public class openssl_h_Compatibility {
static {
String versionString = OpenSSL_version(0).getString(0);
OPENSSL = versionString.contains("OpenSSL");
- OPENSSL1 = OPENSSL && OpenSSL_version_num() < 0x3000000fL;
- OPENSSL3 = OPENSSL && OpenSSL_version_num() >= 0x3000000fL;
+ OPENSSL1 = OPENSSL && OpenSSL_version_num() < 0x30000000L;
+ OPENSSL3 = OPENSSL && OpenSSL_version_num() >= 0x30000000L;
BORINGSSL = versionString.contains("BoringSSL");
LIBRESSL = versionString.contains("LibreSSL");
int majorVersion = 0;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 14/23: Keep streams returned from ReferenceCountedJar open
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 679b7107fd0e0dd068026d9863cb4549b6418bee
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 13:16:24 2026 +0200
Keep streams returned from ReferenceCountedJar open
The stream-returning methods released the reference they opened before
the caller could use the returned stream, so when no other reference was
held the wrapped JAR was closed and the stream could no longer be read.
They now transfer the opened reference to the returned stream: it is
released when the stream is closed, exactly once, so the wrapped JAR
stays open for the lifetime of the stream. This matches the Jar contract
under which the caller is responsible for closing streams. A null stream
and failed look-ups release the reference immediately.
---
.../tomcat/util/scan/ReferenceCountedJar.java | 48 +++++-
.../tomcat/util/scan/TestReferenceCountedJar.java | 182 +++++++++++++++++++++
2 files changed, 226 insertions(+), 4 deletions(-)
diff --git a/java/org/apache/tomcat/util/scan/ReferenceCountedJar.java b/java/org/apache/tomcat/util/scan/ReferenceCountedJar.java
index a774104af9..83f3dc1bce 100644
--- a/java/org/apache/tomcat/util/scan/ReferenceCountedJar.java
+++ b/java/org/apache/tomcat/util/scan/ReferenceCountedJar.java
@@ -16,6 +16,7 @@
*/
package org.apache.tomcat.util.scan;
+import java.io.FilterInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
@@ -68,6 +69,37 @@ public class ReferenceCountedJar implements Jar {
}
+ /*
+ * Transfers the reference obtained by open() to the given stream obtained from the wrapped Jar. The reference is
+ * released when - and only when - the stream is closed, so the wrapped Jar remains open while the stream is in
+ * use. A null stream releases the reference immediately.
+ */
+ private InputStream retainReference(InputStream is) {
+ if (is == null) {
+ close();
+ return null;
+ }
+
+ return new FilterInputStream(is) {
+
+ private boolean released = false;
+
+ @Override
+ public synchronized void close() throws IOException {
+ if (released) {
+ return;
+ }
+ released = true;
+ try {
+ in.close();
+ } finally {
+ ReferenceCountedJar.this.close();
+ }
+ }
+ };
+ }
+
+
@Override
public URL getJarFileURL() {
return url;
@@ -76,8 +108,12 @@ public class ReferenceCountedJar implements Jar {
@Override
public InputStream getInputStream(String name) throws IOException {
- try (ReferenceCountedJar jar = open()) {
- return jar.wrappedJar.getInputStream(name);
+ ReferenceCountedJar jar = open();
+ try {
+ return jar.retainReference(jar.wrappedJar.getInputStream(name));
+ } catch (IOException | RuntimeException e) {
+ jar.close();
+ throw e;
}
}
@@ -120,8 +156,12 @@ public class ReferenceCountedJar implements Jar {
@Override
public InputStream getEntryInputStream() throws IOException {
- try (ReferenceCountedJar jar = open()) {
- return jar.wrappedJar.getEntryInputStream();
+ ReferenceCountedJar jar = open();
+ try {
+ return jar.retainReference(jar.wrappedJar.getEntryInputStream());
+ } catch (IOException | RuntimeException e) {
+ jar.close();
+ throw e;
}
}
diff --git a/test/org/apache/tomcat/util/scan/TestReferenceCountedJar.java b/test/org/apache/tomcat/util/scan/TestReferenceCountedJar.java
new file mode 100644
index 0000000000..77a6c8a117
--- /dev/null
+++ b/test/org/apache/tomcat/util/scan/TestReferenceCountedJar.java
@@ -0,0 +1,182 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.util.scan;
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URI;
+import java.nio.charset.StandardCharsets;
+import java.util.jar.Attributes;
+import java.util.jar.JarEntry;
+import java.util.jar.JarOutputStream;
+import java.util.jar.Manifest;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import org.apache.catalina.webresources.TomcatURLStreamHandlerFactory;
+
+public class TestReferenceCountedJar {
+
+ @Rule
+ public TemporaryFolder testDir = new TemporaryFolder();
+
+ private static final String ENTRY_NAME = "test/resource.txt";
+ private static final String CONTENT = "hello";
+
+ @Before
+ public void register() {
+ TomcatURLStreamHandlerFactory.register();
+ }
+
+
+ private File createJar() throws IOException {
+ Manifest manifest = new Manifest();
+ manifest.getMainAttributes().put(Attributes.Name.MANIFEST_VERSION, "1.0");
+
+ File file = new File(testDir.getRoot(), "test.jar");
+ try (JarOutputStream jos = new JarOutputStream(new FileOutputStream(file), manifest)) {
+ jos.putNextEntry(new JarEntry(ENTRY_NAME));
+ jos.write(CONTENT.getBytes(StandardCharsets.UTF_8));
+ jos.closeEntry();
+ }
+ return file;
+ }
+
+
+ private static String readFully(InputStream is) throws IOException {
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ byte[] buffer = new byte[1024];
+ int read = is.read(buffer);
+ while (read > 0) {
+ baos.write(buffer, 0, read);
+ read = is.read(buffer);
+ }
+ return baos.toString("UTF-8");
+ }
+
+
+ /*
+ * The reference obtained internally when a stream is opened must be held until the stream is closed. Without it,
+ * reading a stream obtained after the wrapper has been closed and re-opened (the bug 62976 scenario) would fail
+ * because the wrapped JAR is closed as soon as getInputStream() returns.
+ */
+ @Test
+ public void testInputStreamRemainsValidWhenWrapperHasNoOtherReference() throws Exception {
+ File file = createJar();
+ URI jarUri = URI.create("jar:" + file.toURI().toURL().toExternalForm() + "!/");
+
+ ReferenceCountedJar jar = new ReferenceCountedJar(jarUri.toURL());
+ // Close the reference obtained by the constructor so the wrapper no longer holds any reference.
+ jar.close();
+
+ // This re-opens the wrapped JAR. The returned stream must stay usable even though no other reference is held.
+ try (InputStream is = jar.getInputStream(ENTRY_NAME)) {
+ Assert.assertNotNull(is);
+ Assert.assertEquals(CONTENT, readFully(is));
+ }
+
+ // Re-reading after the stream has been closed (releasing the reference it held) must still work.
+ try (InputStream is = jar.getInputStream(ENTRY_NAME)) {
+ Assert.assertEquals(CONTENT, readFully(is));
+ }
+ }
+
+
+ /*
+ * When a caller holds a reference and iterates the entries (as jasper does when parsing tag files), the reference
+ * obtained for the current entry stream must be held until that stream is closed and must not, on close, release
+ * the caller's reference.
+ */
+ @Test
+ public void testEntryInputStreamHoldsReferenceWhileIterating() throws Exception {
+ File file = createJar();
+ URI jarUri = URI.create("jar:" + file.toURI().toURL().toExternalForm() + "!/");
+
+ // Retain the constructor reference, as a real caller does across nextEntry()/getEntryInputStream().
+ try (ReferenceCountedJar jar = new ReferenceCountedJar(jarUri.toURL())) {
+ boolean found = false;
+ jar.nextEntry();
+ while (jar.getEntryName() != null) {
+ if (ENTRY_NAME.equals(jar.getEntryName())) {
+ try (InputStream is = jar.getEntryInputStream()) {
+ Assert.assertEquals(CONTENT, readFully(is));
+ }
+ found = true;
+ }
+ jar.nextEntry();
+ }
+ Assert.assertTrue(found);
+
+ // Closing the entry stream must not have released the caller's reference.
+ try (InputStream is = jar.getInputStream(ENTRY_NAME)) {
+ Assert.assertEquals(CONTENT, readFully(is));
+ }
+ }
+ }
+
+
+ /*
+ * Closing the returned stream more than once must release the held reference only once so the reference count is
+ * not decremented below the value to which the close of the stream should return it.
+ */
+ @Test
+ public void testStreamDoubleCloseReleasesReferenceOnce() throws Exception {
+ File file = createJar();
+ URI jarUri = URI.create("jar:" + file.toURI().toURL().toExternalForm() + "!/");
+
+ // Here the constructor reference is retained, as a normal caller would do.
+ try (ReferenceCountedJar jar = new ReferenceCountedJar(jarUri.toURL())) {
+ InputStream is = jar.getInputStream(ENTRY_NAME);
+ is.close();
+ is.close();
+
+ // The extra close must not have released the caller's reference. The JAR must still be readable through
+ // the caller's (still open) reference without being re-opened.
+ try (InputStream is2 = jar.getInputStream(ENTRY_NAME)) {
+ Assert.assertEquals(CONTENT, readFully(is2));
+ }
+ }
+ }
+
+
+ /*
+ * A request for a stream for a name that does not exist must release the opened reference immediately and return
+ * null rather than holding a reference that is never released.
+ */
+ @Test
+ public void testMissingEntryReleasesReference() throws Exception {
+ File file = createJar();
+ URI jarUri = URI.create("jar:" + file.toURI().toURL().toExternalForm() + "!/");
+
+ ReferenceCountedJar jar = new ReferenceCountedJar(jarUri.toURL());
+ jar.close();
+
+ Assert.assertNull(jar.getInputStream("test/does-not-exist.txt"));
+
+ // The failed request must not have left a reference held: a later successful request must still work.
+ try (InputStream is = jar.getInputStream(ENTRY_NAME)) {
+ Assert.assertEquals(CONTENT, readFully(is));
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 19/23: Avoid busy spin in TaskQueue.take() when the keep-alive time is zero
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 9cb44cd42f49929de2c58ae3de5282fa17cd528b
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 14:21:04 2026 +0200
Avoid busy spin in TaskQueue.take() when the keep-alive time is zero
A worker whose thread is due to be stopped waits with a timed poll so
that the check for whether the thread should stop runs periodically.
The poll used the pool keep-alive time as the timeout. That time can be
configured to zero, in which case the poll can not block and, for a
core thread that is not subject to being culled, the wait degenerates
into a loop that consumes a full CPU core while the queue is empty.
When the configured timeout rounds down to zero, wait for the thread
renewal delay instead. The interval at which a thread may stop itself is
gated by that same delay, so the check still runs at a bounded interval
without the spin, arriving tasks are still picked up immediately and
the wait remains interruptible. Timed waits for non-core workers, which
rely on a zero keep-alive time to be culled immediately, are not
affected.
Add TestTaskQueue with a regression test that counts the poll
iterations performed by idle, renewal-due workers. Without the bounded
timeout the count reaches millions within the test window.
---
java/org/apache/tomcat/util/threads/TaskQueue.java | 11 ++-
.../apache/tomcat/util/threads/TestTaskQueue.java | 92 ++++++++++++++++++++++
2 files changed, 102 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/tomcat/util/threads/TaskQueue.java b/java/org/apache/tomcat/util/threads/TaskQueue.java
index 3f69271aa0..d906d7a789 100644
--- a/java/org/apache/tomcat/util/threads/TaskQueue.java
+++ b/java/org/apache/tomcat/util/threads/TaskQueue.java
@@ -138,7 +138,16 @@ public class TaskQueue extends LinkedBlockingQueue<Runnable> implements Retryabl
@Override
public Runnable take() throws InterruptedException {
if (parent != null && parent.currentThreadShouldBeStopped()) {
- return poll(parent.getKeepAliveTime(TimeUnit.MILLISECONDS), TimeUnit.MILLISECONDS);
+ long timeout = parent.getKeepAliveTime(TimeUnit.MILLISECONDS);
+ if (timeout <= 0) {
+ /*
+ * A timed wait with a zero timeout can not block. Waiting for the thread renewal delay instead keeps
+ * the check for whether the thread should stop at a bounded interval without consuming CPU while the
+ * queue is empty.
+ */
+ timeout = Math.max(1, parent.getThreadRenewalDelay());
+ }
+ return poll(timeout, TimeUnit.MILLISECONDS);
// yes, this may return null (in case of timeout) which normally
// does not occur with take()
// but the ThreadPoolExecutor implementation allows this
diff --git a/test/org/apache/tomcat/util/threads/TestTaskQueue.java b/test/org/apache/tomcat/util/threads/TestTaskQueue.java
new file mode 100644
index 0000000000..6d117b9cce
--- /dev/null
+++ b/test/org/apache/tomcat/util/threads/TestTaskQueue.java
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.util.threads;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestTaskQueue {
+
+ private static class CountingTaskQueue extends TaskQueue {
+
+ private final AtomicInteger pollCount = new AtomicInteger();
+
+ @Override
+ public Runnable poll(long timeout, TimeUnit unit) throws InterruptedException {
+ pollCount.incrementAndGet();
+ return super.poll(timeout, unit);
+ }
+ }
+
+
+ /*
+ * A thread that is due to be stopped waits with poll() rather than take() so the stop check runs periodically. If
+ * the keep-alive time is zero, a poll() with a zero timeout can not block and the worker spins while the queue is
+ * empty. The wait must be bounded to a non-zero interval instead.
+ */
+ @Test
+ public void testTakeWithZeroKeepAliveDoesNotSpin() throws Exception {
+ CountingTaskQueue queue = new CountingTaskQueue();
+ /*
+ * Two workers so that one stops itself (the first to claim the renewal slot) while the other remains due for
+ * renewal and waiting. A long renewal delay keeps the waiting thread renewing rather than stopping during the
+ * observation window below.
+ */
+ ThreadPoolExecutor executor = new ThreadPoolExecutor(2, 2, 0, TimeUnit.MILLISECONDS, queue,
+ new TaskThreadFactory("test-take-zero", false, Thread.NORM_PRIORITY));
+ queue.setParent(executor);
+ executor.setThreadRenewalDelay(10000);
+
+ try {
+ CountDownLatch executed = new CountDownLatch(4);
+ executor.execute(executed::countDown);
+ executor.execute(executed::countDown);
+ // Wait for both workers to be created and to enter take()
+ Thread.sleep(200);
+
+ // Mark the (older) worker threads as due to be stopped
+ executor.contextStopping();
+ executor.execute(executed::countDown);
+ executor.execute(executed::countDown);
+ // The workers complete these tasks and then re-enter take() which, since the threads are due to be
+ // stopped, switches to poll() with a bounded timeout
+ Assert.assertTrue(executed.await(5, TimeUnit.SECONDS));
+
+ int pollsAfterTask = queue.pollCount.get();
+ Thread.sleep(500);
+ int spinIterations = queue.pollCount.get() - pollsAfterTask;
+
+ // Without the bounded timeout this is orders of magnitude larger (millions in this window)
+ Assert.assertTrue("Unexpected poll count while idle: " + spinIterations, spinIterations < 100);
+
+ // Both workers must have entered the renewal wait at least once
+ Assert.assertTrue("Renewal wait was not exercised", queue.pollCount.get() >= 2);
+
+ // The workers must still be able to process tasks
+ CountDownLatch processed = new CountDownLatch(1);
+ executor.execute(processed::countDown);
+ Assert.assertTrue(processed.await(5, TimeUnit.SECONDS));
+ } finally {
+ executor.shutdownNow();
+ Assert.assertTrue(executor.awaitTermination(10, TimeUnit.SECONDS));
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 05/23: Correct the cipher authentication method computation of the OpenSSL FFM implementation
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit bd550d39bfd3d6406e3f852187365809e193ed87
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 09:22:08 2026 +0200
Correct the cipher authentication method computation of the OpenSSL FFM implementation
getCipherAuthenticationMethod() compared the key exchange and
authentication values returned by SSL_CIPHER_get_kx_nid() and
SSL_CIPHER_get_auth_nid(), which are NIDs, against the legacy OpenSSL
SSL_k* and SSL_a* flag constants. Only NID_kx_rsa could ever match, so
a custom X509TrustManager received UNKNOWN as the authentication type
for virtually every cipher suite.
The legacy constants trace back to the algorithm_mkey/algorithm_auth
bitmask table of OpenSSL 1.0.x ssl_locl.h, as also carried by the
LibreSSL compatibility branch of Tomcat Native. They were copied into
the FFM port with the hex base dropped (0x10 became 10, 0x40 became 40,
and so on) and then compared against NIDs, so they could not match
anything.
The constants now hold the real NID_kx_rsa, NID_kx_ecdhe, NID_kx_dhe,
NID_auth_rsa, NID_auth_ecdsa, NID_auth_dss and NID_auth_null values,
hard-coded because these macros are not present in the generated
bindings, and the switch now mirrors SSL_CIPHER_authentication_method()
of Tomcat Native byte for byte, including the UNKNOWN results for
combinations without a JSSE authentication type such as the TLS 1.3
kx-any value.
---
.../util/net/openssl/panama/OpenSSLContext.java | 73 ++++++++--------------
webapps/docs/changelog.xml | 9 +++
2 files changed, 34 insertions(+), 48 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
index 48e4296632..4ad3b80801 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
@@ -836,66 +836,43 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext {
}
}
- private static final int NID_kx_rsa = 1037/* NID_kx_rsa() */;
- // private static final int NID_kx_dhe = NID_kx_dhe();
- // private static final int NID_kx_ecdhe = NID_kx_ecdhe();
-
- // private static final int NID_auth_rsa = NID_auth_rsa();
- // private static final int NID_auth_dss = NID_auth_dss();
- // private static final int NID_auth_null = NID_auth_null();
- // private static final int NID_auth_ecdsa = NID_auth_ecdsa();
-
- // private static final int SSL_kRSA = 1;
- private static final int SSL_kDHr = 2;
- private static final int SSL_kDHd = 4;
- private static final int SSL_kEDH = 8;
- private static final int SSL_kDHE = SSL_kEDH;
- private static final int SSL_kKRB5 = 10;
- private static final int SSL_kECDHr = 20;
- private static final int SSL_kECDHe = 40;
- private static final int SSL_kEECDH = 80;
- private static final int SSL_kECDHE = SSL_kEECDH;
- // private static final int SSL_kPSK = 100;
- // private static final int SSL_kGOST = 200;
- // private static final int SSL_kSRP = 400;
-
- private static final int SSL_aRSA = 1;
- private static final int SSL_aDSS = 2;
- private static final int SSL_aNULL = 4;
- // private static final int SSL_aDH = 8;
- // private static final int SSL_aECDH = 10;
- // private static final int SSL_aKRB5 = 20;
- private static final int SSL_aECDSA = 40;
- // private static final int SSL_aPSK = 80;
- // private static final int SSL_aGOST94 = 100;
- // private static final int SSL_aGOST01 = 200;
- // private static final int SSL_aSRP = 400;
+ // Key exchange and authentication NIDs. The values are hard-coded since
+ // these OpenSSL constants are macros that are not present in the
+ // generated bindings. They are defined in OpenSSL's obj_mac.h.
+ private static final int NID_kx_rsa = 1037;
+ private static final int NID_kx_ecdhe = 1038;
+ private static final int NID_kx_dhe = 1039;
+
+ private static final int NID_auth_rsa = 1046;
+ private static final int NID_auth_ecdsa = 1047;
+ private static final int NID_auth_dss = 1049;
+ private static final int NID_auth_null = 1053;
private static final String SSL_TXT_RSA = "RSA";
private static final String SSL_TXT_DH = "DH";
private static final String SSL_TXT_DSS = "DSS";
- private static final String SSL_TXT_KRB5 = "KRB5";
private static final String SSL_TXT_ECDH = "ECDH";
private static final String SSL_TXT_ECDSA = "ECDSA";
private static String getCipherAuthenticationMethod(int auth, int kx) {
+ // Mirrors the computation Tomcat Native performs for the JNI based
+ // implementation: a switch on the key exchange NID of the current
+ // cipher, using the authentication NID to disambiguate where
+ // required. Combinations without a JSSE authentication type,
+ // including the TLS 1.3 "any" values, result in "UNKNOWN" there and
+ // do so here.
return switch (kx) {
case NID_kx_rsa -> SSL_TXT_RSA;
- case SSL_kDHr -> SSL_TXT_DH + "_" + SSL_TXT_RSA;
- case SSL_kDHd -> SSL_TXT_DH + "_" + SSL_TXT_DSS;
- case SSL_kDHE -> switch (auth) {
- case SSL_aDSS -> "DHE_" + SSL_TXT_DSS;
- case SSL_aRSA -> "DHE_" + SSL_TXT_RSA;
- case SSL_aNULL -> SSL_TXT_DH + "_anon";
+ case NID_kx_dhe -> switch (auth) {
+ case NID_auth_dss -> "DHE_" + SSL_TXT_DSS;
+ case NID_auth_rsa -> "DHE_" + SSL_TXT_RSA;
+ case NID_auth_null -> SSL_TXT_DH + "_anon";
default -> "UNKNOWN";
};
- case SSL_kKRB5 -> SSL_TXT_KRB5;
- case SSL_kECDHr -> SSL_TXT_ECDH + "_" + SSL_TXT_RSA;
- case SSL_kECDHe -> SSL_TXT_ECDH + "_" + SSL_TXT_ECDSA;
- case SSL_kECDHE -> switch (auth) {
- case SSL_aECDSA -> "ECDHE_" + SSL_TXT_ECDSA;
- case SSL_aRSA -> "ECDHE_" + SSL_TXT_RSA;
- case SSL_aNULL -> SSL_TXT_ECDH + "_anon";
+ case NID_kx_ecdhe -> switch (auth) {
+ case NID_auth_ecdsa -> "ECDHE_" + SSL_TXT_ECDSA;
+ case NID_auth_rsa -> "ECDHE_" + SSL_TXT_RSA;
+ case NID_auth_null -> SSL_TXT_ECDH + "_anon";
default -> "UNKNOWN";
};
default -> "UNKNOWN";
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 1dcad89624..e900eea853 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -362,6 +362,15 @@
than adding <code>null</code> to the lists of cipher suites they
expose. (remm)
</fix>
+ <fix>
+ Correct the computation of the authentication method passed to a custom
+ <code>X509TrustManager</code> by the FFM based OpenSSL implementation.
+ The key exchange and authentication NIDs of the negotiated cipher were
+ compared against legacy OpenSSL flag values rather than the actual NID
+ constants, so the trust manager received <code>UNKNOWN</code> as the
+ authentication type for almost every cipher suite. The computation now
+ matches the one performed by the JNI based implementation. (remm)
+ </fix>
</changelog>
</subsection>
<subsection name="Web applications">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 23/23: Shut down the internal virtual thread executor when the endpoint stops
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 93dea1dc4db626c072526e7587bb41ca95bef0bd
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 15:31:00 2026 +0200
Shut down the internal virtual thread executor when the endpoint stops
shutdownExecutor() only shut down the thread pool branch, leaving a
virtual thread executor accepting tasks submitted through a reference
captured before the stop. Shut it down as well, so such submissions
are rejected, as they already are with the thread pool executor.
---
.../apache/tomcat/util/net/AbstractEndpoint.java | 6 +++
.../tomcat/util/net/TestAbstractEndpoint.java | 44 ++++++++++++++++++++++
2 files changed, 50 insertions(+)
diff --git a/java/org/apache/tomcat/util/net/AbstractEndpoint.java b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
index 3a9335085d..9af6762bd2 100644
--- a/java/org/apache/tomcat/util/net/AbstractEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
@@ -1993,6 +1993,12 @@ public abstract class AbstractEndpoint<S, U> {
}
TaskQueue queue = (TaskQueue) tpe.getQueue();
queue.setParent(null);
+ } else if (executor instanceof VirtualThreadExecutor virtualThreadExecutor) {
+ // The virtual thread executor accepts new tasks until it is shut
+ // down. Shut it down, so tasks submitted via a reference captured
+ // before this point are rejected, as they are with the thread
+ // pool executor.
+ virtualThreadExecutor.shutdownNow();
}
}
}
diff --git a/test/org/apache/tomcat/util/net/TestAbstractEndpoint.java b/test/org/apache/tomcat/util/net/TestAbstractEndpoint.java
index 055cab7512..4b06e254c9 100644
--- a/test/org/apache/tomcat/util/net/TestAbstractEndpoint.java
+++ b/test/org/apache/tomcat/util/net/TestAbstractEndpoint.java
@@ -16,12 +16,17 @@
*/
package org.apache.tomcat.util.net;
+import java.util.concurrent.Executor;
+import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
+import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.ScheduledExecutorService;
import org.junit.Assert;
import org.junit.Test;
+import org.apache.tomcat.util.threads.VirtualThreadExecutor;
+
public class TestAbstractEndpoint {
@Test
@@ -65,4 +70,43 @@ public class TestAbstractEndpoint {
executor.shutdown();
}
}
+
+ /*
+ * The virtual thread executor accepts tasks until it is shut down. A reference captured before the endpoint
+ * stopped must be rejected, as it is when the executor is a thread pool.
+ */
+ @Test
+ public void testCreatedVirtualThreadExecutorIsShutDownOnStop() throws Exception {
+ NioEndpoint endpoint = new NioEndpoint();
+ endpoint.setUseVirtualThreads(true);
+ endpoint.createExecutor();
+ Executor executor = endpoint.getExecutor();
+ Assert.assertTrue(executor instanceof VirtualThreadExecutor);
+
+ endpoint.shutdownExecutor();
+
+ Assert.assertTrue(((ExecutorService) executor).isShutdown());
+ try {
+ executor.execute(() -> {
+ });
+ Assert.fail("Expected RejectedExecutionException");
+ } catch (RejectedExecutionException e) {
+ // Expected.
+ }
+ }
+
+ @Test
+ public void testExternalVirtualThreadExecutorIsNotShutDownOnStop() throws Exception {
+ NioEndpoint endpoint = new NioEndpoint();
+ VirtualThreadExecutor executor = new VirtualThreadExecutor("test-external-");
+ endpoint.setExecutor(executor);
+ try {
+ endpoint.shutdownExecutor();
+
+ // Externally provided executors are owned by the caller
+ Assert.assertFalse(executor.isShutdown());
+ } finally {
+ executor.shutdownNow();
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 08/23: Correct the FILE check for SSL_CONF commands in the OpenSSL FFM code
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit b973929ab1bd2fec7358edc2406f905a3d3eba62
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 10:47:53 2026 +0200
Correct the FILE check for SSL_CONF commands in the OpenSSL FFM code
The checkConf() method of the panama OpenSSLContext rejected an
SSL_CONF_TYPE_FILE command only when the configured path was neither a
file nor readable, i.e. when both conditions failed. As a result, a
readable directory or an unreadable regular file passed the check and
the invalid value was only rejected later, when SSL_CONF_cmd tried to
load it, reporting a raw OpenSSL error instead of the clear
opensslconf.badFile message.
Use the intended 'not a file or not readable' test so that only a
readable regular file is accepted. This matches the behaviour of the
JNI/tomcat-native implementation, whose path_check() requires the path
to exist and to be a regular file. Symbolic links to readable regular
files continue to pass, since File.isFile() and File.canRead() resolve
links just like apr_stat() does on the JNI side.
---
java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
index 4ad3b80801..88a99ce4ae 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
@@ -383,7 +383,7 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext {
if (code == SSL_CONF_TYPE_FILE()) {
// Check file
File file = new File(value);
- if (!file.isFile() && !file.canRead()) {
+ if (!file.isFile() || !file.canRead()) {
log.error(sm.getString("opensslconf.badFile", name, value));
ok = false;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 17/23: Report the resource path when a WEB-INF/lib JAR cannot be scanned
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 11.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 3eaebe9d490b85318d506dd6c77995353cd41ce1
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 16 13:51:30 2026 +0200
Report the resource path when a WEB-INF/lib JAR cannot be scanned
When scanning WEB-INF/lib, the debug message, the warning for a null
resource and the trace message for a skipped JAR all identify the JAR
by its web application relative path. The warning for an IOException
used the URL local variable instead. That variable is still null if it
is the getResource() call itself that fails, and a full URL is
redundant in a message that already states the JAR came from
/WEB-INF/lib. Use the path in this message as well so every log
message from the loop identifies the JAR the same way.
---
java/org/apache/tomcat/util/scan/StandardJarScanner.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/java/org/apache/tomcat/util/scan/StandardJarScanner.java b/java/org/apache/tomcat/util/scan/StandardJarScanner.java
index da1fa5468b..c0df03db83 100644
--- a/java/org/apache/tomcat/util/scan/StandardJarScanner.java
+++ b/java/org/apache/tomcat/util/scan/StandardJarScanner.java
@@ -259,7 +259,7 @@ public class StandardJarScanner implements JarScanner {
log.warn(sm.getString("jarScan.webinflibFail", path));
}
} catch (IOException ioe) {
- log.warn(sm.getString("jarScan.webinflibFail", url), ioe);
+ log.warn(sm.getString("jarScan.webinflibFail", path), ioe);
}
} else {
if (log.isTraceEnabled()) {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org