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 12:57:57 UTC
(pulsar) branch branch-3.3 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.3
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/branch-3.3 by this push:
new 58a60dcec8d [fix] [broker] No longer allow creating subscription that contains slash (#23594)
58a60dcec8d is described below
commit 58a60dcec8d2ee7b73ce1a29ad483d0d6f098838
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 968dc1e5401..32969bf47a2 100644
--- a/conf/broker.conf
+++ b/conf/broker.conf
@@ -198,6 +198,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 7ed61134965..0dd5b6298b7 100644
--- a/conf/standalone.conf
+++ b/conf/standalone.conf
@@ -1189,6 +1189,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 2e51bec1cda..7503a3e68fd 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
@@ -2088,6 +2088,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 ec19c4928b3..06b2507c644 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
@@ -59,6 +59,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;
@@ -1708,6 +1709,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 eeb01157e34..43497692d0f 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
@@ -154,6 +154,7 @@ import org.apache.pulsar.common.compression.CompressionCodecProvider;
import org.apache.pulsar.common.intercept.InterceptException;
import org.apache.pulsar.common.lookup.data.LookupData;
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;
@@ -1338,9 +1339,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 6e022946b02..b58ee1ac91c 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