This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit c9607d8165f9b0512389518af716a9878308791b Author: opencode <[email protected]> AuthorDate: Sun Oct 11 11:24:49 2026 +0200 Reject non-positive membership timing properties at start The timing properties of the membership services were stored as they were configured, which let a non positive value take effect. A non positive mcast frequency made the multicast sender send pings in a tight loop, as the exception that a non positive sleep raises is swallowed around the send cycle, and a non positive member drop time expired every member on every cycle, churning memberDisappeared and memberAdded events. The static membership provider showed the same behaviour with its ping interval and expiration time. Both services now validate the parsed values where the configuration converges, the multicast service when it builds its implementation and the static provider when it initialises, and reject them with an illegal argument exception naming the property and its value. The checks run before any socket or thread exists and cover both the setters and a replaced set of properties, as the setters only write the properties that these points read. The check of the ping interval moved to the top of the provider initialisation, next to the one of the expiration time, so that neither can leave a channel listener registered behind a rejected configuration. Add tests that a non positive value is rejected by each service before anything starts and that the static service still starts and stops with the default configuration. --- .../tribes/membership/LocalStrings.properties | 2 + .../catalina/tribes/membership/McastService.java | 14 +++- .../membership/StaticMembershipProvider.java | 12 +++- .../tribes/membership/TestMcastService.java | 37 +++++++++++ .../membership/TestStaticMembershipService.java | 77 ++++++++++++++++++++++ 5 files changed, 138 insertions(+), 4 deletions(-) diff --git a/java/org/apache/catalina/tribes/membership/LocalStrings.properties b/java/org/apache/catalina/tribes/membership/LocalStrings.properties index 230802e124..5080e92c5c 100644 --- a/java/org/apache/catalina/tribes/membership/LocalStrings.properties +++ b/java/org/apache/catalina/tribes/membership/LocalStrings.properties @@ -23,6 +23,8 @@ mcastService.exceed.maxPacketSize=Packet length [{0}] exceeds max packet size of mcastService.missing.property=McastService:Required property [{0}] is missing. mcastService.noStart=Multicast send is not started or enabled. +membershipService.invalidTimingProperty=The timing property [{0}] is set to [{1}]. It must be greater than zero. + mcastServiceImpl.bind=Attempting to bind the multicast socket to [{0}:{1}] mcastServiceImpl.bind.failed=Binding to multicast address, failed. Binding to port only. mcastServiceImpl.error.receiving=Error receiving mcast package. Sleeping 500ms diff --git a/java/org/apache/catalina/tribes/membership/McastService.java b/java/org/apache/catalina/tribes/membership/McastService.java index 0a05ed2f4d..900ed90271 100644 --- a/java/org/apache/catalina/tribes/membership/McastService.java +++ b/java/org/apache/catalina/tribes/membership/McastService.java @@ -383,8 +383,18 @@ public class McastService extends MembershipServiceBase implements MessageListen } } - impl = new McastServiceImpl(localMember, Long.parseLong(properties.getProperty("mcastFrequency")), - Long.parseLong(properties.getProperty("memberDropTime")), + long frequency = Long.parseLong(properties.getProperty("mcastFrequency")); + if (frequency <= 0) { + throw new IllegalArgumentException( + sm.getString("membershipService.invalidTimingProperty", "mcastFrequency", Long.valueOf(frequency))); + } + long dropTime = Long.parseLong(properties.getProperty("memberDropTime")); + if (dropTime <= 0) { + throw new IllegalArgumentException( + sm.getString("membershipService.invalidTimingProperty", "memberDropTime", Long.valueOf(dropTime))); + } + + impl = new McastServiceImpl(localMember, frequency, dropTime, Integer.parseInt(properties.getProperty("mcastPort")), bind, java.net.InetAddress.getByName(properties.getProperty("mcastAddress")), ttl, soTimeout, this, this, Boolean.parseBoolean(properties.getProperty("localLoopbackDisabled"))); diff --git a/java/org/apache/catalina/tribes/membership/StaticMembershipProvider.java b/java/org/apache/catalina/tribes/membership/StaticMembershipProvider.java index fdfc54bc41..aa0d891867 100644 --- a/java/org/apache/catalina/tribes/membership/StaticMembershipProvider.java +++ b/java/org/apache/catalina/tribes/membership/StaticMembershipProvider.java @@ -123,6 +123,16 @@ public class StaticMembershipProvider extends MembershipProviderBase // All the default properties have been set by StaticMembershipService.setDefaults, so no need to check String expirationTimeStr = properties.getProperty("expirationTime"); this.expirationTime = Long.parseLong(expirationTimeStr); + if (expirationTime <= 0) { + throw new IllegalArgumentException(sm.getString("membershipService.invalidTimingProperty", "expirationTime", + Long.valueOf(expirationTime))); + } + String pingIntervalStr = properties.getProperty("pingInterval"); + this.pingInterval = Long.parseLong(pingIntervalStr); + if (pingInterval <= 0) { + throw new IllegalArgumentException(sm.getString("membershipService.invalidTimingProperty", "pingInterval", + Long.valueOf(pingInterval))); + } String connectTimeoutStr = properties.getProperty("connectTimeout"); this.connectTimeout = Integer.parseInt(connectTimeoutStr); String rpcTimeouStr = properties.getProperty("rpcTimeout"); @@ -134,8 +144,6 @@ public class StaticMembershipProvider extends MembershipProviderBase this.channel.addChannelListener(this); String useThreadStr = properties.getProperty("useThread"); this.useThread = Boolean.parseBoolean(useThreadStr); - String pingIntervalStr = properties.getProperty("pingInterval"); - this.pingInterval = Long.parseLong(pingIntervalStr); } @Override diff --git a/test/org/apache/catalina/tribes/membership/TestMcastService.java b/test/org/apache/catalina/tribes/membership/TestMcastService.java index 77e43af28c..fa0144b841 100644 --- a/test/org/apache/catalina/tribes/membership/TestMcastService.java +++ b/test/org/apache/catalina/tribes/membership/TestMcastService.java @@ -123,6 +123,43 @@ public class TestMcastService { Assert.assertEquals(0, service.getTtl()); } + /* + * A non-positive send frequency makes the sender spin sending pings without a pause, and a + * non-positive drop time expires every member on every cycle. Such configuration must be + * rejected when the service starts, before any socket or thread is created. + */ + @Test + public void testStartRejectsNonPositiveTimingProperties() throws Exception { + String[] invalid = { "mcastFrequency=0", "mcastFrequency=-1", "memberDropTime=0", "memberDropTime=-5" }; + for (String entry : invalid) { + String[] property = entry.split("="); + Properties props = new Properties(); + props.setProperty("mcastAddress", "228.0.0.4"); + props.setProperty("mcastPort", "45564"); + props.setProperty("tcpListenHost", "127.0.0.1"); + props.setProperty("tcpListenPort", "4000"); + props.setProperty("tcpSecurePort", "0"); + props.setProperty("udpListenPort", "0"); + props.setProperty("mcastFrequency", "500"); + props.setProperty("memberDropTime", "3000"); + props.setProperty(property[0], property[1]); + + GroupChannel channel = new GroupChannel(); + channel.setName("TestMcastService-" + System.nanoTime()); + + McastService service = new McastService(); + service.setChannel(channel); + service.setProperties(props); + try { + service.start(Channel.MBR_TX_SEQ); + Assert.fail("start() must reject a non-positive " + property[0]); + } catch (IllegalArgumentException expected) { + // The configuration must be rejected before anything is started + } + Assert.assertNull(service.impl); + } + } + /* * The 65535 byte protocol limit of an IP packet includes the IP and UDP headers, so a * broadcast that fits within it can still exceed what the socket is able to send. Such a diff --git a/test/org/apache/catalina/tribes/membership/TestStaticMembershipService.java b/test/org/apache/catalina/tribes/membership/TestStaticMembershipService.java new file mode 100644 index 0000000000..c2b3992f18 --- /dev/null +++ b/test/org/apache/catalina/tribes/membership/TestStaticMembershipService.java @@ -0,0 +1,77 @@ +/* + * 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.membership; + +import java.util.Properties; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.catalina.tribes.Channel; +import org.apache.catalina.tribes.group.GroupChannel; + +public class TestStaticMembershipService { + + /* + * A non-positive ping interval makes the ping thread spin without a pause, and a non-positive + * expiration time expires every member on every cycle. Such configuration must be rejected + * when the service initializes its provider. + */ + @Test + public void testStartRejectsNonPositiveTimingProperties() throws Exception { + String[] invalid = { "pingInterval=0", "pingInterval=-1", "expirationTime=0", "expirationTime=-5" }; + for (String entry : invalid) { + String[] property = entry.split("="); + Properties props = new Properties(); + props.setProperty(property[0], property[1]); + + GroupChannel channel = new GroupChannel(); + channel.setName("TestStaticMembershipService-" + System.nanoTime()); + + StaticMembershipService service = new StaticMembershipService(); + service.setChannel(channel); + service.setProperties(props); + // The local member must be part of the static member list + service.addStaticMember(new StaticMember("127.0.0.1", 4001, -1)); + service.setLocalMemberProperties("127.0.0.1", 4001, 0, 0); + try { + service.start(Channel.MBR_TX_SEQ); + Assert.fail("start() must reject a non-positive " + property[0]); + } catch (IllegalArgumentException expected) { + // The configuration must be rejected before the provider is built + } + } + } + + /* + * A valid configuration, as setDefaults provides it, must still start and stop cleanly. + */ + @Test + public void testValidTimingPropertiesStartAndStop() throws Exception { + GroupChannel channel = new GroupChannel(); + channel.setName("TestStaticMembershipService-" + System.nanoTime()); + + StaticMembershipService service = new StaticMembershipService(); + service.setChannel(channel); + service.addStaticMember(new StaticMember("127.0.0.1", 4002, -1)); + service.setLocalMemberProperties("127.0.0.1", 4002, 0, 0); + + int level = Channel.MBR_RX_SEQ | Channel.MBR_TX_SEQ; + service.start(level); + service.stop(level); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
