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/09 10:44:07 UTC
(tomcat) branch 9.0.x updated (9a93d1e3be -> bd100b3335)
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a change to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
from 9a93d1e3be Java compat
new 51202c9bb9 Fix storeconfig default-manager check always rejecting started managers
new ac2c4b2b30 Do not store the runtime-derived OpenSSLConfCmd groups command
new 3f55f914fe Do not store the runtime placeholder UNDEFINED Certificate element
new 0edf7f4a07 Report failure when storing a context class without store description
new 47ebd7c941 Log the ignored IOException when storing a child element fails
new b94194b5d4 Make the configuration file swap in StoreFileMover crash-safe
new 2607fb1fc5 Fix NullPointerException in ChannelException.getMessage() without detail
new 05a2a814ea Copy the byte array in the UniqueId(byte[]) constructor
new 4656f23c4b Fix use-after-return of pooled send buffer in async sends
new 5c1ac02fd8 Do not re-start already started services on overlapping starts
new 51472725f2 Do not send an RPC reply when the callback returns null
new 2d2f2557de Keep monitor, heartbeat and JMX alive across partial channel stops
new 6d85b214c8 Make RpcMessage serialization tolerate null identifiers
new 6e3827031f Java compat
new bd100b3335 Drop test
The 15 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:
.../catalina/storeconfig/LocalStrings.properties | 3 +-
.../org/apache/catalina/storeconfig/ManagerSF.java | 57 ++--
.../catalina/storeconfig/SSLHostConfigSF.java | 39 ++-
.../apache/catalina/storeconfig/StoreConfig.java | 20 +-
.../catalina/storeconfig/StoreFactoryBase.java | 7 +-
.../catalina/storeconfig/StoreFileMover.java | 55 +++-
.../apache/catalina/tribes/ChannelException.java | 6 +-
java/org/apache/catalina/tribes/UniqueId.java | 9 +-
.../catalina/tribes/group/ChannelCoordinator.java | 23 +-
.../apache/catalina/tribes/group/GroupChannel.java | 43 ++-
.../apache/catalina/tribes/group/RpcChannel.java | 8 +
.../apache/catalina/tribes/group/RpcMessage.java | 27 +-
.../interceptors/MessageDispatchInterceptor.java | 13 +-
java/org/apache/tomcat/util/net/SSLHostConfig.java | 10 +
.../catalina/storeconfig/TestStoreConfig.java | 361 +++++++++++++++++++++
.../catalina/tribes/TestChannelException.java | 61 ++++
test/org/apache/catalina/tribes/TestUniqueId.java | 67 ++++
.../tribes/group/TestGroupChannelStartStop.java | 59 ++++
.../catalina/tribes/group/TestRpcChannel.java | 163 ++++++++++
.../catalina/tribes/group/TestRpcMessage.java | 88 +++++
.../TestMessageDispatchInterceptor.java | 141 ++++++++
webapps/docs/changelog.xml | 26 ++
22 files changed, 1197 insertions(+), 89 deletions(-)
create mode 100644 test/org/apache/catalina/tribes/TestChannelException.java
create mode 100644 test/org/apache/catalina/tribes/TestUniqueId.java
create mode 100644 test/org/apache/catalina/tribes/group/TestRpcChannel.java
create mode 100644 test/org/apache/catalina/tribes/group/TestRpcMessage.java
create mode 100644 test/org/apache/catalina/tribes/group/interceptors/TestMessageDispatchInterceptor.java
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 08/15: Copy the byte array in the UniqueId(byte[]) constructor
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 05a2a814ead578e25b76d1e3d3219632459394a9
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 9 11:09:27 2026 +0200
Copy the byte array in the UniqueId(byte[]) constructor
The single-argument constructor retained the caller's array by
reference, unlike the three-argument constructor which copies. Since
UniqueId computes equals() and hashCode() from that array and is used
as a map key, e.g. in TwoPhaseCommitInterceptor, a caller mutating the
original array after construction could silently corrupt key lookups.
No current call site mutates a shared array, so this is a latent API
hazard rather than an active bug.
Make the constructor store a defensive copy and update the Javadoc of
the constructor and of getBytes() accordingly. Add TestUniqueId to
cover the copy semantics, null handling and map key stability.
---
java/org/apache/catalina/tribes/UniqueId.java | 9 +--
test/org/apache/catalina/tribes/TestUniqueId.java | 67 +++++++++++++++++++++++
2 files changed, 72 insertions(+), 4 deletions(-)
diff --git a/java/org/apache/catalina/tribes/UniqueId.java b/java/org/apache/catalina/tribes/UniqueId.java
index bbe2d7918f..ceb7baf27d 100644
--- a/java/org/apache/catalina/tribes/UniqueId.java
+++ b/java/org/apache/catalina/tribes/UniqueId.java
@@ -39,12 +39,13 @@ public final class UniqueId implements Serializable {
}
/**
- * Constructs a new UniqueId from the given byte array.
+ * Constructs a new UniqueId from the given byte array. A defensive copy of the array is made, so later changes to
+ * the caller's array do not affect this object.
*
- * @param id the byte array containing the identifier. The array is retained by reference and must not be modified.
+ * @param id the byte array containing the identifier
*/
public UniqueId(byte[] id) {
- this.id = id;
+ this.id = id != null ? id.clone() : null;
}
/**
@@ -86,7 +87,7 @@ public final class UniqueId implements Serializable {
}
/**
- * Returns the raw bytes of this unique identifier.
+ * Returns the raw bytes of this unique identifier. Do not modify the returned array.
*
* @return the byte array
*/
diff --git a/test/org/apache/catalina/tribes/TestUniqueId.java b/test/org/apache/catalina/tribes/TestUniqueId.java
new file mode 100644
index 0000000000..966f0bf5c1
--- /dev/null
+++ b/test/org/apache/catalina/tribes/TestUniqueId.java
@@ -0,0 +1,67 @@
+/*
+ * 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.catalina.tribes;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestUniqueId {
+
+ @Test
+ public void testNullId() {
+ UniqueId uid = new UniqueId();
+ Assert.assertNull(uid.getBytes());
+ Assert.assertEquals(new UniqueId(), uid);
+ Assert.assertEquals(new UniqueId().hashCode(), uid.hashCode());
+ }
+
+ @Test
+ public void testByteArrayConstructorDoesNotRetainReference() {
+ byte[] id = new byte[] { 1, 2, 3, 4 };
+ UniqueId uid = new UniqueId(id);
+ Assert.assertNotSame(id, uid.getBytes());
+ Assert.assertArrayEquals(id, uid.getBytes());
+ id[0] = 99;
+ // The UniqueId must be unaffected by the mutation above
+ Assert.assertEquals(1, uid.getBytes()[0]);
+ }
+
+ @Test
+ public void testMapKeySurvivesCallerMutation() {
+ byte[] id = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 };
+ UniqueId uid = new UniqueId(id);
+ Map<UniqueId, String> map = new HashMap<>();
+ map.put(uid, "value");
+ id[0] = 99;
+ Assert.assertEquals("value", map.get(uid));
+ UniqueId equalId = new UniqueId(
+ new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 });
+ Assert.assertTrue(map.containsKey(equalId));
+ }
+
+ @Test
+ public void testRangeConstructorCopies() {
+ byte[] source = new byte[] { 0, 1, 2, 3, 4 };
+ UniqueId uid = new UniqueId(source, 1, 3);
+ Assert.assertNotNull(uid.getBytes());
+ source[1] = 99;
+ Assert.assertEquals(1, uid.getBytes()[0]);
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 06/15: Make the configuration file swap in StoreFileMover crash-safe
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit b94194b5d4ef0fc0bb83a20226b210278c4a79ca
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 9 10:47:41 2026 +0200
Make the configuration file swap in StoreFileMover crash-safe
move() installed a newly stored configuration file with two renames: the
previous file was first renamed to the timestamped backup name, then the
new file was renamed to the active name. A crash between the two left
the active server.xml or context.xml missing, and Tomcat could not start
until an administrator renamed the backup back manually.
Copy the previous file to the backup location instead of renaming it
away, and install the new file with a single move which replaces the
target atomically where the file system supports it. The active
configuration file is now never absent during the swap and the restore
path is no longer needed.
---
.../catalina/storeconfig/LocalStrings.properties | 2 +-
.../catalina/storeconfig/StoreFileMover.java | 55 +++++++---
.../catalina/storeconfig/TestStoreFileMover.java | 118 +++++++++++++++++++++
3 files changed, 158 insertions(+), 17 deletions(-)
diff --git a/java/org/apache/catalina/storeconfig/LocalStrings.properties b/java/org/apache/catalina/storeconfig/LocalStrings.properties
index 33f5ee22fa..973f4b3b98 100644
--- a/java/org/apache/catalina/storeconfig/LocalStrings.properties
+++ b/java/org/apache/catalina/storeconfig/LocalStrings.properties
@@ -50,10 +50,10 @@ storeConfigListener.registerError=Error registering StoreConfig MBean
storeFactory.noDescriptor=Descriptor for element [{0}].[{1}] not configured
+storeFileMover.copyError=Failed to copy the configuration file [{0}] to the backup location [{1}]
storeFileMover.directoryCreationError=Cannot create directory [{0}]
storeFileMover.null=Invalid null basename, filename or encoding
storeFileMover.renameError=Cannot rename [{0}] to [{1}]
-storeFileMover.restoreError=Rename [{0}] to [{1}] failed and restoring also failed
xmlFormatPreserver.failed=Failed to preserve the original layout of the configuration file
xmlFormatPreserver.unreadable=Failed to read the previous version of the configuration file [{0}]
diff --git a/java/org/apache/catalina/storeconfig/StoreFileMover.java b/java/org/apache/catalina/storeconfig/StoreFileMover.java
index 43180e314a..f0706a2423 100644
--- a/java/org/apache/catalina/storeconfig/StoreFileMover.java
+++ b/java/org/apache/catalina/storeconfig/StoreFileMover.java
@@ -21,6 +21,9 @@ import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
+import java.nio.file.AtomicMoveNotSupportedException;
+import java.nio.file.Files;
+import java.nio.file.StandardCopyOption;
import java.sql.Timestamp;
import org.apache.catalina.Globals;
@@ -176,33 +179,53 @@ public class StoreFileMover {
}
/**
- * Shuffle old->save and new->old.
+ * Install the new configuration file, saving the previous one as a backup when it exists. The backup is a copy and
+ * the active file is replaced by a single move, so the configuration file is never absent.
*
* @throws IOException a file operation error occurred
*/
public void move() throws IOException {
- if (configOld.renameTo(configSave)) {
- if (!configNew.renameTo(configOld)) {
- if (!configSave.renameTo(configOld)) {
- throw new IOException(sm.getString("storeFileMover.restoreError", configNew.getAbsolutePath(),
- configOld.getAbsolutePath()));
- }
+ if (configOld.exists()) {
+ // Copy the existing configuration file to the backup location rather
+ // than renaming it away. Replacing the configuration file with a move
+ // is atomic, so the active configuration file is never absent, even
+ // when the process dies before the replacement happens
+ try {
+ Files.copy(configOld.toPath(), configSave.toPath(), StandardCopyOption.COPY_ATTRIBUTES);
+ } catch (IOException ioe) {
+ throw new IOException(sm.getString("storeFileMover.copyError", configOld.getAbsolutePath(),
+ configSave.getAbsolutePath()), ioe);
+ }
+ try {
+ Files.move(configNew.toPath(), configOld.toPath(), StandardCopyOption.REPLACE_EXISTING,
+ StandardCopyOption.ATOMIC_MOVE);
+ } catch (AtomicMoveNotSupportedException amns) {
+ // Fall back to a plain replace when atomic moves are unsupported
+ moveWithReplace();
+ } catch (IOException ioe) {
throw new IOException(sm.getString("storeFileMover.renameError", configNew.getAbsolutePath(),
- configOld.getAbsolutePath()));
+ configOld.getAbsolutePath()), ioe);
}
} else {
- if (!configOld.exists()) {
- if (!configNew.renameTo(configOld)) {
- throw new IOException(sm.getString("storeFileMover.renameError", configNew.getAbsolutePath(),
- configOld.getAbsolutePath()));
- }
- } else {
- throw new IOException(sm.getString("storeFileMover.renameError", configOld.getAbsolutePath(),
- configSave.getAbsolutePath()));
+ // Nothing to back up: install the new file directly
+ try {
+ Files.move(configNew.toPath(), configOld.toPath());
+ } catch (IOException ioe) {
+ throw new IOException(sm.getString("storeFileMover.renameError", configNew.getAbsolutePath(),
+ configOld.getAbsolutePath()), ioe);
}
}
}
+ private void moveWithReplace() throws IOException {
+ try {
+ Files.move(configNew.toPath(), configOld.toPath(), StandardCopyOption.REPLACE_EXISTING);
+ } catch (IOException ioe) {
+ throw new IOException(sm.getString("storeFileMover.renameError", configNew.getAbsolutePath(),
+ configOld.getAbsolutePath()), ioe);
+ }
+ }
+
/**
* Open an output writer for the new configuration file.
*
diff --git a/test/org/apache/catalina/storeconfig/TestStoreFileMover.java b/test/org/apache/catalina/storeconfig/TestStoreFileMover.java
new file mode 100644
index 0000000000..e09661bce5
--- /dev/null
+++ b/test/org/apache/catalina/storeconfig/TestStoreFileMover.java
@@ -0,0 +1,118 @@
+/*
+ * 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.catalina.storeconfig;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Comparator;
+import java.util.stream.Stream;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestStoreFileMover {
+
+ /**
+ * Verify the regular swap: the new content is installed, the old content is preserved in the backup and the
+ * intermediate file is consumed.
+ *
+ * @throws Exception if the test experiences an unexpected error
+ */
+ @Test
+ public void testMoveWithBackup() throws Exception {
+ Path dir = Files.createTempDirectory("storeFileMover");
+ try {
+ Files.writeString(dir.resolve("server.xml"), "old");
+ StoreFileMover mover = new StoreFileMover(dir.toString(), "server.xml", "UTF-8");
+ try (PrintWriter writer = mover.getWriter()) {
+ writer.print("new");
+ }
+ mover.move();
+ Assert.assertEquals("new", Files.readString(dir.resolve("server.xml")));
+ Assert.assertFalse(Files.exists(dir.resolve("server.xml.new")));
+ Path backup = mover.getConfigSave().toPath();
+ Assert.assertTrue(Files.exists(backup));
+ Assert.assertEquals("old", Files.readString(backup));
+ } finally {
+ deleteRecursively(dir);
+ }
+ }
+
+ /**
+ * Verify that without an existing configuration file, the new file is installed directly without creating a
+ * backup.
+ *
+ * @throws Exception if the test experiences an unexpected error
+ */
+ @Test
+ public void testMoveWithoutExistingConfiguration() throws Exception {
+ Path dir = Files.createTempDirectory("storeFileMover");
+ try {
+ StoreFileMover mover = new StoreFileMover(dir.toString(), "server.xml", "UTF-8");
+ try (PrintWriter writer = mover.getWriter()) {
+ writer.print("new");
+ }
+ mover.move();
+ Assert.assertEquals("new", Files.readString(dir.resolve("server.xml")));
+ Assert.assertFalse(Files.exists(mover.getConfigSave().toPath()));
+ } finally {
+ deleteRecursively(dir);
+ }
+ }
+
+ /**
+ * Verify that a failure to create the backup leaves the active configuration file in place and unchanged. The
+ * backup is copied rather than renamed away, so no failure during the swap can leave the configuration file
+ * missing.
+ *
+ * @throws Exception if the test experiences an unexpected error
+ */
+ @Test
+ public void testBackupFailureLeavesConfigurationInPlace() throws Exception {
+ Path dir = Files.createTempDirectory("storeFileMover");
+ try {
+ Files.writeString(dir.resolve("server.xml"), "old");
+ StoreFileMover mover = new StoreFileMover(dir.toString(), "server.xml", "UTF-8");
+ try (PrintWriter writer = mover.getWriter()) {
+ writer.print("new");
+ }
+ // Block the backup location so that the backup copy fails
+ Files.createDirectory(mover.getConfigSave().toPath());
+ try {
+ mover.move();
+ Assert.fail("move() should have failed");
+ } catch (IOException ioe) {
+ // Expected, and it must report the failed backup copy
+ Assert.assertTrue(ioe.getMessage(), ioe.getMessage().contains("Failed to copy"));
+ }
+ Assert.assertEquals("old", Files.readString(dir.resolve("server.xml")));
+ Assert.assertTrue(Files.exists(dir.resolve("server.xml.new")));
+ } finally {
+ deleteRecursively(dir);
+ }
+ }
+
+ private static void deleteRecursively(Path dir) throws IOException {
+ try (Stream<Path> paths = Files.walk(dir)) {
+ for (Path path : (Iterable<Path>) paths.sorted(Comparator.reverseOrder())::iterator) {
+ Files.deleteIfExists(path);
+ }
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 05/15: Log the ignored IOException when storing a child element fails
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 47ebd7c9414a9c1c0af7e6fba426ce414c2bdc74
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 9 10:34:59 2026 +0200
Log the ignored IOException when storing a child element fails
storeElementArray() caught and silently discarded IOExceptions thrown
per element. Those exceptions are only produced by the context
separate-file storage paths (storeWithBackup / storeContextSeparate)
when the target file cannot be written, and those methods throw without
logging. A context whose configuration file could not be written was
therefore dropped from a full configuration save without any log entry,
while the save reported success, so the administrator's changes were
silently lost on the next restart.
Report a warning naming the element class along with the exception, and
keep storing the remaining elements as before.
---
java/org/apache/catalina/storeconfig/LocalStrings.properties | 1 +
java/org/apache/catalina/storeconfig/StoreFactoryBase.java | 7 ++++---
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/catalina/storeconfig/LocalStrings.properties b/java/org/apache/catalina/storeconfig/LocalStrings.properties
index e26bf11e07..33f5ee22fa 100644
--- a/java/org/apache/catalina/storeconfig/LocalStrings.properties
+++ b/java/org/apache/catalina/storeconfig/LocalStrings.properties
@@ -22,6 +22,7 @@ config.objectNameNotFound=Object name [{0}] not found
config.storeContextError=Error storing context [{0}]
config.storeServerError=Error storing server
+factory.storeElementFailed=Failed to store element [{0}], it will be missing from the stored configuration
factory.storeNoDescriptor=Descriptor for element class [{0}] not configured!
factory.storeTag=store tag [{0}] ( Object: [{1}] )
diff --git a/java/org/apache/catalina/storeconfig/StoreFactoryBase.java b/java/org/apache/catalina/storeconfig/StoreFactoryBase.java
index 5cbb2ede30..273b1f24a9 100644
--- a/java/org/apache/catalina/storeconfig/StoreFactoryBase.java
+++ b/java/org/apache/catalina/storeconfig/StoreFactoryBase.java
@@ -169,9 +169,10 @@ public class StoreFactoryBase implements IStoreFactory {
for (Object element : elements) {
try {
storeElement(aWriter, indent, element);
- } catch (IOException ignore) {
- // Ignore. Children do not report the error themselves (e.g.
- // StandardContextSF.storeWithBackup() throws without logging).
+ } catch (IOException ioe) {
+ // Keep storing the other elements but report the failure since some children throw without
+ // logging the error themselves (e.g. StandardContextSF.storeWithBackup())
+ log.warn(sm.getString("factory.storeElementFailed", element.getClass().getName()), ioe);
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 01/15: Fix storeconfig default-manager check always rejecting started managers
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 51202c9bb9d6e2854293b5f9e0ef2e9fcaf964c5
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 9 10:03:54 2026 +0200
Fix storeconfig default-manager check always rejecting started managers
ManagerBase.startInternal() always propagates the Engine's jvmRoute
(null
by default) to the session ID generator, overwriting its empty-string
default. The isDefaultManager() check in ManagerSF compared the
generator's
jvmRoute against "", so it returned false for every started
StandardManager
and storeConfig wrote a redundant <Manager> element with a nested
<SessionIdGenerator> for every context using the default configuration.
Treat a generator jvmRoute that is empty or equal to the value the
manager
propagates as runtime state rather than configuration, and treat a not
yet
created generator as default. Since making the skip path reachable would
otherwise silently drop configuration, require the exact manager and
generator classes and check the persistable throwOnFailure property.
---
.../org/apache/catalina/storeconfig/ManagerSF.java | 52 +++--
.../catalina/storeconfig/TestStoreConfig.java | 249 +++++++++++++++++++++
webapps/docs/changelog.xml | 14 ++
3 files changed, 298 insertions(+), 17 deletions(-)
diff --git a/java/org/apache/catalina/storeconfig/ManagerSF.java b/java/org/apache/catalina/storeconfig/ManagerSF.java
index fd28c92a7f..954aa1e7c6 100644
--- a/java/org/apache/catalina/storeconfig/ManagerSF.java
+++ b/java/org/apache/catalina/storeconfig/ManagerSF.java
@@ -68,11 +68,21 @@ public class ManagerSF extends StoreFactoryBase {
*/
protected boolean isDefaultManager(StandardManager smanager) {
+ // A custom sub-class is configured through its className attribute
+ if (!StandardManager.class.equals(smanager.getClass())) {
+ return false;
+ }
+
// StandardManager-specific property
if (!"SESSIONS.ser".equals(smanager.getPathname())) {
return false;
}
+ // LifecycleBase property
+ if (!smanager.getThrowOnFailure()) {
+ return false;
+ }
+
// ManagerBase properties
if (smanager.getMaxActiveSessions() != -1) {
return false;
@@ -108,25 +118,33 @@ public class ManagerSF extends StoreFactoryBase {
return false;
}
SessionIdGenerator sessionIdGenerator = smanager.getSessionIdGenerator();
- SessionIdGeneratorBase sigBase = null;
- if (sessionIdGenerator == null || !StandardSessionIdGenerator.class.isInstance(sessionIdGenerator)) {
+ if (sessionIdGenerator != null &&
+ !StandardSessionIdGenerator.class.equals(sessionIdGenerator.getClass())) {
return false;
}
- sigBase = (SessionIdGeneratorBase) sessionIdGenerator;
- if (!"".equals(sigBase.getJvmRoute())) {
- return false;
- }
- if (sigBase.getSecureRandomClass() != null) {
- return false;
- }
- if (!SessionIdGeneratorBase.DEFAULT_SECURE_RANDOM_ALGORITHM.equals(sigBase.getSecureRandomAlgorithm())) {
- return false;
- }
- if (sigBase.getSecureRandomProvider() != null) {
- return false;
- }
- if (sigBase.getSessionIdLength() != 16) {
- return false;
+ if (sessionIdGenerator instanceof SessionIdGeneratorBase sigBase) {
+ // The manager propagates its jvmRoute to the generator when it starts,
+ // so a value matching the manager's is runtime state, not configuration
+ String sigJvmRoute = sigBase.getJvmRoute();
+ if (sigJvmRoute != null && !sigJvmRoute.isEmpty() &&
+ !sigJvmRoute.equals(smanager.getJvmRoute())) {
+ return false;
+ }
+ if (!sigBase.getThrowOnFailure()) {
+ return false;
+ }
+ if (sigBase.getSecureRandomClass() != null) {
+ return false;
+ }
+ if (!SessionIdGeneratorBase.DEFAULT_SECURE_RANDOM_ALGORITHM.equals(sigBase.getSecureRandomAlgorithm())) {
+ return false;
+ }
+ if (sigBase.getSecureRandomProvider() != null) {
+ return false;
+ }
+ if (sigBase.getSessionIdLength() != 16) {
+ return false;
+ }
}
return true;
diff --git a/test/org/apache/catalina/storeconfig/TestStoreConfig.java b/test/org/apache/catalina/storeconfig/TestStoreConfig.java
index e63c34a342..383d0b2947 100644
--- a/test/org/apache/catalina/storeconfig/TestStoreConfig.java
+++ b/test/org/apache/catalina/storeconfig/TestStoreConfig.java
@@ -34,11 +34,13 @@ import org.apache.catalina.Host;
import org.apache.catalina.connector.Connector;
import org.apache.catalina.core.StandardContext;
import org.apache.catalina.realm.LockOutRealm;
+import org.apache.catalina.session.StandardManager;
import org.apache.catalina.startup.Catalina;
import org.apache.catalina.startup.CatalinaBaseConfigurationSource;
import org.apache.catalina.startup.Tomcat;
import org.apache.catalina.startup.TomcatBaseTest;
import org.apache.catalina.util.IOTools;
+import org.apache.catalina.util.SessionIdGeneratorBase;
import org.apache.catalina.valves.AccessLogValve;
import org.xml.sax.InputSource;
@@ -315,6 +317,253 @@ public class TestStoreConfig extends TomcatBaseTest {
.parse(new InputSource(new StringReader(contextXmlDump)));
}
+ /**
+ * Verify that the <Manager> element is not stored for a running context using the default StandardManager
+ * configuration. The jvmRoute of the session ID generator is runtime state that the manager propagates from the
+ * Engine when it starts and must not defeat the default detection.
+ *
+ * @throws Exception if the test experiences an unexpected error
+ */
+ @Test
+ public void testDefaultManagerNotStored() throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+ StoreConfigLifecycleListener storeConfigListener = new StoreConfigLifecycleListener();
+ tomcat.getServer().addLifecycleListener(storeConfigListener);
+
+ // Use a storable realm. The default embedded realm (Tomcat.SimpleRealm) is an inner class that the store
+ // path cannot instantiate a default instance of.
+ tomcat.getEngine().setRealm(new LockOutRealm());
+
+ File appDir = new File(getTemporaryDirectory(), "webapps/defmgr");
+ if (!appDir.mkdirs()) {
+ Assert.fail("Unable to create the webapp directory");
+ }
+ Context context = tomcat.addContext("/defmgr", appDir.getAbsolutePath());
+ ((StandardContext) context).setDeployedFromServerXml(true);
+
+ File conf = new File(getTemporaryDirectory(), "conf");
+ if (!conf.isDirectory() && !conf.mkdirs()) {
+ Assert.fail("Unable to create conf directory");
+ }
+ addDeleteOnTearDown(conf);
+
+ tomcat.start();
+
+ // Undo the FastNonSecureRandom configuration the test base applies to every manager, so the started manager
+ // is in the same (default) state as it would be on a regular server
+ StandardManager manager = (StandardManager) ((StandardContext) context).getManager();
+ manager.setSecureRandomClass(null);
+ ((SessionIdGeneratorBase) manager.getSessionIdGenerator()).setSecureRandomClass(null);
+
+ IStoreConfig storeConfig = storeConfigListener.getStoreConfig();
+ StoreDescription desc = storeConfig.getRegistry().findDescription(StandardContext.class);
+ Assert.assertNotNull(desc);
+ boolean oldSeparate = desc.isStoreSeparate();
+ boolean oldExternalAllowed = desc.isExternalAllowed();
+ boolean oldExternalOnly = desc.isExternalOnly();
+ String serverXmlDump;
+ try {
+ desc.setStoreSeparate(true);
+ desc.setExternalAllowed(true);
+ desc.setExternalOnly(false);
+ StringWriter buffer = new StringWriter();
+ storeConfig.store(new PrintWriter(buffer), -2, tomcat.getServer());
+ serverXmlDump = buffer.toString();
+ } finally {
+ desc.setStoreSeparate(oldSeparate);
+ desc.setExternalAllowed(oldExternalAllowed);
+ desc.setExternalOnly(oldExternalOnly);
+ }
+
+ Assert.assertTrue(serverXmlDump, serverXmlDump.contains("path=\"/defmgr\""));
+ // The manager of the started context is at its default configuration, so no <Manager> element may be stored
+ Assert.assertFalse(serverXmlDump, serverXmlDump.contains("<Manager"));
+ Assert.assertFalse(serverXmlDump, serverXmlDump.contains("StandardManager"));
+ Assert.assertFalse(serverXmlDump, serverXmlDump.contains("SessionIdGenerator"));
+ }
+
+ /**
+ * Verify that a <Manager> element is still stored for a context whose StandardManager deviates from the
+ * default configuration.
+ *
+ * @throws Exception if the test experiences an unexpected error
+ */
+ @Test
+ public void testNonDefaultManagerStored() throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+ StoreConfigLifecycleListener storeConfigListener = new StoreConfigLifecycleListener();
+ tomcat.getServer().addLifecycleListener(storeConfigListener);
+
+ // Use a storable realm. The default embedded realm (Tomcat.SimpleRealm) is an inner class that the store
+ // path cannot instantiate a default instance of.
+ tomcat.getEngine().setRealm(new LockOutRealm());
+
+ File appDir = new File(getTemporaryDirectory(), "webapps/nondflmgr");
+ if (!appDir.mkdirs()) {
+ Assert.fail("Unable to create the webapp directory");
+ }
+ Context context = tomcat.addContext("/nondflmgr", appDir.getAbsolutePath());
+ ((StandardContext) context).setDeployedFromServerXml(true);
+ StandardManager manager = new StandardManager();
+ manager.setMaxActiveSessions(100);
+ ((StandardContext) context).setManager(manager);
+
+ File conf = new File(getTemporaryDirectory(), "conf");
+ if (!conf.isDirectory() && !conf.mkdirs()) {
+ Assert.fail("Unable to create conf directory");
+ }
+ addDeleteOnTearDown(conf);
+
+ tomcat.start();
+
+ IStoreConfig storeConfig = storeConfigListener.getStoreConfig();
+ StoreDescription desc = storeConfig.getRegistry().findDescription(StandardContext.class);
+ Assert.assertNotNull(desc);
+ boolean oldSeparate = desc.isStoreSeparate();
+ boolean oldExternalAllowed = desc.isExternalAllowed();
+ boolean oldExternalOnly = desc.isExternalOnly();
+ String serverXmlDump;
+ try {
+ desc.setStoreSeparate(true);
+ desc.setExternalAllowed(true);
+ desc.setExternalOnly(false);
+ StringWriter buffer = new StringWriter();
+ storeConfig.store(new PrintWriter(buffer), -2, tomcat.getServer());
+ serverXmlDump = buffer.toString();
+ } finally {
+ desc.setStoreSeparate(oldSeparate);
+ desc.setExternalAllowed(oldExternalAllowed);
+ desc.setExternalOnly(oldExternalOnly);
+ }
+
+ Assert.assertTrue(serverXmlDump, serverXmlDump.contains("<Manager"));
+ Assert.assertTrue(serverXmlDump, serverXmlDump.contains("maxActiveSessions=\"100\""));
+ }
+
+ /**
+ * Verify that a <Manager> element with a StandardManager sub-class is still stored, since the sub-class is
+ * configured through its class name even when all inherited properties are at their defaults.
+ *
+ * @throws Exception if the test experiences an unexpected error
+ */
+ @Test
+ public void testSubClassManagerStored() throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+ StoreConfigLifecycleListener storeConfigListener = new StoreConfigLifecycleListener();
+ tomcat.getServer().addLifecycleListener(storeConfigListener);
+
+ // Use a storable realm. The default embedded realm (Tomcat.SimpleRealm) is an inner class that the store
+ // path cannot instantiate a default instance of.
+ tomcat.getEngine().setRealm(new LockOutRealm());
+
+ File appDir = new File(getTemporaryDirectory(), "webapps/subcmgr");
+ if (!appDir.mkdirs()) {
+ Assert.fail("Unable to create the webapp directory");
+ }
+ Context context = tomcat.addContext("/subcmgr", appDir.getAbsolutePath());
+ ((StandardContext) context).setDeployedFromServerXml(true);
+ ((StandardContext) context).setManager(new CustomTestManager());
+
+ File conf = new File(getTemporaryDirectory(), "conf");
+ if (!conf.isDirectory() && !conf.mkdirs()) {
+ Assert.fail("Unable to create conf directory");
+ }
+ addDeleteOnTearDown(conf);
+
+ tomcat.start();
+
+ IStoreConfig storeConfig = storeConfigListener.getStoreConfig();
+ StoreDescription desc = storeConfig.getRegistry().findDescription(StandardContext.class);
+ Assert.assertNotNull(desc);
+ boolean oldSeparate = desc.isStoreSeparate();
+ boolean oldExternalAllowed = desc.isExternalAllowed();
+ boolean oldExternalOnly = desc.isExternalOnly();
+ String serverXmlDump;
+ try {
+ desc.setStoreSeparate(true);
+ desc.setExternalAllowed(true);
+ desc.setExternalOnly(false);
+ StringWriter buffer = new StringWriter();
+ storeConfig.store(new PrintWriter(buffer), -2, tomcat.getServer());
+ serverXmlDump = buffer.toString();
+ } finally {
+ desc.setStoreSeparate(oldSeparate);
+ desc.setExternalAllowed(oldExternalAllowed);
+ desc.setExternalOnly(oldExternalOnly);
+ }
+
+ Assert.assertTrue(serverXmlDump, serverXmlDump.contains("<Manager"));
+ Assert.assertTrue(serverXmlDump, serverXmlDump.contains(CustomTestManager.class.getName()));
+ }
+
+ /**
+ * Verify that a <Manager> element is stored when the only deviation from the default configuration is a
+ * non-default <code>throwOnFailure</code> value, which is a persistable property that must not be lost.
+ *
+ * @throws Exception if the test experiences an unexpected error
+ */
+ @Test
+ public void testManagerThrowOnFailureStored() throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+ StoreConfigLifecycleListener storeConfigListener = new StoreConfigLifecycleListener();
+ tomcat.getServer().addLifecycleListener(storeConfigListener);
+
+ // Use a storable realm. The default embedded realm (Tomcat.SimpleRealm) is an inner class that the store
+ // path cannot instantiate a default instance of.
+ tomcat.getEngine().setRealm(new LockOutRealm());
+
+ File appDir = new File(getTemporaryDirectory(), "webapps/tofmgr");
+ if (!appDir.mkdirs()) {
+ Assert.fail("Unable to create the webapp directory");
+ }
+ Context context = tomcat.addContext("/tofmgr", appDir.getAbsolutePath());
+ ((StandardContext) context).setDeployedFromServerXml(true);
+
+ File conf = new File(getTemporaryDirectory(), "conf");
+ if (!conf.isDirectory() && !conf.mkdirs()) {
+ Assert.fail("Unable to create conf directory");
+ }
+ addDeleteOnTearDown(conf);
+
+ tomcat.start();
+
+ // Undo the FastNonSecureRandom configuration the test base applies to every manager, so throwOnFailure is the
+ // only deviation from the default manager configuration
+ StandardManager manager = (StandardManager) ((StandardContext) context).getManager();
+ manager.setSecureRandomClass(null);
+ ((SessionIdGeneratorBase) manager.getSessionIdGenerator()).setSecureRandomClass(null);
+ manager.setThrowOnFailure(false);
+
+ IStoreConfig storeConfig = storeConfigListener.getStoreConfig();
+ StoreDescription desc = storeConfig.getRegistry().findDescription(StandardContext.class);
+ Assert.assertNotNull(desc);
+ boolean oldSeparate = desc.isStoreSeparate();
+ boolean oldExternalAllowed = desc.isExternalAllowed();
+ boolean oldExternalOnly = desc.isExternalOnly();
+ String serverXmlDump;
+ try {
+ desc.setStoreSeparate(true);
+ desc.setExternalAllowed(true);
+ desc.setExternalOnly(false);
+ StringWriter buffer = new StringWriter();
+ storeConfig.store(new PrintWriter(buffer), -2, tomcat.getServer());
+ serverXmlDump = buffer.toString();
+ } finally {
+ desc.setStoreSeparate(oldSeparate);
+ desc.setExternalAllowed(oldExternalAllowed);
+ desc.setExternalOnly(oldExternalOnly);
+ }
+
+ Assert.assertTrue(serverXmlDump, serverXmlDump.contains("<Manager"));
+ Assert.assertTrue(serverXmlDump, serverXmlDump.contains("throwOnFailure=\"false\""));
+ }
+
+ public static class CustomTestManager extends StandardManager {
+
+ public CustomTestManager() {
+ }
+ }
+
/**
* Verify that a Context parsed from a Context element in server.xml is flagged as deployed from server.xml, so
* storeconfig can detect it. The flag must also be set when server.xml is processed through the generated code
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 142dc13400..60377b08f8 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -2124,6 +2124,20 @@
configured with reauthentication enabled. This is so that the delegated
credentials will be available to the web application. (markt)
</fix>
+ <fix>
+ Fix the <code>ManagerSF</code> store configuration factory always
+ treating a started <code>StandardManager</code> as non-default, since
+ the manager propagates its (possibly <code>null</code>) node identifier
+ to its session ID generator when it starts, which the default check
+ compared against an empty string. This caused a redundant
+ <code>Manager</code> element with a nested
+ <code>SessionIdGenerator</code> to be written for every context using
+ the default manager configuration when the running configuration was
+ stored. The default check was strengthened to require the exact
+ manager and session ID generator classes and to test
+ <code>throwOnFailure</code>, so the now reachable skip path cannot
+ silently drop a customised configuration. (remm)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 09/15: Fix use-after-return of pooled send buffer in async sends
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 4656f23c4bb675340ccff2e67ebfe89f6997a00b
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 9 11:18:44 2026 +0200
Fix use-after-return of pooled send buffer in async sends
GroupChannel.send() obtains the message buffer from the BufferPool and
returns it to the pool in a finally block once sendMessage() returns.
MessageDispatchInterceptor queues async messages for deferred sending
and only detached the message from that buffer via a deep clone when
useDeepClone was true, which is the default. With useDeepClone=false,
the original ChannelData wrapping the pooled buffer was queued as-is,
so a concurrent send could clear and refill the buffer before the
dispatch thread serialized the message, silently corrupting cluster
traffic and the queue size accounting.
When useDeepClone is false, queue a shallow ChannelData clone instead,
which copies the message bytes into a new, non pooled buffer while
still avoiding the full re-serialization of the deep clone. Add
TestMessageDispatchInterceptor with a deterministic reproduction and a
check that synchronous sends pass the message through unchanged.
---
.../interceptors/MessageDispatchInterceptor.java | 13 +-
.../TestMessageDispatchInterceptor.java | 141 +++++++++++++++++++++
webapps/docs/changelog.xml | 12 ++
3 files changed, 164 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/catalina/tribes/group/interceptors/MessageDispatchInterceptor.java b/java/org/apache/catalina/tribes/group/interceptors/MessageDispatchInterceptor.java
index f3af235295..ae08a76dd4 100644
--- a/java/org/apache/catalina/tribes/group/interceptors/MessageDispatchInterceptor.java
+++ b/java/org/apache/catalina/tribes/group/interceptors/MessageDispatchInterceptor.java
@@ -30,6 +30,7 @@ import org.apache.catalina.tribes.Member;
import org.apache.catalina.tribes.UniqueId;
import org.apache.catalina.tribes.group.ChannelInterceptorBase;
import org.apache.catalina.tribes.group.InterceptorPayload;
+import org.apache.catalina.tribes.io.ChannelData;
import org.apache.catalina.tribes.util.ExecutorFactory;
import org.apache.catalina.tribes.util.StringManager;
import org.apache.catalina.tribes.util.TcclThreadFactory;
@@ -58,7 +59,8 @@ public class MessageDispatchInterceptor extends ChannelInterceptorBase implement
*/
protected volatile boolean run = false;
/**
- * Whether to use deep clone.
+ * Whether to use deep clone. When false, the message is still detached from the caller's buffer by a shallow
+ * clone before it is queued, but the unique id and address are shared rather than re-serialized.
*/
protected boolean useDeepClone = true;
/**
@@ -113,6 +115,12 @@ public class MessageDispatchInterceptor extends ChannelInterceptorBase implement
// add to queue
if (useDeepClone) {
msg = (ChannelMessage) msg.deepclone();
+ } else if (msg instanceof ChannelData channelData) {
+ // The caller of sendMessage() may reuse the buffer that backs the
+ // message as soon as this method returns, e.g. GroupChannel.send()
+ // returns it to the BufferPool. Clone, which copies the message
+ // data, so the queued message is independent of that buffer.
+ msg = channelData.clone();
}
if (!addToQueue(msg, destination, payload)) {
throw new ChannelException(sm.getString("messageDispatchInterceptor.unableAdd.queue"));
@@ -187,7 +195,8 @@ public class MessageDispatchInterceptor extends ChannelInterceptorBase implement
/**
- * Set whether to use deep clone.
+ * Set whether to use deep clone. When false, the queued message is a shallow clone of the original with the
+ * message bytes copied to a new buffer rather than a fully re-serialized deep clone.
* @param useDeepClone whether to use deep clone
*/
public void setUseDeepClone(boolean useDeepClone) {
diff --git a/test/org/apache/catalina/tribes/group/interceptors/TestMessageDispatchInterceptor.java b/test/org/apache/catalina/tribes/group/interceptors/TestMessageDispatchInterceptor.java
new file mode 100644
index 0000000000..b3b5efd2a4
--- /dev/null
+++ b/test/org/apache/catalina/tribes/group/interceptors/TestMessageDispatchInterceptor.java
@@ -0,0 +1,141 @@
+/*
+ * 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.catalina.tribes.group.interceptors;
+
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.catalina.tribes.Channel;
+import org.apache.catalina.tribes.ChannelException;
+import org.apache.catalina.tribes.ChannelMessage;
+import org.apache.catalina.tribes.Member;
+import org.apache.catalina.tribes.group.ChannelInterceptorBase;
+import org.apache.catalina.tribes.group.GroupChannel;
+import org.apache.catalina.tribes.group.InterceptorPayload;
+import org.apache.catalina.tribes.io.ChannelData;
+import org.apache.catalina.tribes.io.XByteBuffer;
+import org.apache.catalina.tribes.membership.MemberImpl;
+
+public class TestMessageDispatchInterceptor {
+
+ /*
+ * Captures the message on the dispatch thread, but only after the caller thread has simulated the reuse of the
+ * original (pooled) buffer that GroupChannel.send() returns to the BufferPool.
+ */
+ private static class GatedCollector extends ChannelInterceptorBase {
+
+ private final CountDownLatch dispatchStarted = new CountDownLatch(1);
+ private final CountDownLatch bufferReused = new CountDownLatch(1);
+ private final CountDownLatch collected = new CountDownLatch(1);
+
+ private volatile XByteBuffer capturedBuffer;
+ private volatile byte[] capturedData;
+
+ @Override
+ public void sendMessage(Member[] destination, ChannelMessage msg, InterceptorPayload payload)
+ throws ChannelException {
+ dispatchStarted.countDown();
+ try {
+ if (!bufferReused.await(10, TimeUnit.SECONDS)) {
+ throw new ChannelException("Timed out waiting for buffer reuse");
+ }
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new ChannelException(e);
+ }
+ capturedBuffer = msg.getMessage();
+ int len = capturedBuffer.getLength();
+ capturedData = Arrays.copyOfRange(capturedBuffer.getBytesDirect(), 0, len);
+ collected.countDown();
+ }
+ }
+
+ @Test
+ public void testAsyncSendWithoutDeepCloneDetachesPooledBuffer() throws Exception {
+ byte[] original = "ORIGINAL".getBytes(StandardCharsets.UTF_8);
+ byte[] reused = "OVERWRITTEN-BY-POOL".getBytes(StandardCharsets.UTF_8);
+
+ GroupChannel channel = new GroupChannel();
+ MessageDispatchInterceptor interceptor = new MessageDispatchInterceptor();
+ interceptor.setUseDeepClone(false);
+ GatedCollector collector = new GatedCollector();
+ interceptor.setChannel(channel);
+ interceptor.setNext(collector);
+ interceptor.startQueue();
+ try {
+ ChannelData data = new ChannelData(false);
+ data.setOptions(Channel.SEND_OPTIONS_ASYNCHRONOUS);
+ XByteBuffer buffer = new XByteBuffer(original.length + 128, false);
+ buffer.append(original, 0, original.length);
+ data.setMessage(buffer);
+ Member[] destination = new Member[] { new MemberImpl("localhost", 4000, -1) };
+
+ interceptor.sendMessage(destination, data, null);
+ Assert.assertTrue("Dispatch thread did not start",
+ collector.dispatchStarted.await(10, TimeUnit.SECONDS));
+ // Simulate GroupChannel.send() returning the buffer to the pool for reuse
+ buffer.clear();
+ buffer.append(reused, 0, reused.length);
+ collector.bufferReused.countDown();
+ Assert.assertTrue("Dispatch thread did not collect the message",
+ collector.collected.await(10, TimeUnit.SECONDS));
+
+ Assert.assertNotSame("Queued message must not share the pooled buffer",
+ buffer, collector.capturedBuffer);
+ Assert.assertArrayEquals("Queued message content was corrupted by buffer reuse",
+ original, collector.capturedData);
+ } finally {
+ interceptor.stopQueue();
+ }
+ }
+
+ @Test
+ public void testSyncSendPassesMessageThrough() throws Exception {
+ GroupChannel channel = new GroupChannel();
+ MessageDispatchInterceptor interceptor = new MessageDispatchInterceptor();
+ RecordingCollector collector = new RecordingCollector();
+ interceptor.setChannel(channel);
+ interceptor.setNext(collector);
+
+ ChannelData data = new ChannelData(false);
+ XByteBuffer buffer = new XByteBuffer(64, false);
+ byte[] payload = "SYNC".getBytes(StandardCharsets.UTF_8);
+ buffer.append(payload, 0, payload.length);
+ data.setMessage(buffer);
+ Member[] destination = new Member[] { new MemberImpl("localhost", 4000, -1) };
+
+ interceptor.sendMessage(destination, data, null);
+
+ Assert.assertSame("Synchronous sends must pass the original message through", data, collector.message);
+ }
+
+ private static class RecordingCollector extends ChannelInterceptorBase {
+
+ private volatile ChannelMessage message;
+
+ @Override
+ public void sendMessage(Member[] destination, ChannelMessage msg, InterceptorPayload payload)
+ throws ChannelException {
+ message = msg;
+ }
+ }
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 60377b08f8..b556af9e29 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -126,6 +126,18 @@
</fix>
</changelog>
</subsection>
+ <subsection name="Tribes">
+ <changelog>
+ <fix>
+ When the <code>MessageDispatchInterceptor</code> is configured with
+ <code>useDeepClone="false"</code>, asynchronously sent messages are now
+ detached from the caller's buffer with a shallow clone before being
+ queued. Previously, the buffer could be returned to the send buffer pool
+ and reused by a concurrent send while the queued message still
+ referenced it, corrupting the transmitted data. (remm)
+ </fix>
+ </changelog>
+ </subsection>
</section>
<section name="Tomcat 9.0.123 (remm)" rtext="release in progress">
<subsection name="Catalina">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 13/15: Make RpcMessage serialization tolerate null identifiers
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 6d85b214c85ee33931ce051c193b9768b1671cf0
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 9 11:52:24 2026 +0200
Make RpcMessage serialization tolerate null identifiers
The public RpcMessage constructor accepts null rpcId and uuid arrays,
but writeExternal dereferenced them, so serializing such a message
failed with a NullPointerException. The same package already treats
null byte arrays as a supported input in ByteMessage, which writes a
zero length and skips the bytes.
Follow that pattern in the writeExternal methods of RpcMessage and
NoRpcChannelReply. Streams with non-null arrays are unchanged byte for
byte, and zero lengths are read back by readExternal as zero-length
arrays. Document the normalization in the constructor. Add
TestRpcMessage covering normal, null and mixed null round-trips.
---
.../apache/catalina/tribes/group/RpcMessage.java | 27 ++++---
.../catalina/tribes/group/TestRpcMessage.java | 88 ++++++++++++++++++++++
2 files changed, 106 insertions(+), 9 deletions(-)
diff --git a/java/org/apache/catalina/tribes/group/RpcMessage.java b/java/org/apache/catalina/tribes/group/RpcMessage.java
index 574e661f66..7a4dfe4498 100644
--- a/java/org/apache/catalina/tribes/group/RpcMessage.java
+++ b/java/org/apache/catalina/tribes/group/RpcMessage.java
@@ -57,7 +57,8 @@ public class RpcMessage implements Externalizable {
}
/**
- * Create a new RpcMessage.
+ * Create a new RpcMessage. A null identifier is serialized as a zero-length
+ * array and deserialized back into one.
*
* @param rpcId The RPC channel identifier
* @param uuid The unique identifier for this message
@@ -84,10 +85,14 @@ public class RpcMessage implements Externalizable {
@Override
public void writeExternal(ObjectOutput out) throws IOException {
out.writeBoolean(reply);
- out.writeInt(uuid.length);
- out.write(uuid, 0, uuid.length);
- out.writeInt(rpcId.length);
- out.write(rpcId, 0, rpcId.length);
+ out.writeInt(uuid != null ? uuid.length : 0);
+ if (uuid != null) {
+ out.write(uuid, 0, uuid.length);
+ }
+ out.writeInt(rpcId != null ? rpcId.length : 0);
+ if (rpcId != null) {
+ out.write(rpcId, 0, rpcId.length);
+ }
out.writeObject(message);
}
@@ -132,10 +137,14 @@ public class RpcMessage implements Externalizable {
@Override
public void writeExternal(ObjectOutput out) throws IOException {
- out.writeInt(uuid.length);
- out.write(uuid, 0, uuid.length);
- out.writeInt(rpcId.length);
- out.write(rpcId, 0, rpcId.length);
+ out.writeInt(uuid != null ? uuid.length : 0);
+ if (uuid != null) {
+ out.write(uuid, 0, uuid.length);
+ }
+ out.writeInt(rpcId != null ? rpcId.length : 0);
+ if (rpcId != null) {
+ out.write(rpcId, 0, rpcId.length);
+ }
}
}
diff --git a/test/org/apache/catalina/tribes/group/TestRpcMessage.java b/test/org/apache/catalina/tribes/group/TestRpcMessage.java
new file mode 100644
index 0000000000..37e78391c0
--- /dev/null
+++ b/test/org/apache/catalina/tribes/group/TestRpcMessage.java
@@ -0,0 +1,88 @@
+/*
+ * 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.catalina.tribes.group;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.nio.charset.StandardCharsets;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.catalina.tribes.group.RpcMessage.NoRpcChannelReply;
+
+public class TestRpcMessage {
+
+ private static Object roundTrip(Object original) throws Exception {
+ ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+ try (ObjectOutputStream oos = new ObjectOutputStream(bytes)) {
+ oos.writeObject(original);
+ }
+ try (ObjectInputStream ois = new ObjectInputStream(new ByteArrayInputStream(bytes.toByteArray()))) {
+ return ois.readObject();
+ }
+ }
+
+ @Test
+ public void testRoundTrip() throws Exception {
+ RpcMessage original = new RpcMessage(new byte[] { 1 }, new byte[] { 2 }, "PAYLOAD");
+ RpcMessage copy = (RpcMessage) roundTrip(original);
+ Assert.assertArrayEquals(new byte[] { 1 }, copy.rpcId);
+ Assert.assertArrayEquals(new byte[] { 2 }, copy.uuid);
+ Assert.assertEquals("PAYLOAD", copy.message);
+ Assert.assertFalse(copy.reply);
+ }
+
+ @Test
+ public void testNullArraysDoNotBreakSerialization() throws Exception {
+ RpcMessage original = new RpcMessage(null, null, "PAYLOAD");
+ RpcMessage copy = (RpcMessage) roundTrip(original);
+ Assert.assertEquals(0, copy.rpcId.length);
+ Assert.assertEquals(0, copy.uuid.length);
+ Assert.assertEquals("PAYLOAD", copy.message);
+ }
+
+ @Test
+ public void testMixedNullArraysDoNotBreakSerialization() throws Exception {
+ RpcMessage original = new RpcMessage(new byte[] { 1 }, null, "PAYLOAD");
+ RpcMessage copy = (RpcMessage) roundTrip(original);
+ Assert.assertArrayEquals(new byte[] { 1 }, copy.rpcId);
+ Assert.assertEquals(0, copy.uuid.length);
+ Assert.assertEquals("PAYLOAD", copy.message);
+ }
+
+ @Test
+ public void testNoRpcChannelReplyNullArraysDoNotBreakSerialization() throws Exception {
+ NoRpcChannelReply original = new NoRpcChannelReply(null, null);
+ NoRpcChannelReply copy = (NoRpcChannelReply) roundTrip(original);
+ Assert.assertEquals(0, copy.rpcId.length);
+ Assert.assertEquals(0, copy.uuid.length);
+ Assert.assertTrue(copy.reply);
+ }
+
+ @Test
+ public void testNoRpcChannelReplyRoundTrip() throws Exception {
+ byte[] id = "id".getBytes(StandardCharsets.UTF_8);
+ NoRpcChannelReply original = new NoRpcChannelReply(id, id);
+ NoRpcChannelReply copy = (NoRpcChannelReply) roundTrip(original);
+ Assert.assertArrayEquals(id, copy.rpcId);
+ Assert.assertArrayEquals(id, copy.uuid);
+ Assert.assertTrue(copy.reply);
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 11/15: Do not send an RPC reply when the callback returns null
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 51472725f2e4d6120ad68c2f6069d7cca0160922
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 9 11:34:41 2026 +0200
Do not send an RPC reply when the callback returns null
RpcCallback.replyRequest is documented to return null if no reply
should be sent, but RpcChannel.messageReceived ignored that and sent
a reply message carrying the null payload. The requester then received
a Response with a null message, which could complete first reply or
majority reply collectors early with an empty payload or consume a
slot of an all reply collector.
Return without sending anything when the callback returns null, and
notify an ExtendedRpcCallback of the completion synchronously, passing
the original request message and a null response. Add TestRpcChannel
covering the no-reply, synchronous notification, asynchronous silent
and regular reply paths.
---
.../apache/catalina/tribes/group/RpcChannel.java | 8 +
.../catalina/tribes/group/TestRpcChannel.java | 163 +++++++++++++++++++++
2 files changed, 171 insertions(+)
diff --git a/java/org/apache/catalina/tribes/group/RpcChannel.java b/java/org/apache/catalina/tribes/group/RpcChannel.java
index 782afc7341..af1b50bcf4 100644
--- a/java/org/apache/catalina/tribes/group/RpcChannel.java
+++ b/java/org/apache/catalina/tribes/group/RpcChannel.java
@@ -170,6 +170,14 @@ public class RpcChannel implements ChannelListener {
boolean asyncReply =
((replyMessageOptions & Channel.SEND_OPTIONS_ASYNCHRONOUS) == Channel.SEND_OPTIONS_ASYNCHRONOUS);
Serializable reply = callback.replyRequest(rmsg.message, sender);
+ if (reply == null) {
+ // The callback is documented to return null when no reply
+ // should be sent
+ if (excallback != null && !asyncReply) {
+ excallback.replySucceeded(msg, null, sender);
+ }
+ return;
+ }
ErrorHandler handler = null;
final Serializable request = msg;
final Serializable response = reply;
diff --git a/test/org/apache/catalina/tribes/group/TestRpcChannel.java b/test/org/apache/catalina/tribes/group/TestRpcChannel.java
new file mode 100644
index 0000000000..0967b84b3a
--- /dev/null
+++ b/test/org/apache/catalina/tribes/group/TestRpcChannel.java
@@ -0,0 +1,163 @@
+/*
+ * 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.catalina.tribes.group;
+
+import java.io.Serializable;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.catalina.tribes.Channel;
+import org.apache.catalina.tribes.ChannelException;
+import org.apache.catalina.tribes.ErrorHandler;
+import org.apache.catalina.tribes.Member;
+import org.apache.catalina.tribes.UniqueId;
+import org.apache.catalina.tribes.membership.MemberImpl;
+
+public class TestRpcChannel {
+
+ private static class RecordingChannel extends GroupChannel {
+
+ private final List<Serializable> sent = new ArrayList<>();
+
+ @Override
+ public UniqueId send(Member[] destination, Serializable msg, int options) throws ChannelException {
+ sent.add(msg);
+ return null;
+ }
+
+ @Override
+ public UniqueId send(Member[] destination, Serializable msg, int options, ErrorHandler handler)
+ throws ChannelException {
+ sent.add(msg);
+ return null;
+ }
+ }
+
+ private static class FixedCallback implements RpcCallback {
+
+ private final Serializable reply;
+
+ FixedCallback(Serializable reply) {
+ this.reply = reply;
+ }
+
+ @Override
+ public Serializable replyRequest(Serializable msg, Member sender) {
+ return reply;
+ }
+
+ @Override
+ public void leftOver(Serializable msg, Member sender) {
+ // NO-OP
+ }
+ }
+
+ private RpcChannel createRpcChannel(RecordingChannel channel, RpcCallback callback) {
+ return new RpcChannel(new byte[] { 1 }, channel, callback);
+ }
+
+ @Test
+ public void testNullReplySendsNothing() throws Exception {
+ RecordingChannel channel = new RecordingChannel();
+ RpcChannel rpcChannel = createRpcChannel(channel, new FixedCallback(null));
+ Member sender = new MemberImpl("localhost", 4000, -1);
+ RpcMessage request =
+ new RpcMessage(rpcChannel.getRpcId(), "uuid".getBytes(StandardCharsets.UTF_8), "REQUEST");
+
+ rpcChannel.messageReceived(request, sender);
+
+ Assert.assertTrue("A null reply means no reply should be sent", channel.sent.isEmpty());
+ }
+
+ private static class RecordingExtendedCallback extends FixedCallback implements ExtendedRpcCallback {
+
+ private volatile int succeededCount;
+ private volatile Serializable succeededRequest;
+ private volatile Serializable succeededResponse;
+
+ RecordingExtendedCallback(Serializable reply) {
+ super(reply);
+ }
+
+ @Override
+ public void replyFailed(Serializable request, Serializable response, Member sender, Exception reason) {
+ // NO-OP
+ }
+
+ @Override
+ public void replySucceeded(Serializable request, Serializable response, Member sender) {
+ succeededRequest = request;
+ succeededResponse = response;
+ succeededCount++;
+ }
+ }
+
+ @Test
+ public void testNullReplySynchronousExtendedCallbackNotified() throws Exception {
+ RecordingChannel channel = new RecordingChannel();
+ RecordingExtendedCallback callback = new RecordingExtendedCallback(null);
+ RpcChannel rpcChannel = createRpcChannel(channel, callback);
+ Member sender = new MemberImpl("localhost", 4000, -1);
+ RpcMessage request =
+ new RpcMessage(rpcChannel.getRpcId(), "uuid".getBytes(StandardCharsets.UTF_8), "REQUEST");
+
+ rpcChannel.messageReceived(request, sender);
+
+ Assert.assertTrue(channel.sent.isEmpty());
+ Assert.assertEquals(1, callback.succeededCount);
+ Assert.assertSame(request, callback.succeededRequest);
+ Assert.assertNull(callback.succeededResponse);
+ }
+
+ @Test
+ public void testNullReplyAsynchronousExtendedCallbackNotNotified() throws Exception {
+ RecordingChannel channel = new RecordingChannel();
+ RecordingExtendedCallback callback = new RecordingExtendedCallback(null);
+ RpcChannel rpcChannel = createRpcChannel(channel, callback);
+ rpcChannel.setReplyMessageOptions(Channel.SEND_OPTIONS_ASYNCHRONOUS);
+ Member sender = new MemberImpl("localhost", 4000, -1);
+ RpcMessage request =
+ new RpcMessage(rpcChannel.getRpcId(), "uuid".getBytes(StandardCharsets.UTF_8), "REQUEST");
+
+ rpcChannel.messageReceived(request, sender);
+
+ Assert.assertTrue(channel.sent.isEmpty());
+ Assert.assertEquals(0, callback.succeededCount);
+ }
+
+ @Test
+ public void testNonNullReplyIsSent() throws Exception {
+ RecordingChannel channel = new RecordingChannel();
+ RpcChannel rpcChannel = createRpcChannel(channel, new FixedCallback("RESPONSE"));
+ Member sender = new MemberImpl("localhost", 4000, -1);
+ RpcMessage request =
+ new RpcMessage(rpcChannel.getRpcId(), "uuid".getBytes(StandardCharsets.UTF_8), "REQUEST");
+
+ rpcChannel.messageReceived(request, sender);
+
+ Assert.assertEquals(1, channel.sent.size());
+ Object sent = channel.sent.get(0);
+ Assert.assertTrue(sent instanceof RpcMessage);
+ RpcMessage reply = (RpcMessage) sent;
+ Assert.assertTrue(reply.reply);
+ Assert.assertEquals("RESPONSE", reply.message);
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 03/15: Do not store the runtime placeholder UNDEFINED Certificate element
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 3f55f914fee2248e835935537b98712c607683a9
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 9 10:22:46 2026 +0200
Do not store the runtime placeholder UNDEFINED Certificate element
When an SSL host configuration contains no Certificate element, the
endpoint registers a bare UNDEFINED-type placeholder certificate when it
creates the SSL context. SSLHostConfigSF only removed UNDEFINED
certificates when more than one was present, a case that can never occur
since addCertificate() rejects mixing UNDEFINED with typed certificates,
so the single placeholder was stored as a <Certificate
type="UNDEFINED"/> element. Adding a typed certificate to the saved file
then made the next start fail because only one certificate may use the
undefined type.
Expose the placeholder through SSLHostConfig.getDefaultCertificate() and
skip that exact instance when storing. An explicitly configured,
type-less certificate remains stored since it is a distinct instance
that
the administrator wrote.
---
.../catalina/storeconfig/SSLHostConfigSF.java | 9 ++---
java/org/apache/tomcat/util/net/SSLHostConfig.java | 10 ++++++
.../catalina/storeconfig/TestStoreConfig.java | 39 ++++++++++++++++++++++
3 files changed, 54 insertions(+), 4 deletions(-)
diff --git a/java/org/apache/catalina/storeconfig/SSLHostConfigSF.java b/java/org/apache/catalina/storeconfig/SSLHostConfigSF.java
index d26d82483f..f672ccb70a 100644
--- a/java/org/apache/catalina/storeconfig/SSLHostConfigSF.java
+++ b/java/org/apache/catalina/storeconfig/SSLHostConfigSF.java
@@ -23,7 +23,6 @@ import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.net.SSLHostConfig;
import org.apache.tomcat.util.net.SSLHostConfigCertificate;
-import org.apache.tomcat.util.net.SSLHostConfigCertificate.Type;
import org.apache.tomcat.util.net.openssl.OpenSSLConf;
import org.apache.tomcat.util.net.openssl.OpenSSLConfCmd;
@@ -88,11 +87,13 @@ public class SSLHostConfigSF extends StoreFactoryBase {
// Store nested <SSLHostConfigCertificate> elements
SSLHostConfigCertificate[] hostConfigsCertificates =
sslHostConfig.getCertificates().toArray(new SSLHostConfigCertificate[0]);
- // Remove a possible default UNDEFINED certificate
- if (hostConfigsCertificates.length > 1) {
+ // Remove the runtime placeholder certificate that was created when
+ // no certificate was configured
+ SSLHostConfigCertificate defaultCertificate = sslHostConfig.getDefaultCertificate();
+ if (defaultCertificate != null) {
ArrayList<SSLHostConfigCertificate> certificates = new ArrayList<>();
for (SSLHostConfigCertificate certificate : hostConfigsCertificates) {
- if (Type.UNDEFINED != certificate.getType()) {
+ if (certificate != defaultCertificate) {
certificates.add(certificate);
}
}
diff --git a/java/org/apache/tomcat/util/net/SSLHostConfig.java b/java/org/apache/tomcat/util/net/SSLHostConfig.java
index 2bbaf240c4..43220880fc 100644
--- a/java/org/apache/tomcat/util/net/SSLHostConfig.java
+++ b/java/org/apache/tomcat/util/net/SSLHostConfig.java
@@ -575,6 +575,16 @@ public class SSLHostConfig implements Serializable {
}
+ /**
+ * Returns the placeholder certificate that was created at runtime because no certificate was configured, if any.
+ *
+ * @return the runtime placeholder certificate or {@code null} if none was created
+ */
+ public SSLHostConfigCertificate getDefaultCertificate() {
+ return defaultCertificate;
+ }
+
+
// ----------------------------------------- Common configuration properties
// TODO: This certificate setter can be removed once it is no longer
diff --git a/test/org/apache/catalina/storeconfig/TestStoreConfig.java b/test/org/apache/catalina/storeconfig/TestStoreConfig.java
index faba0f337d..cfb0de62ee 100644
--- a/test/org/apache/catalina/storeconfig/TestStoreConfig.java
+++ b/test/org/apache/catalina/storeconfig/TestStoreConfig.java
@@ -43,6 +43,7 @@ import org.apache.catalina.util.IOTools;
import org.apache.catalina.util.SessionIdGeneratorBase;
import org.apache.catalina.valves.AccessLogValve;
import org.apache.tomcat.util.net.SSLHostConfig;
+import org.apache.tomcat.util.net.SSLHostConfigCertificate;
import org.apache.tomcat.util.net.openssl.OpenSSLConf;
import org.apache.tomcat.util.net.openssl.OpenSSLConfCmd;
import org.xml.sax.InputSource;
@@ -605,6 +606,44 @@ public class TestStoreConfig extends TomcatBaseTest {
Assert.assertTrue(dump, dump.contains("name=\"groups\""));
}
+ /**
+ * Verify that the placeholder certificate of type UNDEFINED that is created at runtime when no certificate is
+ * configured is not stored. The placeholder would fail the start of a server when a typed certificate is added to
+ * the stored configuration later. An explicitly configured certificate of type UNDEFINED must be kept since the
+ * filter matches the runtime registered instance.
+ *
+ * @throws Exception if the test experiences an unexpected error
+ */
+ @Test
+ public void testDefaultCertificatePlaceholderNotStored() throws Exception {
+ StoreLoader loader = new StoreLoader();
+ loader.load(null);
+ StoreRegistry registry = loader.getRegistry();
+ StoreDescription desc = registry.findDescription(SSLHostConfig.class);
+ Assert.assertNotNull(desc);
+
+ // The placeholder registered at runtime when the endpoint creates the SSL context
+ SSLHostConfig placeholder = new SSLHostConfig();
+ placeholder.getCertificates(true);
+ String dump = storeToString(desc, placeholder);
+ Assert.assertTrue(dump, dump.contains("<SSLHostConfig"));
+ Assert.assertFalse(dump, dump.contains("<Certificate"));
+
+ // An explicitly configured, type-less certificate must be kept
+ SSLHostConfig explicit = new SSLHostConfig();
+ explicit.addCertificate(new SSLHostConfigCertificate(explicit, SSLHostConfigCertificate.Type.UNDEFINED));
+ dump = storeToString(desc, explicit);
+ Assert.assertTrue(dump, dump.contains("<Certificate"));
+ Assert.assertTrue(dump, dump.contains("type=\"UNDEFINED\""));
+
+ // Typed certificates are unaffected
+ SSLHostConfig typed = new SSLHostConfig();
+ typed.addCertificate(new SSLHostConfigCertificate(typed, SSLHostConfigCertificate.Type.EC));
+ dump = storeToString(desc, typed);
+ Assert.assertTrue(dump, dump.contains("<Certificate"));
+ Assert.assertTrue(dump, dump.contains("type=\"EC\""));
+ }
+
private static String storeToString(StoreDescription desc, Object element) throws Exception {
StringWriter buffer = new StringWriter();
desc.getStoreFactory().store(new PrintWriter(buffer), -2, element);
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 14/15: Java compat
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 6e3827031fda6642c133acccc3b2cd75505a2d82
Author: remm <re...@apache.org>
AuthorDate: Fri Oct 9 12:35:36 2026 +0200
Java compat
---
java/org/apache/catalina/storeconfig/ManagerSF.java | 3 ++-
.../tribes/group/interceptors/MessageDispatchInterceptor.java | 4 ++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/catalina/storeconfig/ManagerSF.java b/java/org/apache/catalina/storeconfig/ManagerSF.java
index 954aa1e7c6..22062e6547 100644
--- a/java/org/apache/catalina/storeconfig/ManagerSF.java
+++ b/java/org/apache/catalina/storeconfig/ManagerSF.java
@@ -122,7 +122,8 @@ public class ManagerSF extends StoreFactoryBase {
!StandardSessionIdGenerator.class.equals(sessionIdGenerator.getClass())) {
return false;
}
- if (sessionIdGenerator instanceof SessionIdGeneratorBase sigBase) {
+ if (sessionIdGenerator instanceof SessionIdGeneratorBase) {
+ SessionIdGeneratorBase sigBase = (SessionIdGeneratorBase) sessionIdGenerator;
// The manager propagates its jvmRoute to the generator when it starts,
// so a value matching the manager's is runtime state, not configuration
String sigJvmRoute = sigBase.getJvmRoute();
diff --git a/java/org/apache/catalina/tribes/group/interceptors/MessageDispatchInterceptor.java b/java/org/apache/catalina/tribes/group/interceptors/MessageDispatchInterceptor.java
index ae08a76dd4..129f7c6d88 100644
--- a/java/org/apache/catalina/tribes/group/interceptors/MessageDispatchInterceptor.java
+++ b/java/org/apache/catalina/tribes/group/interceptors/MessageDispatchInterceptor.java
@@ -115,12 +115,12 @@ public class MessageDispatchInterceptor extends ChannelInterceptorBase implement
// add to queue
if (useDeepClone) {
msg = (ChannelMessage) msg.deepclone();
- } else if (msg instanceof ChannelData channelData) {
+ } else if (msg instanceof ChannelData) {
// The caller of sendMessage() may reuse the buffer that backs the
// message as soon as this method returns, e.g. GroupChannel.send()
// returns it to the BufferPool. Clone, which copies the message
// data, so the queued message is independent of that buffer.
- msg = channelData.clone();
+ msg = ((ChannelData) msg).clone();
}
if (!addToQueue(msg, destination, payload)) {
throw new ChannelException(sm.getString("messageDispatchInterceptor.unableAdd.queue"));
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 10/15: Do not re-start already started services on overlapping starts
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 5c1ac02fd8151dbd85ba1f8e139d45fd4466d42f
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 9 11:26:58 2026 +0200
Do not re-start already started services on overlapping starts
ChannelCoordinator.internalStart() only rejected a start request when
all of the requested service flags were already started. For a request
that partially overlapped with the started services, e.g. a start of
Channel.DEFAULT after a start of Channel.SND_RX_SEQ, the already
started services were started a second time. For the data receiver,
that recreated the task pool and bound a second socket; for the
membership receiver, it failed with an IllegalStateException, leaving
the channel in a partially started state.
Compute the flags that are not started yet and drive the individual
service start blocks from those, keeping the existing behavior of
throwing when the whole request has already been started. Add
testOverlappingPartialStart to cover the overlapping case.
---
.../catalina/tribes/group/ChannelCoordinator.java | 14 +++++++++-----
.../tribes/group/TestGroupChannelStartStop.java | 21 +++++++++++++++++++++
2 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/java/org/apache/catalina/tribes/group/ChannelCoordinator.java b/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
index fb91d76997..b9409f88f5 100644
--- a/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
+++ b/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
@@ -152,13 +152,17 @@ public class ChannelCoordinator extends ChannelInterceptorBase implements Messag
return;// nothing to start
}
- if (svc == (svc & startLevel)) {
+ // only start the services that are not started yet, so that a request
+ // that partially overlaps with the started services does not start the
+ // already started ones a second time
+ int toStart = svc & ~startLevel;
+ if (toStart == 0) {
throw new ChannelException(sm.getString("channelCoordinator.alreadyStarted", Integer.toString(svc)));
}
// must start the receiver first so that we can coordinate the port it
// listens to with the local membership settings
- if (Channel.SND_RX_SEQ == (svc & Channel.SND_RX_SEQ)) {
+ if (Channel.SND_RX_SEQ == (toStart & Channel.SND_RX_SEQ)) {
clusterReceiver.setMessageListener(this);
clusterReceiver.setChannel(getChannel());
clusterReceiver.start();
@@ -178,13 +182,13 @@ public class ChannelCoordinator extends ChannelInterceptorBase implements Messag
}
valid = true;
}
- if (Channel.SND_TX_SEQ == (svc & Channel.SND_TX_SEQ)) {
+ if (Channel.SND_TX_SEQ == (toStart & Channel.SND_TX_SEQ)) {
clusterSender.setChannel(getChannel());
clusterSender.start();
valid = true;
}
- if (Channel.MBR_RX_SEQ == (svc & Channel.MBR_RX_SEQ)) {
+ if (Channel.MBR_RX_SEQ == (toStart & Channel.MBR_RX_SEQ)) {
membershipService.setMembershipListener(this);
membershipService.setChannel(getChannel());
if (membershipService instanceof McastService) {
@@ -193,7 +197,7 @@ public class ChannelCoordinator extends ChannelInterceptorBase implements Messag
membershipService.start(MembershipService.MBR_RX);
valid = true;
}
- if (Channel.MBR_TX_SEQ == (svc & Channel.MBR_TX_SEQ)) {
+ if (Channel.MBR_TX_SEQ == (toStart & Channel.MBR_TX_SEQ)) {
membershipService.setChannel(getChannel());
membershipService.start(MembershipService.MBR_TX);
valid = true;
diff --git a/test/org/apache/catalina/tribes/group/TestGroupChannelStartStop.java b/test/org/apache/catalina/tribes/group/TestGroupChannelStartStop.java
index 584b86e5e5..48bc9631f2 100644
--- a/test/org/apache/catalina/tribes/group/TestGroupChannelStartStop.java
+++ b/test/org/apache/catalina/tribes/group/TestGroupChannelStartStop.java
@@ -126,6 +126,27 @@ public class TestGroupChannelStartStop {
channel.stop(Channel.DEFAULT);
}
+ @Test
+ public void testOverlappingPartialStart() throws Exception {
+ // start one service, then start everything: the already
+ // started service must not be started a second time
+ ReceiverBase rb = (ReceiverBase) channel.getChannelReceiver();
+ rb.setPort(45588);
+ // one bind attempt only, so a second start on the same port fails
+ rb.setAutoBind(0);
+ int count = 0;
+ try {
+ channel.start(Channel.SND_RX_SEQ);
+ count++;
+ } catch ( Exception x){x.printStackTrace();}
+ try {
+ channel.start(Channel.DEFAULT);
+ count++;
+ } catch ( Exception x){x.printStackTrace();}
+ Assert.assertEquals(2,count);
+ channel.stop(Channel.DEFAULT);
+ }
+
@Test
public void testFalseOption() throws Exception {
int flag = 0xFFF0;//should get ignored by the underlying components
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 04/15: Report failure when storing a context class without store description
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 0edf7f4a071098a11632e15ff0f8a1342e31e0a8
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 9 10:29:21 2026 +0200
Report failure when storing a context class without store description
StoreConfig.store(Context) is the MBean operation behind the Manager
webapp "save" action. It returned true even when the store registry had
no description for the context's class and nothing was stored, so the
manager reported the configuration as saved when it was not. The
registry lookup is keyed by the exact class with only a limited set of
interfaces as fallback, and Context is not among them, so any custom
context implementation hit this path.
Return false and log a factory.storeNoDescriptor warning when the
description is missing, and log the same warning in the
storeContext(ObjectName,...) operation which silently did nothing in
that case.
---
.../org/apache/catalina/storeconfig/StoreConfig.java | 20 ++++++++++++--------
.../apache/catalina/storeconfig/TestStoreConfig.java | 20 ++++++++++++++++++++
2 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/java/org/apache/catalina/storeconfig/StoreConfig.java b/java/org/apache/catalina/storeconfig/StoreConfig.java
index bb861281db..8290e02082 100644
--- a/java/org/apache/catalina/storeconfig/StoreConfig.java
+++ b/java/org/apache/catalina/storeconfig/StoreConfig.java
@@ -185,6 +185,8 @@ public class StoreConfig implements IStoreConfig {
desc.setBackup(oldBackup);
desc.setExternalAllowed(oldExternalAllowed);
}
+ } else {
+ log.warn(sm.getString("factory.storeNoDescriptor", aContext.getClass()));
}
} else {
log.error(sm.getString("config.missingContextFile", aContext.getPath()));
@@ -223,14 +225,16 @@ public class StoreConfig implements IStoreConfig {
public synchronized boolean store(Context aContext) {
try {
StoreDescription desc = getRegistry().findDescription(aContext.getClass());
- if (desc != null) {
- boolean old = desc.isStoreSeparate();
- try {
- desc.setStoreSeparate(true);
- desc.getStoreFactory().store(null, -2, aContext);
- } finally {
- desc.setStoreSeparate(old);
- }
+ if (desc == null) {
+ log.warn(sm.getString("factory.storeNoDescriptor", aContext.getClass()));
+ return false;
+ }
+ boolean old = desc.isStoreSeparate();
+ try {
+ desc.setStoreSeparate(true);
+ desc.getStoreFactory().store(null, -2, aContext);
+ } finally {
+ desc.setStoreSeparate(old);
}
return true;
} catch (Exception e) {
diff --git a/test/org/apache/catalina/storeconfig/TestStoreConfig.java b/test/org/apache/catalina/storeconfig/TestStoreConfig.java
index cfb0de62ee..5f922cdd2c 100644
--- a/test/org/apache/catalina/storeconfig/TestStoreConfig.java
+++ b/test/org/apache/catalina/storeconfig/TestStoreConfig.java
@@ -644,6 +644,26 @@ public class TestStoreConfig extends TomcatBaseTest {
Assert.assertTrue(dump, dump.contains("type=\"EC\""));
}
+ /**
+ * Verify that storing a Context whose class has no store description is reported as a failure rather than as a
+ * success, since nothing is persisted for it.
+ *
+ * @throws Exception if the test experiences an unexpected error
+ */
+ @Test
+ public void testStoreContextWithoutDescriptionFails() throws Exception {
+ StoreLoader loader = new StoreLoader();
+ loader.load(null);
+ StoreConfig storeConfig = new StoreConfig();
+ storeConfig.setRegistry(loader.getRegistry());
+
+ Context context = new StandardContext() {
+ // Sub-class that the store registry does not know
+ };
+ Assert.assertNull(loader.getRegistry().findDescription(context.getClass()));
+ Assert.assertFalse(storeConfig.store(context));
+ }
+
private static String storeToString(StoreDescription desc, Object element) throws Exception {
StringWriter buffer = new StringWriter();
desc.getStoreFactory().store(new PrintWriter(buffer), -2, element);
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 15/15: Drop test
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit bd100b33357b34efca54b1daf4540b3dbc32c968
Author: remm <re...@apache.org>
AuthorDate: Fri Oct 9 12:42:23 2026 +0200
Drop test
---
.../catalina/storeconfig/TestStoreFileMover.java | 118 ---------------------
1 file changed, 118 deletions(-)
diff --git a/test/org/apache/catalina/storeconfig/TestStoreFileMover.java b/test/org/apache/catalina/storeconfig/TestStoreFileMover.java
deleted file mode 100644
index e09661bce5..0000000000
--- a/test/org/apache/catalina/storeconfig/TestStoreFileMover.java
+++ /dev/null
@@ -1,118 +0,0 @@
-/*
- * 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.catalina.storeconfig;
-
-import java.io.IOException;
-import java.io.PrintWriter;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.util.Comparator;
-import java.util.stream.Stream;
-
-import org.junit.Assert;
-import org.junit.Test;
-
-public class TestStoreFileMover {
-
- /**
- * Verify the regular swap: the new content is installed, the old content is preserved in the backup and the
- * intermediate file is consumed.
- *
- * @throws Exception if the test experiences an unexpected error
- */
- @Test
- public void testMoveWithBackup() throws Exception {
- Path dir = Files.createTempDirectory("storeFileMover");
- try {
- Files.writeString(dir.resolve("server.xml"), "old");
- StoreFileMover mover = new StoreFileMover(dir.toString(), "server.xml", "UTF-8");
- try (PrintWriter writer = mover.getWriter()) {
- writer.print("new");
- }
- mover.move();
- Assert.assertEquals("new", Files.readString(dir.resolve("server.xml")));
- Assert.assertFalse(Files.exists(dir.resolve("server.xml.new")));
- Path backup = mover.getConfigSave().toPath();
- Assert.assertTrue(Files.exists(backup));
- Assert.assertEquals("old", Files.readString(backup));
- } finally {
- deleteRecursively(dir);
- }
- }
-
- /**
- * Verify that without an existing configuration file, the new file is installed directly without creating a
- * backup.
- *
- * @throws Exception if the test experiences an unexpected error
- */
- @Test
- public void testMoveWithoutExistingConfiguration() throws Exception {
- Path dir = Files.createTempDirectory("storeFileMover");
- try {
- StoreFileMover mover = new StoreFileMover(dir.toString(), "server.xml", "UTF-8");
- try (PrintWriter writer = mover.getWriter()) {
- writer.print("new");
- }
- mover.move();
- Assert.assertEquals("new", Files.readString(dir.resolve("server.xml")));
- Assert.assertFalse(Files.exists(mover.getConfigSave().toPath()));
- } finally {
- deleteRecursively(dir);
- }
- }
-
- /**
- * Verify that a failure to create the backup leaves the active configuration file in place and unchanged. The
- * backup is copied rather than renamed away, so no failure during the swap can leave the configuration file
- * missing.
- *
- * @throws Exception if the test experiences an unexpected error
- */
- @Test
- public void testBackupFailureLeavesConfigurationInPlace() throws Exception {
- Path dir = Files.createTempDirectory("storeFileMover");
- try {
- Files.writeString(dir.resolve("server.xml"), "old");
- StoreFileMover mover = new StoreFileMover(dir.toString(), "server.xml", "UTF-8");
- try (PrintWriter writer = mover.getWriter()) {
- writer.print("new");
- }
- // Block the backup location so that the backup copy fails
- Files.createDirectory(mover.getConfigSave().toPath());
- try {
- mover.move();
- Assert.fail("move() should have failed");
- } catch (IOException ioe) {
- // Expected, and it must report the failed backup copy
- Assert.assertTrue(ioe.getMessage(), ioe.getMessage().contains("Failed to copy"));
- }
- Assert.assertEquals("old", Files.readString(dir.resolve("server.xml")));
- Assert.assertTrue(Files.exists(dir.resolve("server.xml.new")));
- } finally {
- deleteRecursively(dir);
- }
- }
-
- private static void deleteRecursively(Path dir) throws IOException {
- try (Stream<Path> paths = Files.walk(dir)) {
- for (Path path : (Iterable<Path>) paths.sorted(Comparator.reverseOrder())::iterator) {
- Files.deleteIfExists(path);
- }
- }
- }
-}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 12/15: Keep monitor, heartbeat and JMX alive across partial channel stops
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 2d2f2557de2052d2847d8d94090d6504542a069a
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 9 11:44:08 2026 +0200
Keep monitor, heartbeat and JMX alive across partial channel stops
GroupChannel.stop(svc) unconditionally cancelled the monitor and
heartbeat futures, unregistered the channel from JMX and, when the
channel owned its utility executor, shut it down, even when only a
subset of the services was stopped. A channel with services still
running was left without heartbeats and monitoring, and a following
start scheduled its periodic monitor task on an executor that had
been shut down. A shutdown of a scheduled executor does not cancel
periodic tasks, so a monitor task scheduled by an earlier start could
keep the non daemon pool threads of an owned executor alive and
prevent the JVM from exiting. GroupChannel.start(svc) also scheduled
a new monitor task on every call, leaking duplicates on repeated
partial or full starts.
Perform that teardown only once all services have been stopped,
based on the started service flags of the coordinator, and schedule
the monitor task only when it is not already running. Add tests for
the monitor leak on a repeated start and for the partial stop.
---
.../catalina/tribes/group/ChannelCoordinator.java | 9 +++++
.../apache/catalina/tribes/group/GroupChannel.java | 43 +++++++++++++---------
.../tribes/group/TestGroupChannelStartStop.java | 38 +++++++++++++++++++
3 files changed, 73 insertions(+), 17 deletions(-)
diff --git a/java/org/apache/catalina/tribes/group/ChannelCoordinator.java b/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
index b9409f88f5..1794441d74 100644
--- a/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
+++ b/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
@@ -125,6 +125,15 @@ public class ChannelCoordinator extends ChannelInterceptorBase implements Messag
this.internalStop(svc);
}
+ /**
+ * Returns the bit mask of the services that are currently started.
+ *
+ * @return the started service flags
+ */
+ synchronized int getStartLevel() {
+ return startLevel;
+ }
+
/**
* Starts up the channel. This can be called multiple times for individual services to start The svc parameter can
diff --git a/java/org/apache/catalina/tribes/group/GroupChannel.java b/java/org/apache/catalina/tribes/group/GroupChannel.java
index 788142a109..4a660753dd 100644
--- a/java/org/apache/catalina/tribes/group/GroupChannel.java
+++ b/java/org/apache/catalina/tribes/group/GroupChannel.java
@@ -456,7 +456,9 @@ public class GroupChannel extends ChannelInterceptorBase implements ManagedChann
ownExecutor = true;
}
super.start(svc);
- monitorFuture = utilityExecutor.scheduleWithFixedDelay(this::startHeartbeat, 0, 60, TimeUnit.SECONDS);
+ if (monitorFuture == null || monitorFuture.isDone()) {
+ monitorFuture = utilityExecutor.scheduleWithFixedDelay(this::startHeartbeat, 0, 60, TimeUnit.SECONDS);
+ }
}
/**
@@ -479,23 +481,30 @@ public class GroupChannel extends ChannelInterceptorBase implements ManagedChann
@Override
public synchronized void stop(int svc) throws ChannelException {
- if (monitorFuture != null) {
- monitorFuture.cancel(true);
- monitorFuture = null;
- }
- if (heartbeatFuture != null) {
- heartbeatFuture.cancel(true);
- heartbeatFuture = null;
- }
super.stop(svc);
- if (ownExecutor) {
- utilityExecutor.shutdown();
- utilityExecutor = null;
- ownExecutor = false;
- }
- if (oname != null) {
- JmxRegistry.getRegistry(this).unregisterJmx(oname);
- oname = null;
+ if (coordinator.getStartLevel() == 0) {
+ // The monitor and the heartbeat only make sense while services
+ // are running. Cancel them, and release an executor that this
+ // channel owns, only once all services have been stopped. A
+ // partial stop must not tear down what the started services
+ // still rely on.
+ if (monitorFuture != null) {
+ monitorFuture.cancel(true);
+ monitorFuture = null;
+ }
+ if (heartbeatFuture != null) {
+ heartbeatFuture.cancel(true);
+ heartbeatFuture = null;
+ }
+ if (ownExecutor && utilityExecutor != null) {
+ utilityExecutor.shutdown();
+ utilityExecutor = null;
+ ownExecutor = false;
+ }
+ if (oname != null) {
+ JmxRegistry.getRegistry(this).unregisterJmx(oname);
+ oname = null;
+ }
}
}
diff --git a/test/org/apache/catalina/tribes/group/TestGroupChannelStartStop.java b/test/org/apache/catalina/tribes/group/TestGroupChannelStartStop.java
index 48bc9631f2..4460f5dc1b 100644
--- a/test/org/apache/catalina/tribes/group/TestGroupChannelStartStop.java
+++ b/test/org/apache/catalina/tribes/group/TestGroupChannelStartStop.java
@@ -16,6 +16,8 @@
*/
package org.apache.catalina.tribes.group;
+import java.util.concurrent.ScheduledFuture;
+
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
@@ -126,6 +128,42 @@ public class TestGroupChannelStartStop {
channel.stop(Channel.DEFAULT);
}
+ @Test
+ public void testNoMonitorLeakOnRepeatedStart() throws Exception {
+ channel.start(Channel.DEFAULT);
+ ScheduledFuture<?> firstMonitor = channel.monitorFuture;
+ Assert.assertNotNull(firstMonitor);
+ // a second start must not schedule a second monitor task
+ channel.start(Channel.DEFAULT);
+ Assert.assertSame(firstMonitor, channel.monitorFuture);
+ channel.stop(Channel.DEFAULT);
+ }
+
+ @Test
+ public void testPartialStopKeepsHeartbeatAlive() throws Exception {
+ channel.start(Channel.DEFAULT);
+ // the monitor task runs immediately and schedules the heartbeat
+ long deadline = System.currentTimeMillis() + 10000;
+ while (channel.heartbeatFuture == null && System.currentTimeMillis() < deadline) {
+ Thread.sleep(50);
+ }
+ ScheduledFuture<?> monitor = channel.monitorFuture;
+ ScheduledFuture<?> heartbeat = channel.heartbeatFuture;
+ Assert.assertNotNull(heartbeat);
+ // a partial stop must not cancel the monitor or the heartbeat
+ channel.stop(Channel.SND_RX_SEQ);
+ Assert.assertSame(monitor, channel.monitorFuture);
+ Assert.assertFalse(monitor.isCancelled());
+ Assert.assertFalse(heartbeat.isCancelled());
+ Assert.assertNotNull(channel.utilityExecutor);
+ // a full stop cancels them
+ channel.stop(Channel.DEFAULT);
+ Assert.assertNull(channel.monitorFuture);
+ Assert.assertNull(channel.heartbeatFuture);
+ Assert.assertTrue(monitor.isCancelled());
+ Assert.assertTrue(heartbeat.isCancelled());
+ }
+
@Test
public void testOverlappingPartialStart() throws Exception {
// start one service, then start everything: the already
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 07/15: Fix NullPointerException in ChannelException.getMessage() without detail
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 2607fb1fc5b57530095460c149431859f806753d
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 9 11:00:27 2026 +0200
Fix NullPointerException in ChannelException.getMessage() without detail
getMessage() built its result from new StringBuilder(super.getMessage()),
which throws a NullPointerException when the exception was created without
a detail message, for example through the public no-arg constructor.
Since Throwable.toString() calls getMessage(), logging or otherwise
stringifying such an instance failed with an NPE, masking the original
failure. This is a latent robustness bug in the public API: all internal
construction sites pass a message or a cause, but custom interceptors or
error handlers may not.
Use the class name as the base when the detail message is null, in line
with Throwable.toString(). The formatting for instances that do have a
message is unchanged.
---
.../apache/catalina/tribes/ChannelException.java | 6 ++-
.../catalina/tribes/TestChannelException.java | 61 ++++++++++++++++++++++
2 files changed, 66 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/catalina/tribes/ChannelException.java b/java/org/apache/catalina/tribes/ChannelException.java
index b90424bb19..5565906f8f 100644
--- a/java/org/apache/catalina/tribes/ChannelException.java
+++ b/java/org/apache/catalina/tribes/ChannelException.java
@@ -91,7 +91,11 @@ public class ChannelException extends Exception {
*/
@Override
public String getMessage() {
- StringBuilder buf = new StringBuilder(super.getMessage());
+ // The detail message is null when the exception is created without a
+ // message and without a cause. Use the class name as the base, in line
+ // with Throwable.toString(), to keep getMessage() null-safe
+ String base = super.getMessage();
+ StringBuilder buf = new StringBuilder(base != null ? base : getClass().getName());
if (faultyMembers == null || faultyMembers.isEmpty()) {
buf.append("; No faulty members identified.");
} else {
diff --git a/test/org/apache/catalina/tribes/TestChannelException.java b/test/org/apache/catalina/tribes/TestChannelException.java
new file mode 100644
index 0000000000..9319fd8c43
--- /dev/null
+++ b/test/org/apache/catalina/tribes/TestChannelException.java
@@ -0,0 +1,61 @@
+/*
+ * 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.catalina.tribes;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.catalina.tribes.membership.MemberImpl;
+
+public class TestChannelException {
+
+ /**
+ * Verify that getMessage() and toString() work for exceptions created without a detail message, instead of
+ * throwing a NullPointerException because the base message is null.
+ *
+ * @throws Exception if the test experiences an unexpected error
+ */
+ @Test
+ public void testMessageWithoutDetail() throws Exception {
+ ChannelException e = new ChannelException();
+ String message = e.getMessage();
+ Assert.assertNotNull(message);
+ Assert.assertTrue(message, message.contains(ChannelException.class.getName()));
+ Assert.assertTrue(message, message.contains("No faulty members identified."));
+ Assert.assertNotNull(e.toString());
+
+ ChannelException withCause = new ChannelException(new IllegalStateException("root"));
+ Assert.assertTrue(withCause.getMessage(), withCause.getMessage().contains("root"));
+ }
+
+ /**
+ * Verify the unchanged formatting for a detail message and for faulty members.
+ *
+ * @throws Exception if the test experiences an unexpected error
+ */
+ @Test
+ public void testMessageWithDetail() throws Exception {
+ ChannelException e = new ChannelException("detail");
+ Assert.assertTrue(e.getMessage(), e.getMessage().startsWith("detail"));
+
+ e.addFaultyMember(new MemberImpl("10.0.0.1", 4000, -1), new Exception("failed"));
+ String message = e.getMessage();
+ Assert.assertTrue(message, message.startsWith("detail"));
+ Assert.assertTrue(message, message.contains("Faulty members:"));
+ Assert.assertTrue(message, message.contains("10.0.0.1"));
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 02/15: Do not store the runtime-derived OpenSSLConfCmd groups command
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit ac2c4b2b3036cd5cae7addefa395cdb87d7d131a
Author: opencode <re...@apache.org>
AuthorDate: Fri Oct 9 10:13:40 2026 +0200
Do not store the runtime-derived OpenSSLConfCmd groups command
When using the JNI OpenSSL implementation, creating the SSL context adds
a
"groups" OpenSSLConfCmd derived from the SSLHostConfig "groups"
attribute
directly into the configuration's command list. StoreConfig did not skip
it
(only the four internal OCSP commands were skipped), so saving the
running
configuration wrote a command that duplicated the persisted attribute.
On
the next start the stored command suppressed the derivation from the
attribute, silently ignoring any later change of the attribute.
SSLHostConfigSF now stores a copy of the OpenSSLConf without groups
commands whose value is derived from the attribute. A user-written
command
with a different value is kept, and no filtering occurs when the
attribute
is not set, since the runtime cannot have injected a command then.
---
.../catalina/storeconfig/SSLHostConfigSF.java | 30 ++++++++++++
.../catalina/storeconfig/TestStoreConfig.java | 53 ++++++++++++++++++++++
2 files changed, 83 insertions(+)
diff --git a/java/org/apache/catalina/storeconfig/SSLHostConfigSF.java b/java/org/apache/catalina/storeconfig/SSLHostConfigSF.java
index 97c7602552..d26d82483f 100644
--- a/java/org/apache/catalina/storeconfig/SSLHostConfigSF.java
+++ b/java/org/apache/catalina/storeconfig/SSLHostConfigSF.java
@@ -25,6 +25,7 @@ import org.apache.tomcat.util.net.SSLHostConfig;
import org.apache.tomcat.util.net.SSLHostConfigCertificate;
import org.apache.tomcat.util.net.SSLHostConfigCertificate.Type;
import org.apache.tomcat.util.net.openssl.OpenSSLConf;
+import org.apache.tomcat.util.net.openssl.OpenSSLConfCmd;
/**
* Store SSLHostConfig
@@ -100,8 +101,37 @@ public class SSLHostConfigSF extends StoreFactoryBase {
storeElementArray(aWriter, indent, hostConfigsCertificates);
// Store nested <OpenSSLConf> element
OpenSSLConf openSslConf = sslHostConfig.getOpenSslConf();
+ openSslConf = removeRuntimeCommands(openSslConf, sslHostConfig);
storeElement(aWriter, indent, openSslConf);
}
}
+ /*
+ * When the SSL context is created, the OpenSSL implementation adds a runtime "groups" command derived from the
+ * SSLHostConfig "groups" attribute. Store a copy of the configuration without such a command, since it duplicates
+ * the attribute and a stored command would shadow later changes of the attribute. A command whose value differs
+ * from the derived one was written by the user and is kept.
+ */
+ private OpenSSLConf removeRuntimeCommands(OpenSSLConf openSslConf, SSLHostConfig sslHostConfig) {
+ if (openSslConf == null || sslHostConfig.getGroups() == null) {
+ return openSslConf;
+ }
+ String derivedValue = sslHostConfig.getGroups().replace(',', ':');
+ ArrayList<OpenSSLConfCmd> retainedCommands = new ArrayList<>();
+ for (OpenSSLConfCmd command : openSslConf.getCommands()) {
+ if (OpenSSLConfCmd.GROUPS.equals(command.getName()) && derivedValue.equals(command.getValue())) {
+ continue;
+ }
+ retainedCommands.add(command);
+ }
+ if (retainedCommands.size() == openSslConf.getCommands().size()) {
+ return openSslConf;
+ }
+ OpenSSLConf filteredCommands = new OpenSSLConf();
+ for (OpenSSLConfCmd command : retainedCommands) {
+ filteredCommands.addCmd(command);
+ }
+ return filteredCommands;
+ }
+
}
diff --git a/test/org/apache/catalina/storeconfig/TestStoreConfig.java b/test/org/apache/catalina/storeconfig/TestStoreConfig.java
index 383d0b2947..faba0f337d 100644
--- a/test/org/apache/catalina/storeconfig/TestStoreConfig.java
+++ b/test/org/apache/catalina/storeconfig/TestStoreConfig.java
@@ -42,6 +42,9 @@ import org.apache.catalina.startup.TomcatBaseTest;
import org.apache.catalina.util.IOTools;
import org.apache.catalina.util.SessionIdGeneratorBase;
import org.apache.catalina.valves.AccessLogValve;
+import org.apache.tomcat.util.net.SSLHostConfig;
+import org.apache.tomcat.util.net.openssl.OpenSSLConf;
+import org.apache.tomcat.util.net.openssl.OpenSSLConfCmd;
import org.xml.sax.InputSource;
public class TestStoreConfig extends TomcatBaseTest {
@@ -558,6 +561,56 @@ public class TestStoreConfig extends TomcatBaseTest {
Assert.assertTrue(serverXmlDump, serverXmlDump.contains("throwOnFailure=\"false\""));
}
+ /**
+ * Verify that the "groups" OpenSSLConfCmd that the OpenSSL implementation derives at runtime from the
+ * SSLHostConfig "groups" attribute is not stored, since a stored command would shadow later changes of the
+ * attribute. A user-written groups command must be kept.
+ *
+ * @throws Exception if the test experiences an unexpected error
+ */
+ @Test
+ public void testOpenSSLConfGroupsCommandHandling() throws Exception {
+ StoreLoader loader = new StoreLoader();
+ loader.load(null);
+ StoreRegistry registry = loader.getRegistry();
+ StoreDescription desc = registry.findDescription(SSLHostConfig.class);
+ Assert.assertNotNull(desc);
+
+ // The runtime command that the OpenSSL implementation adds from the attribute must not be stored
+ SSLHostConfig derived = new SSLHostConfig();
+ derived.setGroups("secp256r1,secp384r1");
+ derived.setOpenSslConf(new OpenSSLConf());
+ derived.getOpenSslConf().addCmd(new OpenSSLConfCmd(OpenSSLConfCmd.GROUPS, "secp256r1:secp384r1"));
+ String dump = storeToString(desc, derived);
+ Assert.assertTrue(dump, dump.contains("groups=\"secp256r1,secp384r1\""));
+ Assert.assertFalse(dump, dump.contains("OpenSSLConfCmd"));
+
+ // A user-written command with a value different from the derived one must be kept
+ SSLHostConfig userCommand = new SSLHostConfig();
+ userCommand.setGroups("secp256r1,secp384r1");
+ userCommand.setOpenSslConf(new OpenSSLConf());
+ userCommand.getOpenSslConf().addCmd(new OpenSSLConfCmd(OpenSSLConfCmd.GROUPS, "X25519"));
+ dump = storeToString(desc, userCommand);
+ Assert.assertTrue(dump, dump.contains("OpenSSLConfCmd"));
+ Assert.assertTrue(dump, dump.contains("name=\"groups\""));
+ Assert.assertTrue(dump, dump.contains("value=\"X25519\""));
+
+ // Without the attribute, a groups command can only be user-written and must be kept
+ SSLHostConfig noAttribute = new SSLHostConfig();
+ noAttribute.setGroups(null);
+ noAttribute.setOpenSslConf(new OpenSSLConf());
+ noAttribute.getOpenSslConf().addCmd(new OpenSSLConfCmd(OpenSSLConfCmd.GROUPS, "secp256r1"));
+ dump = storeToString(desc, noAttribute);
+ Assert.assertTrue(dump, dump.contains("OpenSSLConfCmd"));
+ Assert.assertTrue(dump, dump.contains("name=\"groups\""));
+ }
+
+ private static String storeToString(StoreDescription desc, Object element) throws Exception {
+ StringWriter buffer = new StringWriter();
+ desc.getStoreFactory().store(new PrintWriter(buffer), -2, element);
+ return buffer.toString();
+ }
+
public static class CustomTestManager extends StandardManager {
public CustomTestManager() {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org