This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new ed8d633903 Review "Validate fragment metadata in
FragmentationInterceptor ..."
ed8d633903 is described below
commit ed8d6339035fb93f8628dbff0a1e4ff1df851c2d
Author: Mark Thomas <[email protected]>
AuthorDate: Wed Oct 21 12:19:13 2026 +0100
Review "Validate fragment metadata in FragmentationInterceptor ..."
Add additional validation
Co-authored by: CoPilot / GPT-5.6 Sol
---
.../interceptors/FragmentationInterceptor.java | 22 ++++++++++++++--------
.../interceptors/TestFragmentationInterceptor.java | 18 ++++++++++++++++--
2 files changed, 30 insertions(+), 10 deletions(-)
diff --git
a/java/org/apache/catalina/tribes/group/interceptors/FragmentationInterceptor.java
b/java/org/apache/catalina/tribes/group/interceptors/FragmentationInterceptor.java
index 9f1ea5dd83..4a1f21b513 100644
---
a/java/org/apache/catalina/tribes/group/interceptors/FragmentationInterceptor.java
+++
b/java/org/apache/catalina/tribes/group/interceptors/FragmentationInterceptor.java
@@ -94,7 +94,14 @@ public class FragmentationInterceptor extends
ChannelInterceptorBase implements
@Override
public void messageReceived(ChannelMessage msg) {
- boolean isFrag =
XByteBuffer.toBoolean(msg.getMessage().getBytesDirect(),
msg.getMessage().getLength() - 1);
+ int length = msg.getMessage().getLength();
+ if (length < 1) {
+
log.warn(sm.getString("fragmentationInterceptor.fragment.discarded"),
+ new IllegalArgumentException(
+
sm.getString("fragmentationInterceptor.fragment.too-short",
Integer.toString(length))));
+ return;
+ }
+ boolean isFrag =
XByteBuffer.toBoolean(msg.getMessage().getBytesDirect(), length - 1);
msg.getMessage().trim(1);
if (isFrag) {
defrag(msg);
@@ -144,6 +151,11 @@ public class FragmentationInterceptor extends
ChannelInterceptorBase implements
FragKey key = new FragKey(msg.getUniqueId());
ChannelMessage complete = null;
try {
+ int length = msg.getMessage().getLength();
+ if (length < 8) {
+ throw new IllegalArgumentException(
+
sm.getString("fragmentationInterceptor.fragment.too-short",
Integer.toString(length)));
+ }
FragCollection coll = getFragCollection(key, msg);
// The deep clone does not touch shared state, so it happens
before the
// monitor is entered. The fragment storage of a collection is
shared
@@ -300,13 +312,7 @@ public class FragmentationInterceptor extends
ChannelInterceptorBase implements
*/
public synchronized void addMessage(ChannelMessage msg) {
// At least the fragment number and the fragment count must be
present.
- // Check before trimming, trimming more bytes than are available
throws
- // an ArrayIndexOutOfBoundsException.
- int length = msg.getMessage().getLength();
- if (length < 8) {
- throw new IllegalArgumentException(
-
sm.getString("fragmentationInterceptor.fragment.too-short",
Integer.toString(length)));
- }
+ // Size has already been checked so trim() is safe
// remove the total messages
msg.getMessage().trim(4);
// get the msg nr
diff --git
a/test/org/apache/catalina/tribes/group/interceptors/TestFragmentationInterceptor.java
b/test/org/apache/catalina/tribes/group/interceptors/TestFragmentationInterceptor.java
index 2f85b42e3e..e5f6ccc737 100644
---
a/test/org/apache/catalina/tribes/group/interceptors/TestFragmentationInterceptor.java
+++
b/test/org/apache/catalina/tribes/group/interceptors/TestFragmentationInterceptor.java
@@ -313,6 +313,20 @@ public class TestFragmentationInterceptor {
Assert.assertNull("Fragment that is too short must not be delivered",
delivery.delivered);
}
+ @Test
+ public void testEmptyMessageIsDiscarded() throws Exception {
+ Member destination = new MemberImpl("localhost", 4000, -1);
+ FragmentationInterceptor receiver = new FragmentationInterceptor();
+ DeliveringCollector delivery = new DeliveringCollector();
+ receiver.setChannel(new GroupChannel());
+ receiver.setPrevious(delivery);
+
+ ChannelData message = createMessage(new byte[0], destination);
+ receiver.messageReceived(message);
+
+ Assert.assertNull("Empty message must not be delivered",
delivery.delivered);
+ }
+
@Test
public void testShortNonFirstFragmentIsDiscarded() throws Exception {
Member destination = new MemberImpl("localhost", 4000, -1);
@@ -352,9 +366,9 @@ public class TestFragmentationInterceptor {
byte[] uniqueId = UUIDGenerator.randomUUID(false);
+ // A valid count without a fragment number must be rejected before it
creates reassembly state
XByteBuffer malformed = new XByteBuffer(16, false);
- malformed.append("data".getBytes(StandardCharsets.UTF_8), 0, 4);
- malformed.append(Integer.MAX_VALUE);
+ malformed.append(3);
malformed.append(true);
ChannelData malformedFrag = createMessage(new byte[0], destination,
uniqueId);
malformedFrag.setMessage(malformed);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
