From 5a8c3adaaa9beac2237cd243f661564dcdb1d4bb Mon Sep 17 00:00:00 2001 From: Quan Date: Tue, 20 Oct 2026 17:39:36 +0800 Subject: [PATCH 1/3] [ISSUE #11466] Make broker-side subscription group max length configurable - Add a broker setting with the existing 120-character default - Apply the configurable limit to broker-side group creation paths - Cover default, custom, fallback, cap, and batch behavior --- .../processor/AdminBrokerProcessor.java | 6 +- .../SubscriptionGroupManager.java | 3 +- .../processor/AdminBrokerProcessorTest.java | 57 +++++++++++++++++++ .../SubscriptionGroupManagerTest.java | 22 +++++++ .../apache/rocketmq/common/BrokerConfig.java | 14 +++++ .../rocketmq/common/topic/TopicValidator.java | 12 +++- .../common/topic/TopicValidatorTest.java | 42 ++++++++++++++ 7 files changed, 151 insertions(+), 5 deletions(-) diff --git a/broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java b/broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java index 604e3e18ca4..f7099087d30 100644 --- a/broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java +++ b/broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java @@ -1673,7 +1673,8 @@ private RemotingCommand updateAndCreateSubscriptionGroup(ChannelHandlerContext c SubscriptionGroupConfig config = RemotingSerializable.decode(request.getBody(), SubscriptionGroupConfig.class); if (null != config) { - TopicValidator.ValidateResult result = TopicValidator.validateGroup(config.getGroupName()); + TopicValidator.ValidateResult result = TopicValidator.validateGroup(config.getGroupName(), + this.brokerController.getBrokerConfig().getSubscriptionGroupMaxLength()); if (!result.isValid()) { response.setCode(ResponseCode.INVALID_PARAMETER); response.setRemark(result.getRemark()); @@ -1707,7 +1708,8 @@ private RemotingCommand updateAndCreateSubscriptionGroupList(ChannelHandlerConte final StringBuilder builder = new StringBuilder(); for (SubscriptionGroupConfig config : groupConfigList) { - TopicValidator.ValidateResult result = TopicValidator.validateGroup(config.getGroupName()); + TopicValidator.ValidateResult result = TopicValidator.validateGroup(config.getGroupName(), + this.brokerController.getBrokerConfig().getSubscriptionGroupMaxLength()); if (!result.isValid()) { response.setCode(ResponseCode.INVALID_PARAMETER); response.setRemark(result.getRemark()); diff --git a/broker/src/main/java/org/apache/rocketmq/broker/subscription/SubscriptionGroupManager.java b/broker/src/main/java/org/apache/rocketmq/broker/subscription/SubscriptionGroupManager.java index b37c35051aa..10fd141a802 100644 --- a/broker/src/main/java/org/apache/rocketmq/broker/subscription/SubscriptionGroupManager.java +++ b/broker/src/main/java/org/apache/rocketmq/broker/subscription/SubscriptionGroupManager.java @@ -260,7 +260,8 @@ public SubscriptionGroupConfig findSubscriptionGroupConfig(final String group) { if (null == subscriptionGroupConfig) { if (brokerController.getBrokerConfig().isAutoCreateSubscriptionGroup() || MixAll.isSysConsumerGroupAndEnableCreate(group, brokerController.getBrokerConfig().isEnableCreateSysGroup())) { - TopicValidator.ValidateResult result = TopicValidator.validateGroup(group); + TopicValidator.ValidateResult result = TopicValidator.validateGroup(group, + brokerController.getBrokerConfig().getSubscriptionGroupMaxLength()); if (!result.isValid()) { return null; } diff --git a/broker/src/test/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessorTest.java b/broker/src/test/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessorTest.java index a573a25211c..de3ef93bae0 100644 --- a/broker/src/test/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessorTest.java +++ b/broker/src/test/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessorTest.java @@ -1114,6 +1114,63 @@ public void testUpdateAndCreateSubscriptionGroup() throws RemotingCommandExcepti assertThat(response.getCode()).isEqualTo(ResponseCode.SUCCESS); } + @Test + public void testUpdateAndCreateSubscriptionGroupWithConfigurableMaxLength() throws RemotingCommandException { + String groupName = StringUtils.repeat('a', 121); + SubscriptionGroupConfig subscriptionGroupConfig = new SubscriptionGroupConfig(); + subscriptionGroupConfig.setGroupName(groupName); + RemotingCommand request = RemotingCommand.createRequestCommand(RequestCode.UPDATE_AND_CREATE_SUBSCRIPTIONGROUP, null); + request.setBody(JSON.toJSON(subscriptionGroupConfig).toString().getBytes(StandardCharsets.UTF_8)); + + RemotingCommand response = adminBrokerProcessor.processRequest(handlerContext, request); + assertThat(response.getCode()).isEqualTo(ResponseCode.INVALID_PARAMETER); + assertFalse(brokerController.getSubscriptionGroupManager().getSubscriptionGroupTable().containsKey(groupName)); + + brokerController.getBrokerConfig().setSubscriptionGroupMaxLength(255); + response = adminBrokerProcessor.processRequest(handlerContext, request); + assertThat(response.getCode()).isEqualTo(ResponseCode.SUCCESS); + assertTrue(brokerController.getSubscriptionGroupManager().getSubscriptionGroupTable().containsKey(groupName)); + } + + @Test + public void testUpdateAndCreateSubscriptionGroupListWithConfigurableMaxLength() throws RemotingCommandException { + String groupName = StringUtils.repeat('b', 121); + SubscriptionGroupConfig subscriptionGroupConfig = new SubscriptionGroupConfig(); + subscriptionGroupConfig.setGroupName(groupName); + SubscriptionGroupList subscriptionGroupList = + new SubscriptionGroupList(Collections.singletonList(subscriptionGroupConfig)); + RemotingCommand request = RemotingCommand.createRequestCommand(RequestCode.UPDATE_AND_CREATE_SUBSCRIPTIONGROUP_LIST, null); + request.setBody(subscriptionGroupList.encode()); + + RemotingCommand response = adminBrokerProcessor.processRequest(handlerContext, request); + assertThat(response.getCode()).isEqualTo(ResponseCode.INVALID_PARAMETER); + assertFalse(brokerController.getSubscriptionGroupManager().getSubscriptionGroupTable().containsKey(groupName)); + + brokerController.getBrokerConfig().setSubscriptionGroupMaxLength(255); + response = adminBrokerProcessor.processRequest(handlerContext, request); + assertThat(response.getCode()).isEqualTo(ResponseCode.SUCCESS); + assertTrue(brokerController.getSubscriptionGroupManager().getSubscriptionGroupTable().containsKey(groupName)); + } + + @Test + public void testUpdateAndCreateSubscriptionGroupListRejectsWholeBatchWhenOneTooLong() throws RemotingCommandException { + brokerController.getBrokerConfig().setSubscriptionGroupMaxLength(255); + String validGroup = StringUtils.repeat('c', 127); + String tooLongGroup = StringUtils.repeat('d', 256); + SubscriptionGroupConfig validConfig = new SubscriptionGroupConfig(); + validConfig.setGroupName(validGroup); + SubscriptionGroupConfig tooLongConfig = new SubscriptionGroupConfig(); + tooLongConfig.setGroupName(tooLongGroup); + SubscriptionGroupList subscriptionGroupList = new SubscriptionGroupList(Arrays.asList(validConfig, tooLongConfig)); + RemotingCommand request = RemotingCommand.createRequestCommand(RequestCode.UPDATE_AND_CREATE_SUBSCRIPTIONGROUP_LIST, null); + request.setBody(subscriptionGroupList.encode()); + + RemotingCommand response = adminBrokerProcessor.processRequest(handlerContext, request); + assertThat(response.getCode()).isEqualTo(ResponseCode.INVALID_PARAMETER); + assertFalse(brokerController.getSubscriptionGroupManager().getSubscriptionGroupTable().containsKey(validGroup)); + assertFalse(brokerController.getSubscriptionGroupManager().getSubscriptionGroupTable().containsKey(tooLongGroup)); + } + @Test public void testUpdateAndCreateSubscriptionGroupRejectsEmptyLiteBindTopic() { String groupName = "GID-EMPTY-LITE-BIND"; diff --git a/broker/src/test/java/org/apache/rocketmq/broker/subscription/SubscriptionGroupManagerTest.java b/broker/src/test/java/org/apache/rocketmq/broker/subscription/SubscriptionGroupManagerTest.java index 9f709f6502a..3abab57fa97 100644 --- a/broker/src/test/java/org/apache/rocketmq/broker/subscription/SubscriptionGroupManagerTest.java +++ b/broker/src/test/java/org/apache/rocketmq/broker/subscription/SubscriptionGroupManagerTest.java @@ -27,7 +27,9 @@ import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.atomic.AtomicLong; +import org.apache.commons.lang3.StringUtils; import org.apache.rocketmq.broker.BrokerController; +import org.apache.rocketmq.common.BrokerConfig; import org.apache.rocketmq.common.MixAll; import org.apache.rocketmq.common.SubscriptionGroupAttributes; import org.apache.rocketmq.common.attribute.BooleanAttribute; @@ -118,6 +120,26 @@ public void updateSubscriptionGroupConfig() { private boolean notToBeExecuted() { return MixAll.isMac(); } + + @Test + public void testFindSubscriptionGroupConfigAutoCreateWithConfigurableMaxLength() { + BrokerConfig brokerConfig = new BrokerConfig(); + brokerConfig.setAutoCreateSubscriptionGroup(true); + Mockito.when(brokerControllerMock.getBrokerConfig()).thenReturn(brokerConfig); + Mockito.doNothing().when(subscriptionGroupManager).persist(); + + String groupName = StringUtils.repeat('a', 121); + assertThat(subscriptionGroupManager.findSubscriptionGroupConfig(groupName)).isNull(); + assertThat(subscriptionGroupManager.getSubscriptionGroupTable()).doesNotContainKey(groupName); + + brokerConfig.setSubscriptionGroupMaxLength(255); + SubscriptionGroupConfig result = subscriptionGroupManager.findSubscriptionGroupConfig(groupName); + assertThat(result).isNotNull(); + assertThat(result.getGroupName()).isEqualTo(groupName); + assertThat(subscriptionGroupManager.findSubscriptionGroupConfig(StringUtils.repeat('b', 256))).isNull(); + verify(subscriptionGroupManager).persist(); + } + @Test public void testUpdateSubscriptionGroupConfigList_NullConfigList() { if (notToBeExecuted()) { diff --git a/common/src/main/java/org/apache/rocketmq/common/BrokerConfig.java b/common/src/main/java/org/apache/rocketmq/common/BrokerConfig.java index 8e49839e3e0..05dc50b4bda 100644 --- a/common/src/main/java/org/apache/rocketmq/common/BrokerConfig.java +++ b/common/src/main/java/org/apache/rocketmq/common/BrokerConfig.java @@ -56,6 +56,12 @@ public class BrokerConfig extends BrokerIdentity { private boolean brokerTopicEnable = true; @ImportantField private boolean autoCreateSubscriptionGroup = true; + /** + * Maximum subscription group name length (default 120, capped at 255). + * Callers must keep retry topics within 255 characters. Groups over 122 characters cannot write to + * {@code %DLQ%} because broker writes reject topics over 127 bytes. + */ + private int subscriptionGroupMaxLength = 120; private String messageStorePlugIn = ""; private static final int PROCESSOR_NUMBER = Runtime.getRuntime().availableProcessors(); @@ -1053,6 +1059,14 @@ public void setAutoCreateSubscriptionGroup(boolean autoCreateSubscriptionGroup) this.autoCreateSubscriptionGroup = autoCreateSubscriptionGroup; } + public int getSubscriptionGroupMaxLength() { + return subscriptionGroupMaxLength; + } + + public void setSubscriptionGroupMaxLength(int subscriptionGroupMaxLength) { + this.subscriptionGroupMaxLength = subscriptionGroupMaxLength; + } + public String getBrokerConfigPath() { return brokerConfigPath; } diff --git a/common/src/main/java/org/apache/rocketmq/common/topic/TopicValidator.java b/common/src/main/java/org/apache/rocketmq/common/topic/TopicValidator.java index ade6289c67f..a2a42da385c 100644 --- a/common/src/main/java/org/apache/rocketmq/common/topic/TopicValidator.java +++ b/common/src/main/java/org/apache/rocketmq/common/topic/TopicValidator.java @@ -140,6 +140,14 @@ public static ValidateResult validateTopic(String topic) { } public static ValidateResult validateGroup(String group) { + return validateGroup(group, GROUP_MAX_LENGTH); + } + + /** + * Validates a group with a custom limit. Non-positive values use the default; values above 255 are capped. + */ + public static ValidateResult validateGroup(String group, int maxLength) { + int groupMaxLength = maxLength <= 0 ? GROUP_MAX_LENGTH : Math.min(maxLength, RETRY_OR_DLQ_TOPIC_MAX_LENGTH); if (UtilAll.isBlank(group)) { return new ValidateResult(false, "The specified group is blank."); @@ -150,8 +158,8 @@ public static ValidateResult validateGroup(String group) { return new ValidateResult(false, falseRemark); } - if (group.length() > GROUP_MAX_LENGTH) { - String falseRemark = "The specified group: " + group + ", is longer than group max length: " + GROUP_MAX_LENGTH; + if (group.length() > groupMaxLength) { + String falseRemark = "The specified group: " + group + ", is longer than group max length: " + groupMaxLength; return new ValidateResult(false, falseRemark); } diff --git a/common/src/test/java/org/apache/rocketmq/common/topic/TopicValidatorTest.java b/common/src/test/java/org/apache/rocketmq/common/topic/TopicValidatorTest.java index 0541c7f6d2e..40bdccd6e87 100644 --- a/common/src/test/java/org/apache/rocketmq/common/topic/TopicValidatorTest.java +++ b/common/src/test/java/org/apache/rocketmq/common/topic/TopicValidatorTest.java @@ -85,6 +85,48 @@ public void testGroupValidator__NotPass() { res = TopicValidator.validateGroup(generateString2(121)); assertThat(res.isValid()).isFalse(); + assertThat(res.getRemark()).contains("group max length: 120"); + } + + @Test + public void testGroupValidatorWithMaxLength() { + assertThat(TopicValidator.validateGroup(generateString2(127), 255).isValid()).isTrue(); + assertThat(TopicValidator.validateGroup(generateString2(255), 255).isValid()).isTrue(); + + TopicValidator.ValidateResult res = TopicValidator.validateGroup(generateString2(256), 255); + assertThat(res.isValid()).isFalse(); + assertThat(res.getRemark()).contains("group max length: 255"); + } + + @Test + public void testGroupValidatorWithMaxLength_CappedAt255() { + assertThat(TopicValidator.validateGroup(generateString2(255), 300).isValid()).isTrue(); + + TopicValidator.ValidateResult res = TopicValidator.validateGroup(generateString2(256), 300); + assertThat(res.isValid()).isFalse(); + assertThat(res.getRemark()).contains("group max length: 255"); + } + + @Test + public void testGroupValidatorWithMaxLength_FallbackToDefault() { + for (int maxLength : new int[] {0, -1}) { + assertThat(TopicValidator.validateGroup(generateString2(120), maxLength).isValid()).isTrue(); + + TopicValidator.ValidateResult res = TopicValidator.validateGroup(generateString2(121), maxLength); + assertThat(res.isValid()).isFalse(); + assertThat(res.getRemark()).contains("group max length: 120"); + } + } + + @Test + public void testGroupValidatorWithMaxLength_BlankAndIllegal() { + TopicValidator.ValidateResult res = TopicValidator.validateGroup("", 255); + assertThat(res.isValid()).isFalse(); + assertThat(res.getRemark()).isEqualTo(TopicValidator.validateGroup("").getRemark()); + + res = TopicValidator.validateGroup("../GroupTest", 255); + assertThat(res.isValid()).isFalse(); + assertThat(res.getRemark()).isEqualTo(TopicValidator.validateGroup("../GroupTest").getRemark()); } @Test From d123846d53ebd9badc821bdfe873eb8b6896c89e Mon Sep 17 00:00:00 2001 From: Quan Date: Tue, 27 Oct 2026 09:41:04 +0800 Subject: [PATCH 2/3] Revert "[ISSUE #11466] Make broker-side subscription group max length configurable" This reverts commit 5a8c3adaaa9beac2237cd243f661564dcdb1d4bb. --- .../processor/AdminBrokerProcessor.java | 6 +- .../SubscriptionGroupManager.java | 3 +- .../processor/AdminBrokerProcessorTest.java | 57 ------------------- .../SubscriptionGroupManagerTest.java | 22 ------- .../apache/rocketmq/common/BrokerConfig.java | 14 ----- .../rocketmq/common/topic/TopicValidator.java | 12 +--- .../common/topic/TopicValidatorTest.java | 42 -------------- 7 files changed, 5 insertions(+), 151 deletions(-) diff --git a/broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java b/broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java index f7099087d30..604e3e18ca4 100644 --- a/broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java +++ b/broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java @@ -1673,8 +1673,7 @@ private RemotingCommand updateAndCreateSubscriptionGroup(ChannelHandlerContext c SubscriptionGroupConfig config = RemotingSerializable.decode(request.getBody(), SubscriptionGroupConfig.class); if (null != config) { - TopicValidator.ValidateResult result = TopicValidator.validateGroup(config.getGroupName(), - this.brokerController.getBrokerConfig().getSubscriptionGroupMaxLength()); + TopicValidator.ValidateResult result = TopicValidator.validateGroup(config.getGroupName()); if (!result.isValid()) { response.setCode(ResponseCode.INVALID_PARAMETER); response.setRemark(result.getRemark()); @@ -1708,8 +1707,7 @@ private RemotingCommand updateAndCreateSubscriptionGroupList(ChannelHandlerConte final StringBuilder builder = new StringBuilder(); for (SubscriptionGroupConfig config : groupConfigList) { - TopicValidator.ValidateResult result = TopicValidator.validateGroup(config.getGroupName(), - this.brokerController.getBrokerConfig().getSubscriptionGroupMaxLength()); + TopicValidator.ValidateResult result = TopicValidator.validateGroup(config.getGroupName()); if (!result.isValid()) { response.setCode(ResponseCode.INVALID_PARAMETER); response.setRemark(result.getRemark()); diff --git a/broker/src/main/java/org/apache/rocketmq/broker/subscription/SubscriptionGroupManager.java b/broker/src/main/java/org/apache/rocketmq/broker/subscription/SubscriptionGroupManager.java index 10fd141a802..b37c35051aa 100644 --- a/broker/src/main/java/org/apache/rocketmq/broker/subscription/SubscriptionGroupManager.java +++ b/broker/src/main/java/org/apache/rocketmq/broker/subscription/SubscriptionGroupManager.java @@ -260,8 +260,7 @@ public SubscriptionGroupConfig findSubscriptionGroupConfig(final String group) { if (null == subscriptionGroupConfig) { if (brokerController.getBrokerConfig().isAutoCreateSubscriptionGroup() || MixAll.isSysConsumerGroupAndEnableCreate(group, brokerController.getBrokerConfig().isEnableCreateSysGroup())) { - TopicValidator.ValidateResult result = TopicValidator.validateGroup(group, - brokerController.getBrokerConfig().getSubscriptionGroupMaxLength()); + TopicValidator.ValidateResult result = TopicValidator.validateGroup(group); if (!result.isValid()) { return null; } diff --git a/broker/src/test/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessorTest.java b/broker/src/test/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessorTest.java index de3ef93bae0..a573a25211c 100644 --- a/broker/src/test/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessorTest.java +++ b/broker/src/test/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessorTest.java @@ -1114,63 +1114,6 @@ public void testUpdateAndCreateSubscriptionGroup() throws RemotingCommandExcepti assertThat(response.getCode()).isEqualTo(ResponseCode.SUCCESS); } - @Test - public void testUpdateAndCreateSubscriptionGroupWithConfigurableMaxLength() throws RemotingCommandException { - String groupName = StringUtils.repeat('a', 121); - SubscriptionGroupConfig subscriptionGroupConfig = new SubscriptionGroupConfig(); - subscriptionGroupConfig.setGroupName(groupName); - RemotingCommand request = RemotingCommand.createRequestCommand(RequestCode.UPDATE_AND_CREATE_SUBSCRIPTIONGROUP, null); - request.setBody(JSON.toJSON(subscriptionGroupConfig).toString().getBytes(StandardCharsets.UTF_8)); - - RemotingCommand response = adminBrokerProcessor.processRequest(handlerContext, request); - assertThat(response.getCode()).isEqualTo(ResponseCode.INVALID_PARAMETER); - assertFalse(brokerController.getSubscriptionGroupManager().getSubscriptionGroupTable().containsKey(groupName)); - - brokerController.getBrokerConfig().setSubscriptionGroupMaxLength(255); - response = adminBrokerProcessor.processRequest(handlerContext, request); - assertThat(response.getCode()).isEqualTo(ResponseCode.SUCCESS); - assertTrue(brokerController.getSubscriptionGroupManager().getSubscriptionGroupTable().containsKey(groupName)); - } - - @Test - public void testUpdateAndCreateSubscriptionGroupListWithConfigurableMaxLength() throws RemotingCommandException { - String groupName = StringUtils.repeat('b', 121); - SubscriptionGroupConfig subscriptionGroupConfig = new SubscriptionGroupConfig(); - subscriptionGroupConfig.setGroupName(groupName); - SubscriptionGroupList subscriptionGroupList = - new SubscriptionGroupList(Collections.singletonList(subscriptionGroupConfig)); - RemotingCommand request = RemotingCommand.createRequestCommand(RequestCode.UPDATE_AND_CREATE_SUBSCRIPTIONGROUP_LIST, null); - request.setBody(subscriptionGroupList.encode()); - - RemotingCommand response = adminBrokerProcessor.processRequest(handlerContext, request); - assertThat(response.getCode()).isEqualTo(ResponseCode.INVALID_PARAMETER); - assertFalse(brokerController.getSubscriptionGroupManager().getSubscriptionGroupTable().containsKey(groupName)); - - brokerController.getBrokerConfig().setSubscriptionGroupMaxLength(255); - response = adminBrokerProcessor.processRequest(handlerContext, request); - assertThat(response.getCode()).isEqualTo(ResponseCode.SUCCESS); - assertTrue(brokerController.getSubscriptionGroupManager().getSubscriptionGroupTable().containsKey(groupName)); - } - - @Test - public void testUpdateAndCreateSubscriptionGroupListRejectsWholeBatchWhenOneTooLong() throws RemotingCommandException { - brokerController.getBrokerConfig().setSubscriptionGroupMaxLength(255); - String validGroup = StringUtils.repeat('c', 127); - String tooLongGroup = StringUtils.repeat('d', 256); - SubscriptionGroupConfig validConfig = new SubscriptionGroupConfig(); - validConfig.setGroupName(validGroup); - SubscriptionGroupConfig tooLongConfig = new SubscriptionGroupConfig(); - tooLongConfig.setGroupName(tooLongGroup); - SubscriptionGroupList subscriptionGroupList = new SubscriptionGroupList(Arrays.asList(validConfig, tooLongConfig)); - RemotingCommand request = RemotingCommand.createRequestCommand(RequestCode.UPDATE_AND_CREATE_SUBSCRIPTIONGROUP_LIST, null); - request.setBody(subscriptionGroupList.encode()); - - RemotingCommand response = adminBrokerProcessor.processRequest(handlerContext, request); - assertThat(response.getCode()).isEqualTo(ResponseCode.INVALID_PARAMETER); - assertFalse(brokerController.getSubscriptionGroupManager().getSubscriptionGroupTable().containsKey(validGroup)); - assertFalse(brokerController.getSubscriptionGroupManager().getSubscriptionGroupTable().containsKey(tooLongGroup)); - } - @Test public void testUpdateAndCreateSubscriptionGroupRejectsEmptyLiteBindTopic() { String groupName = "GID-EMPTY-LITE-BIND"; diff --git a/broker/src/test/java/org/apache/rocketmq/broker/subscription/SubscriptionGroupManagerTest.java b/broker/src/test/java/org/apache/rocketmq/broker/subscription/SubscriptionGroupManagerTest.java index 3abab57fa97..9f709f6502a 100644 --- a/broker/src/test/java/org/apache/rocketmq/broker/subscription/SubscriptionGroupManagerTest.java +++ b/broker/src/test/java/org/apache/rocketmq/broker/subscription/SubscriptionGroupManagerTest.java @@ -27,9 +27,7 @@ import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.atomic.AtomicLong; -import org.apache.commons.lang3.StringUtils; import org.apache.rocketmq.broker.BrokerController; -import org.apache.rocketmq.common.BrokerConfig; import org.apache.rocketmq.common.MixAll; import org.apache.rocketmq.common.SubscriptionGroupAttributes; import org.apache.rocketmq.common.attribute.BooleanAttribute; @@ -120,26 +118,6 @@ public void updateSubscriptionGroupConfig() { private boolean notToBeExecuted() { return MixAll.isMac(); } - - @Test - public void testFindSubscriptionGroupConfigAutoCreateWithConfigurableMaxLength() { - BrokerConfig brokerConfig = new BrokerConfig(); - brokerConfig.setAutoCreateSubscriptionGroup(true); - Mockito.when(brokerControllerMock.getBrokerConfig()).thenReturn(brokerConfig); - Mockito.doNothing().when(subscriptionGroupManager).persist(); - - String groupName = StringUtils.repeat('a', 121); - assertThat(subscriptionGroupManager.findSubscriptionGroupConfig(groupName)).isNull(); - assertThat(subscriptionGroupManager.getSubscriptionGroupTable()).doesNotContainKey(groupName); - - brokerConfig.setSubscriptionGroupMaxLength(255); - SubscriptionGroupConfig result = subscriptionGroupManager.findSubscriptionGroupConfig(groupName); - assertThat(result).isNotNull(); - assertThat(result.getGroupName()).isEqualTo(groupName); - assertThat(subscriptionGroupManager.findSubscriptionGroupConfig(StringUtils.repeat('b', 256))).isNull(); - verify(subscriptionGroupManager).persist(); - } - @Test public void testUpdateSubscriptionGroupConfigList_NullConfigList() { if (notToBeExecuted()) { diff --git a/common/src/main/java/org/apache/rocketmq/common/BrokerConfig.java b/common/src/main/java/org/apache/rocketmq/common/BrokerConfig.java index 05dc50b4bda..8e49839e3e0 100644 --- a/common/src/main/java/org/apache/rocketmq/common/BrokerConfig.java +++ b/common/src/main/java/org/apache/rocketmq/common/BrokerConfig.java @@ -56,12 +56,6 @@ public class BrokerConfig extends BrokerIdentity { private boolean brokerTopicEnable = true; @ImportantField private boolean autoCreateSubscriptionGroup = true; - /** - * Maximum subscription group name length (default 120, capped at 255). - * Callers must keep retry topics within 255 characters. Groups over 122 characters cannot write to - * {@code %DLQ%} because broker writes reject topics over 127 bytes. - */ - private int subscriptionGroupMaxLength = 120; private String messageStorePlugIn = ""; private static final int PROCESSOR_NUMBER = Runtime.getRuntime().availableProcessors(); @@ -1059,14 +1053,6 @@ public void setAutoCreateSubscriptionGroup(boolean autoCreateSubscriptionGroup) this.autoCreateSubscriptionGroup = autoCreateSubscriptionGroup; } - public int getSubscriptionGroupMaxLength() { - return subscriptionGroupMaxLength; - } - - public void setSubscriptionGroupMaxLength(int subscriptionGroupMaxLength) { - this.subscriptionGroupMaxLength = subscriptionGroupMaxLength; - } - public String getBrokerConfigPath() { return brokerConfigPath; } diff --git a/common/src/main/java/org/apache/rocketmq/common/topic/TopicValidator.java b/common/src/main/java/org/apache/rocketmq/common/topic/TopicValidator.java index a2a42da385c..ade6289c67f 100644 --- a/common/src/main/java/org/apache/rocketmq/common/topic/TopicValidator.java +++ b/common/src/main/java/org/apache/rocketmq/common/topic/TopicValidator.java @@ -140,14 +140,6 @@ public static ValidateResult validateTopic(String topic) { } public static ValidateResult validateGroup(String group) { - return validateGroup(group, GROUP_MAX_LENGTH); - } - - /** - * Validates a group with a custom limit. Non-positive values use the default; values above 255 are capped. - */ - public static ValidateResult validateGroup(String group, int maxLength) { - int groupMaxLength = maxLength <= 0 ? GROUP_MAX_LENGTH : Math.min(maxLength, RETRY_OR_DLQ_TOPIC_MAX_LENGTH); if (UtilAll.isBlank(group)) { return new ValidateResult(false, "The specified group is blank."); @@ -158,8 +150,8 @@ public static ValidateResult validateGroup(String group, int maxLength) { return new ValidateResult(false, falseRemark); } - if (group.length() > groupMaxLength) { - String falseRemark = "The specified group: " + group + ", is longer than group max length: " + groupMaxLength; + if (group.length() > GROUP_MAX_LENGTH) { + String falseRemark = "The specified group: " + group + ", is longer than group max length: " + GROUP_MAX_LENGTH; return new ValidateResult(false, falseRemark); } diff --git a/common/src/test/java/org/apache/rocketmq/common/topic/TopicValidatorTest.java b/common/src/test/java/org/apache/rocketmq/common/topic/TopicValidatorTest.java index 40bdccd6e87..0541c7f6d2e 100644 --- a/common/src/test/java/org/apache/rocketmq/common/topic/TopicValidatorTest.java +++ b/common/src/test/java/org/apache/rocketmq/common/topic/TopicValidatorTest.java @@ -85,48 +85,6 @@ public void testGroupValidator__NotPass() { res = TopicValidator.validateGroup(generateString2(121)); assertThat(res.isValid()).isFalse(); - assertThat(res.getRemark()).contains("group max length: 120"); - } - - @Test - public void testGroupValidatorWithMaxLength() { - assertThat(TopicValidator.validateGroup(generateString2(127), 255).isValid()).isTrue(); - assertThat(TopicValidator.validateGroup(generateString2(255), 255).isValid()).isTrue(); - - TopicValidator.ValidateResult res = TopicValidator.validateGroup(generateString2(256), 255); - assertThat(res.isValid()).isFalse(); - assertThat(res.getRemark()).contains("group max length: 255"); - } - - @Test - public void testGroupValidatorWithMaxLength_CappedAt255() { - assertThat(TopicValidator.validateGroup(generateString2(255), 300).isValid()).isTrue(); - - TopicValidator.ValidateResult res = TopicValidator.validateGroup(generateString2(256), 300); - assertThat(res.isValid()).isFalse(); - assertThat(res.getRemark()).contains("group max length: 255"); - } - - @Test - public void testGroupValidatorWithMaxLength_FallbackToDefault() { - for (int maxLength : new int[] {0, -1}) { - assertThat(TopicValidator.validateGroup(generateString2(120), maxLength).isValid()).isTrue(); - - TopicValidator.ValidateResult res = TopicValidator.validateGroup(generateString2(121), maxLength); - assertThat(res.isValid()).isFalse(); - assertThat(res.getRemark()).contains("group max length: 120"); - } - } - - @Test - public void testGroupValidatorWithMaxLength_BlankAndIllegal() { - TopicValidator.ValidateResult res = TopicValidator.validateGroup("", 255); - assertThat(res.isValid()).isFalse(); - assertThat(res.getRemark()).isEqualTo(TopicValidator.validateGroup("").getRemark()); - - res = TopicValidator.validateGroup("../GroupTest", 255); - assertThat(res.isValid()).isFalse(); - assertThat(res.getRemark()).isEqualTo(TopicValidator.validateGroup("../GroupTest").getRemark()); } @Test From c6591414ccd6b8f29eeb821378771b3e3c5ea147 Mon Sep 17 00:00:00 2001 From: Quan Date: Tue, 27 Oct 2026 09:59:12 +0800 Subject: [PATCH 3/3] [ISSUE #11466] Extract group max length into a field in TopicValidator validateGroup compares against the compile-time constant GROUP_MAX_LENGTH directly; extract it into a field with a guarded setter so the default of 120 and the retry/DLQ topic length invariant are preserved. All three group creation paths keep the same validation behavior without per-call-site changes. Verification: TopicValidatorTest 10/10 passed, including the new cases covering the 255 boundary, the cap, and the fallback to the default. --- .../rocketmq/common/topic/TopicValidator.java | 11 ++++++++-- .../common/topic/TopicValidatorTest.java | 21 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/common/src/main/java/org/apache/rocketmq/common/topic/TopicValidator.java b/common/src/main/java/org/apache/rocketmq/common/topic/TopicValidator.java index ade6289c67f..f1b59848c9e 100644 --- a/common/src/main/java/org/apache/rocketmq/common/topic/TopicValidator.java +++ b/common/src/main/java/org/apache/rocketmq/common/topic/TopicValidator.java @@ -48,6 +48,8 @@ public class TopicValidator { private static final int GROUP_MAX_LENGTH = 120; private static final int RETRY_OR_DLQ_TOPIC_MAX_LENGTH = 255; + private static volatile int groupMaxLength = GROUP_MAX_LENGTH; + private static final Set SYSTEM_TOPIC_SET = new HashSet<>(); /** @@ -139,6 +141,11 @@ public static ValidateResult validateTopic(String topic) { return new ValidateResult(true, ""); } + public static void setGroupMaxLength(int maxLength) { + groupMaxLength = maxLength <= 0 ? GROUP_MAX_LENGTH + : Math.min(maxLength, RETRY_OR_DLQ_TOPIC_MAX_LENGTH); + } + public static ValidateResult validateGroup(String group) { if (UtilAll.isBlank(group)) { @@ -150,8 +157,8 @@ public static ValidateResult validateGroup(String group) { return new ValidateResult(false, falseRemark); } - if (group.length() > GROUP_MAX_LENGTH) { - String falseRemark = "The specified group: " + group + ", is longer than group max length: " + GROUP_MAX_LENGTH; + if (group.length() > groupMaxLength) { + String falseRemark = "The specified group: " + group + ", is longer than group max length: " + groupMaxLength; return new ValidateResult(false, falseRemark); } diff --git a/common/src/test/java/org/apache/rocketmq/common/topic/TopicValidatorTest.java b/common/src/test/java/org/apache/rocketmq/common/topic/TopicValidatorTest.java index 0541c7f6d2e..879b9a5f718 100644 --- a/common/src/test/java/org/apache/rocketmq/common/topic/TopicValidatorTest.java +++ b/common/src/test/java/org/apache/rocketmq/common/topic/TopicValidatorTest.java @@ -87,6 +87,27 @@ public void testGroupValidator__NotPass() { assertThat(res.isValid()).isFalse(); } + @Test + public void testGroupValidator_WithCustomMaxLength() { + try { + TopicValidator.setGroupMaxLength(255); + assertThat(TopicValidator.validateGroup(generateString2(255)).isValid()).isTrue(); + assertThat(TopicValidator.validateGroup(generateString2(256)).isValid()).isFalse(); + + // values above 255 are capped at 255 + TopicValidator.setGroupMaxLength(300); + assertThat(TopicValidator.validateGroup(generateString2(255)).isValid()).isTrue(); + assertThat(TopicValidator.validateGroup(generateString2(256)).isValid()).isFalse(); + + // non-positive values fall back to the default of 120 + TopicValidator.setGroupMaxLength(0); + assertThat(TopicValidator.validateGroup(generateString2(120)).isValid()).isTrue(); + assertThat(TopicValidator.validateGroup(generateString2(121)).isValid()).isFalse(); + } finally { + TopicValidator.setGroupMaxLength(120); + } + } + @Test public void testAddSystemTopic() { String topic = "SYSTEM_TOPIC_TEST";