RongtongJin commented on a change in pull request #1985:
URL: https://github.com/apache/rocketmq/pull/1985#discussion_r432421381



##########
File path: 
common/src/main/java/org/apache/rocketmq/common/topic/TopicValidator.java
##########
@@ -0,0 +1,131 @@
+/*
+ * 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.rocketmq.common.topic;
+
+import org.apache.rocketmq.common.UtilAll;
+import org.apache.rocketmq.common.protocol.ResponseCode;
+import org.apache.rocketmq.remoting.protocol.RemotingCommand;
+
+import java.util.HashSet;
+import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+public class TopicValidator {
+
+    public static final String AUTO_CREATE_TOPIC_KEY_TOPIC = "TBW102"; // Will 
be created at broker when isAutoCreateTopicEnable
+    public static final String RMQ_SYS_SCHEDULE_TOPIC = "SCHEDULE_TOPIC_XXXX";
+    public static final String RMQ_SYS_BENCHMARK_TOPIC = "BenchmarkTest";
+    public static final String RMQ_SYS_TRANS_HALF_TOPIC = 
"RMQ_SYS_TRANS_HALF_TOPIC";
+    public static final String RMQ_SYS_TRACE_TOPIC = "RMQ_SYS_TRACE_TOPIC";
+    public static final String RMQ_SYS_TRANS_OP_HALF_TOPIC = 
"RMQ_SYS_TRANS_OP_HALF_TOPIC";
+    public static final String RMQ_SYS_TRANS_CHECK_MAX_TIME_TOPIC = 
"TRANS_CHECK_MAX_TIME_TOPIC";
+    public static final String RMQ_SYS_SELF_TEST_TOPIC = "SELF_TEST_TOPIC";
+    public static final String RMQ_SYS_OFFSET_MOVED_EVENT = 
"OFFSET_MOVED_EVENT";
+
+    public static final String SYSTEM_TOPIC_PREFIX = "rmq_sys_";
+
+    private static final String VALID_PATTERN_STR = "^[%|a-zA-Z0-9_-]+$";
+    private static final Pattern PATTERN = Pattern.compile(VALID_PATTERN_STR);
+    private static final int TOPIC_MAX_LENGTH = 127;
+
+    private static final Set<String> SYSTEM_TOPIC_SET = new HashSet<String>();
+
+    /**
+     * Topics'set which client can not send msg!
+     */
+    private static final Set<String> CLIENT_BLACKLIST_TOPIC_SET = new 
HashSet<String>();
+
+    static {
+        SYSTEM_TOPIC_SET.add(AUTO_CREATE_TOPIC_KEY_TOPIC);
+        SYSTEM_TOPIC_SET.add(RMQ_SYS_SCHEDULE_TOPIC);
+        SYSTEM_TOPIC_SET.add(RMQ_SYS_BENCHMARK_TOPIC);
+        SYSTEM_TOPIC_SET.add(RMQ_SYS_TRANS_HALF_TOPIC);
+        SYSTEM_TOPIC_SET.add(RMQ_SYS_TRACE_TOPIC);
+        SYSTEM_TOPIC_SET.add(RMQ_SYS_TRANS_OP_HALF_TOPIC);
+        SYSTEM_TOPIC_SET.add(RMQ_SYS_TRANS_CHECK_MAX_TIME_TOPIC);
+        SYSTEM_TOPIC_SET.add(RMQ_SYS_SELF_TEST_TOPIC);
+        SYSTEM_TOPIC_SET.add(RMQ_SYS_OFFSET_MOVED_EVENT);
+
+        CLIENT_BLACKLIST_TOPIC_SET.add(RMQ_SYS_SCHEDULE_TOPIC);
+    }
+
+    private static boolean regularExpressionMatcher(String origin, Pattern 
pattern) {
+        if (pattern == null) {
+            return true;
+        }
+        Matcher matcher = pattern.matcher(origin);
+        return matcher.matches();
+    }
+
+    public static boolean validateTopic(String topic, RemotingCommand 
response) {
+
+        if (UtilAll.isBlank(topic)) {
+            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setRemark("The specified topic is blank.");
+            return false;
+        }
+
+        if (!regularExpressionMatcher(topic, PATTERN)) {
+            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setRemark("The specified topic contains illegal 
characters, allowing only " + VALID_PATTERN_STR);
+            return false;
+        }
+
+        if (topic.length() > TOPIC_MAX_LENGTH) {
+            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setRemark("The specified topic is longer than topic max 
length.");
+            return false;
+        }
+
+        return true;
+    }
+
+    public static boolean validateSystemTopic(String topic, RemotingCommand 
response) {
+        if (isSystemTopic(topic)) {
+            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setRemark("The topic[" + topic + "] is conflict with 
system topic.");
+            return false;
+        }
+        return true;
+    }
+
+    public static boolean isSystemTopic(String topic) {
+        return SYSTEM_TOPIC_SET.contains(topic) || 
topic.startsWith(SYSTEM_TOPIC_PREFIX);
+    }
+
+    public static boolean isBlacklistTopic(String topic) {
+        return CLIENT_BLACKLIST_TOPIC_SET.contains(topic);
+    }
+
+    public static boolean validateBlacklistTopic(String topic, RemotingCommand 
response) {
+        if (isBlacklistTopic(topic)) {
+            response.setCode(ResponseCode.NO_PERMISSION);
+            response.setRemark("Sending message to topic[" + topic + "] is 
forbidden.");
+            return false;
+        }
+        return true;

Review comment:
       It would be better to reconsider the naming of these methods. The 
difference between `validateSystemTopic` and `isSystemTopic` are quite 
confusing, same for  `isBlacklistTopic` and `validateBlacklistTopic`. For 
example, when a topic is system topic, but validateSystemTopic return false.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to