You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by yu...@apache.org on 2025/06/10 13:00:25 UTC
(pulsar) branch branch-3.0 updated: [fix] [broker] No longer allow creating subscription that contains slash (#23594)
This is an automated email from the ASF dual-hosted git repository.
yubiao pushed a commit to branch branch-3.0
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/branch-3.0 by this push:
new c28b59a4e9d [fix] [broker] No longer allow creating subscription that contains slash (#23594)
c28b59a4e9d is described below
commit c28b59a4e9d1d4dfa4e9383774b04ae9b2b2dd83
Author: fengyubiao <yu...@streamnative.io>
AuthorDate: Tue Jun 10 20:37:50 2025 +0800
[fix] [broker] No longer allow creating subscription that contains slash (#23594)
(cherry picked from commit 7d54f40d9b5bb8d44468983ad7b04c17c7f6cf1b)
---
conf/broker.conf | 4 +++
conf/standalone.conf | 4 +++
.../apache/pulsar/broker/ServiceConfiguration.java | 5 ++++
.../pulsar/broker/admin/v2/PersistentTopics.java | 12 ++++++++
.../apache/pulsar/broker/service/ServerCnx.java | 19 +++++++++++-
.../client/api/SimpleProducerConsumerTest.java | 34 ++++++++++++++++++++++
.../apache/pulsar/common/naming/NamedEntity.java | 8 +++--
7 files changed, 83 insertions(+), 3 deletions(-)
diff --git a/conf/broker.conf b/conf/broker.conf
index a5c15cce46b..2139e57fc63 100644
--- a/conf/broker.conf
+++ b/conf/broker.conf
@@ -193,6 +193,10 @@ allowAutoTopicCreation=true
# The type of topic that is allowed to be automatically created.(partitioned/non-partitioned)
allowAutoTopicCreationType=non-partitioned
+# If 'strictSubscriptionNameVerification' is true, the new subscription name can only contain (a-zA-Z_0-9) and these
+# special chars -=:.
+strictlyVerifySubscriptionName=false
+
# Enable subscription auto creation if new consumer connected (disable auto creation with value false)
allowAutoSubscriptionCreation=true
diff --git a/conf/standalone.conf b/conf/standalone.conf
index f112b301450..e4532021cc5 100644
--- a/conf/standalone.conf
+++ b/conf/standalone.conf
@@ -1178,6 +1178,10 @@ allowAutoTopicCreation=true
# The type of topic that is allowed to be automatically created.(partitioned/non-partitioned)
allowAutoTopicCreationType=non-partitioned
+# If 'strictSubscriptionNameVerification' is true, the new subscription name can only contain (a-zA-Z_0-9) and these
+# special chars -=:.
+strictlyVerifySubscriptionName=false
+
# Enable subscription auto creation if new consumer connected (disable auto creation with value false)
allowAutoSubscriptionCreation=true
diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
index ab47ba7ecc7..8f170b4337b 100644
--- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
+++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
@@ -2050,6 +2050,11 @@ public class ServiceConfiguration implements PulsarConfiguration {
doc = "The type of topic that is allowed to be automatically created.(partitioned/non-partitioned)"
)
private TopicType allowAutoTopicCreationType = TopicType.NON_PARTITIONED;
+ @FieldContext(category = CATEGORY_SERVER, dynamic = true,
+ doc = "If 'strictSubscriptionNameVerification' is true, the new subscription name can only contain"
+ + " (a-zA-Z_0-9) and these special chars -=:."
+ )
+ private boolean strictlyVerifySubscriptionName = false;
@FieldContext(
category = CATEGORY_STORAGE_ML,
dynamic = true,
diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
index e822761f33c..98fdc475364 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
@@ -58,6 +58,7 @@ import org.apache.pulsar.client.api.SubscriptionType;
import org.apache.pulsar.client.impl.MessageIdImpl;
import org.apache.pulsar.client.impl.ResetCursorData;
import org.apache.pulsar.common.api.proto.CommandSubscribe;
+import org.apache.pulsar.common.naming.NamedEntity;
import org.apache.pulsar.common.naming.PartitionedManagedLedgerInfo;
import org.apache.pulsar.common.partition.PartitionedTopicMetadata;
import org.apache.pulsar.common.policies.data.AuthAction;
@@ -1686,6 +1687,17 @@ public class PersistentTopics extends PersistentTopicsBase {
) {
try {
validateTopicName(tenant, namespace, topic);
+ String decodedSubName = decode(encodedSubName);
+ // If subscription is as "a/b". The url of HTTP API that defined as
+ // "{tenant}/{namespace}/{topic}/{subscription}" will be like below:
+ // "public/default/tp/a/b", then the broker will assume it is a topic that
+ // using the old rule "{tenant}/{cluster}/{namespace}/{topic}/{subscription}".
+ // So denied to create a subscription that contains "/".
+ if (pulsar().getConfig().isStrictlyVerifySubscriptionName()
+ && !NamedEntity.isAllowed(decodedSubName)) {
+ throw new RestException(Response.Status.BAD_REQUEST, "Please let the subscription only contains"
+ + " '/w(a-zA-Z_0-9)' or '_', the current value is " + decodedSubName);
+ }
if (!topicName.isPersistent()) {
throw new RestException(Response.Status.BAD_REQUEST, "Create subscription on non-persistent topic "
+ "can only be done through client");
diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
index 299d07c6839..13af69f3eb8 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
@@ -151,6 +151,7 @@ import org.apache.pulsar.common.compression.CompressionCodec;
import org.apache.pulsar.common.compression.CompressionCodecProvider;
import org.apache.pulsar.common.intercept.InterceptException;
import org.apache.pulsar.common.naming.Metadata;
+import org.apache.pulsar.common.naming.NamedEntity;
import org.apache.pulsar.common.naming.NamespaceName;
import org.apache.pulsar.common.naming.SystemTopicNames;
import org.apache.pulsar.common.naming.TopicName;
@@ -1305,9 +1306,25 @@ public class ServerCnx extends PulsarHandler implements TransportCnx {
}
return service.isAllowAutoSubscriptionCreationAsync(topicName)
.thenCompose(isAllowedAutoSubscriptionCreation -> {
+ boolean subscriptionExists =
+ topic.getSubscriptions().containsKey(subscriptionName);
+ // If subscription is as "a/b". The url of HTTP API that defined as
+ // "{tenant}/{namespace}/{topic}/{subscription}" will be like below:
+ // "public/default/tp/a/b", then the broker will assume it is a topic that
+ // using the old rule "{tenant}/{cluster}/{namespace}/{topic}/{subscription}".
+ // So denied to create a subscription that contains "/".
+ if (getBrokerService().pulsar().getConfig().isStrictlyVerifySubscriptionName()
+ && !subscriptionExists
+ && !NamedEntity.isAllowed(subscriptionName)) {
+ return FutureUtil.failedFuture(
+ new BrokerServiceException.NamingException(
+ "Please let the subscription only contains '/w(a-zA-Z_0-9)' or '_',"
+ + " the current value is " + subscriptionName));
+ }
+
boolean rejectSubscriptionIfDoesNotExist = isDurable
&& !isAllowedAutoSubscriptionCreation
- && !topic.getSubscriptions().containsKey(subscriptionName)
+ && !subscriptionExists
&& topic.isPersistent();
if (rejectSubscriptionIfDoesNotExist) {
diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleProducerConsumerTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleProducerConsumerTest.java
index d91f2fefda0..ed10a13b156 100644
--- a/pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleProducerConsumerTest.java
+++ b/pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleProducerConsumerTest.java
@@ -271,6 +271,40 @@ public class SimpleProducerConsumerTest extends ProducerConsumerBase {
}
}
+ @Test(timeOut = 30000)
+ public void testSlashSubscriptionName() throws Exception {
+ // Enable strictlyVerifySubscriptionName.
+ admin.brokers().updateDynamicConfiguration("strictlyVerifySubscriptionName", "true");
+ Awaitility.await().untilAsserted(() -> {
+ assertTrue(pulsar.getConfiguration().isStrictlyVerifySubscriptionName());
+ });
+
+ final String topic = BrokerTestUtil.newUniqueName("my-property/my-ns/tp");
+ admin.topics().createNonPartitionedTopic(topic);
+ try {
+ admin.topics().createSubscription(topic, "a/b", MessageId.earliest);
+ fail("The creation for the subscription that contains '/' should fail");
+ } catch (PulsarAdminException ex) {
+ assertTrue(ex.getMessage().contains("Please let the subscription only contains"));
+ // Expected.
+ }
+ try {
+ pulsarClient.newConsumer().topic(topic).subscriptionName("b/c").subscribe();
+ fail("The creation for the subscription that contains '/' should fail");
+ } catch (PulsarClientException ex) {
+ assertTrue(ex.getMessage().contains("Please let the subscription only contains"));
+ // Expected.
+ }
+ assertEquals(admin.topics().getStats(topic).getSubscriptions().size(), 0);
+
+ // cleanup.
+ admin.topics().delete(topic);
+ admin.brokers().updateDynamicConfiguration("strictlyVerifySubscriptionName", "false");
+ Awaitility.await().untilAsserted(() -> {
+ assertFalse(pulsar.getConfiguration().isStrictlyVerifySubscriptionName());
+ });
+ }
+
@Test(timeOut = 100000)
public void testPublishTimestampBatchEnabled() throws Exception {
diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/naming/NamedEntity.java b/pulsar-common/src/main/java/org/apache/pulsar/common/naming/NamedEntity.java
index bc160350d85..e75b1da6218 100644
--- a/pulsar-common/src/main/java/org/apache/pulsar/common/naming/NamedEntity.java
+++ b/pulsar-common/src/main/java/org/apache/pulsar/common/naming/NamedEntity.java
@@ -33,9 +33,13 @@ public class NamedEntity {
public static final Pattern NAMED_ENTITY_PATTERN = Pattern.compile("^[-=:.\\w]*$");
public static void checkName(String name) throws IllegalArgumentException {
- Matcher m = NAMED_ENTITY_PATTERN.matcher(name);
- if (!m.matches()) {
+ if (!isAllowed(name)) {
throw new IllegalArgumentException("Invalid named entity: " + name);
}
}
+
+ public static boolean isAllowed(String name) {
+ Matcher m = NAMED_ENTITY_PATTERN.matcher(name);
+ return m.matches();
+ }
}
\ No newline at end of file