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 e52e3879fc Review "Bound the multicast send paths to a sendable packet
size"
e52e3879fc is described below
commit e52e3879fc4b927caec016b855e6b7760a2e6e6d
Author: Mark Thomas <[email protected]>
AuthorDate: Thu Oct 22 17:42:15 2026 +0100
Review "Bound the multicast send paths to a sendable packet size"
Extend coverage to all variable length components
---
.../tribes/membership/LocalStrings.properties | 4 ++
.../catalina/tribes/membership/MemberImpl.java | 45 ++++++++++++++--------
.../catalina/tribes/membership/TestMemberImpl.java | 42 ++++++++++++++++++++
3 files changed, 76 insertions(+), 15 deletions(-)
diff --git a/java/org/apache/catalina/tribes/membership/LocalStrings.properties
b/java/org/apache/catalina/tribes/membership/LocalStrings.properties
index 9676d1ad20..7cf0872988 100644
--- a/java/org/apache/catalina/tribes/membership/LocalStrings.properties
+++ b/java/org/apache/catalina/tribes/membership/LocalStrings.properties
@@ -58,7 +58,11 @@ mcastServiceImpl.waitForMembers.start=Sleeping for [{0}]
milliseconds to establi
memberImpl.invalid.hostLength=Invalid host length [{0}]
memberImpl.invalid.package.begin=Invalid package, should start with:[{0}]
memberImpl.invalid.package.end=Invalid package, should end with:[{0}]
+memberImpl.large.command=Command is too large for tribes to handle.
+memberImpl.large.domain=Domain is too large for tribes to handle.
+memberImpl.large.host=Host is too large for tribes to handle.
memberImpl.large.payload=Payload is too large for tribes to handle.
+memberImpl.large.uniqueId=Unique ID is too large for tribes to handle.
memberImpl.notEnough.bytes=Not enough bytes in member package.
memberImpl.package.small=Member package too small to validate.
diff --git a/java/org/apache/catalina/tribes/membership/MemberImpl.java
b/java/org/apache/catalina/tribes/membership/MemberImpl.java
index 5ec7aa0154..0fb3b8c6cb 100644
--- a/java/org/apache/catalina/tribes/membership/MemberImpl.java
+++ b/java/org/apache/catalina/tribes/membership/MemberImpl.java
@@ -427,12 +427,12 @@ public class MemberImpl implements Member,
java.io.Externalizable {
pos += payload.length;
synchronized (member) {
- member.setHost(addr);
+ member.host = addr;
member.setPort(XByteBuffer.toInt(portd, 0));
member.setSecurePort(XByteBuffer.toInt(sportd, 0));
member.setUdpPort(XByteBuffer.toInt(uportd, 0));
member.setMemberAliveTime(XByteBuffer.toLong(alived, 0));
- member.setUniqueId(uniqueId);
+ member.uniqueId = uniqueId;
member.payload = payload;
member.domain = domain;
member.command = command;
@@ -628,7 +628,9 @@ public class MemberImpl implements Member,
java.io.Externalizable {
* @param host the host
*/
public synchronized void setHost(byte[] host) {
+ checkSize(this.host.length, host != null ? host.length : 0,
"memberImpl.large.host");
this.host = host;
+ this.dataPkg = null;
}
/**
@@ -637,9 +639,12 @@ public class MemberImpl implements Member,
java.io.Externalizable {
* @throws IOException if there is an error converting the host name to an
IP address
*/
public void setHostname(String host) throws IOException {
- hostname = host;
+ byte[] address = java.net.InetAddress.getByName(host).getAddress();
synchronized (this) {
- this.host = java.net.InetAddress.getByName(host).getAddress();
+ checkSize(this.host.length, address.length,
"memberImpl.large.host");
+ hostname = host;
+ this.host = address;
+ this.dataPkg = null;
}
}
@@ -673,31 +678,40 @@ public class MemberImpl implements Member,
java.io.Externalizable {
* @param uniqueId the unique ID
*/
public synchronized void setUniqueId(byte[] uniqueId) {
+ checkSize(this.uniqueId.length, uniqueId != null ? uniqueId.length :
16, "memberImpl.large.uniqueId");
this.uniqueId = uniqueId != null ? uniqueId : new byte[16];
getData(true, true);
}
@Override
public synchronized void setPayload(byte[] payload) {
+ checkSize(this.payload.length, payload != null ? payload.length : 0,
"memberImpl.large.payload");
+ this.payload = payload != null ? payload : new byte[0];
+ getData(true, true);
+ }
+
+ /**
+ * Check that replacing a variable length component of this member will
not take the member ping above the maximum
+ * size that can be sent. Must be called before the component is changed
so that a rejected change leaves the member
+ * unmodified.
+ *
+ * @param oldLength The length of the component being replaced
+ * @param newLength The length of the replacement component
+ * @param msgKey The key of the message for the exception thrown if the
size is exceeded
+ */
+ private void checkSize(int oldLength, int newLength, String msgKey) {
// longs to avoid any possibility of overflow
- long oldPayloadLength = this.payload.length;
- long newPayloadLength = 0;
- if (payload != null) {
- newPayloadLength = payload.length;
- }
- if (newPayloadLength > oldPayloadLength) {
+ if (newLength > oldLength) {
// It is possible that the max packet size will be exceeded
- if ((newPayloadLength - oldPayloadLength +
- getData(false, false).length) >
McastServiceImpl.MAX_SEND_SIZE) {
- throw new
IllegalArgumentException(sm.getString("memberImpl.large.payload"));
+ if (((long) newLength - oldLength + getData(false, false).length)
> McastServiceImpl.MAX_SEND_SIZE) {
+ throw new IllegalArgumentException(sm.getString(msgKey));
}
}
- this.payload = payload != null ? payload : new byte[0];
- getData(true, true);
}
@Override
public synchronized void setCommand(byte[] command) {
+ checkSize(this.command.length, command != null ? command.length : 0,
"memberImpl.large.command");
this.command = command != null ? command : new byte[0];
getData(true, true);
}
@@ -707,6 +721,7 @@ public class MemberImpl implements Member,
java.io.Externalizable {
* @param domain the domain
*/
public synchronized void setDomain(byte[] domain) {
+ checkSize(this.domain.length, domain != null ? domain.length : 0,
"memberImpl.large.domain");
this.domain = domain != null ? domain : new byte[0];
getData(true, true);
}
diff --git a/test/org/apache/catalina/tribes/membership/TestMemberImpl.java
b/test/org/apache/catalina/tribes/membership/TestMemberImpl.java
index b13e8e9030..07c6e8f4d3 100644
--- a/test/org/apache/catalina/tribes/membership/TestMemberImpl.java
+++ b/test/org/apache/catalina/tribes/membership/TestMemberImpl.java
@@ -45,4 +45,46 @@ public class TestMemberImpl {
// The size check must reject it
}
}
+
+ /*
+ * Every variable length component of the ping must be bounded in the same
way as the payload, and
+ * a rejected change must leave the member unmodified and usable.
+ */
+ @Test
+ public void testOtherComponentsAreBoundedAtTheSendableSize() throws
Exception {
+ MemberImpl member = new MemberImpl("localhost", 4000, 100);
+ member.setUniqueId(UUIDGenerator.randomUUID(false));
+ byte[] before = member.getData(false, false);
+ int base = before.length;
+ int tooLarge = McastServiceImpl.MAX_SEND_SIZE - base + 1;
+
+ try {
+ member.setCommand(new byte[tooLarge + member.getCommand().length]);
+ Assert.fail("An oversized command must be rejected");
+ } catch (IllegalArgumentException expected) {
+ // Expected
+ }
+ try {
+ member.setDomain(new byte[tooLarge + member.getDomain().length]);
+ Assert.fail("An oversized domain must be rejected");
+ } catch (IllegalArgumentException expected) {
+ // Expected
+ }
+ try {
+ member.setUniqueId(new byte[tooLarge +
member.getUniqueId().length]);
+ Assert.fail("An oversized unique ID must be rejected");
+ } catch (IllegalArgumentException expected) {
+ // Expected
+ }
+ try {
+ member.setHost(new byte[tooLarge + member.getHost().length]);
+ Assert.fail("An oversized host must be rejected");
+ } catch (IllegalArgumentException expected) {
+ // Expected
+ }
+
+ // Rejected changes leave the member unmodified and usable
+ Assert.assertEquals(base, member.getData(false, true).length);
+ member.setCommand(new byte[4]);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
