This is an automated email from the ASF dual-hosted git repository.
Aias00 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shenyu.git
The following commit(s) were added to refs/heads/master by this push:
new 0c8ebc8b31 fix(rewrite): reject unusable rule handles at the admin
write boundary (#7873) (#7874)
0c8ebc8b31 is described below
commit 0c8ebc8b319c4dbde324829a75953a8a6e9869ac
Author: build <[email protected]>
AuthorDate: Wed Oct 21 11:58:01 2026 +0800
fix(rewrite): reject unusable rule handles at the admin write boundary
(#7873) (#7874)
* fix(rewrite): validate rule handles at the admin write boundary (#7873)
- Extract the rewrite applicability check from RewritePluginDataHandler into
shenyu-common's RewriteHandleUtils, so Admin and the gateway validate a
rule
handle with exactly the same code.
- Add RewriteRuleHandleValidator for the admin write boundary and call it
from
RuleServiceImpl#validateRuleHandle, so a rule whose handle the gateway
could
never apply is rejected when it is created, updated or imported instead of
being stored as enabled and silently ignored by the gateway.
- Tests: RewriteHandleUtilsTest, RewriteRuleHandleValidatorTest and new
RuleServiceTest cases, all of which fail before the change.
* fix(rewrite): validate raw patterns and address review feedback
---
.../shenyu/admin/service/impl/RuleServiceImpl.java | 3 +
.../validator/RewriteRuleHandleValidator.java | 62 ++++++++++++++++++
.../shenyu/admin/service/RuleServiceTest.java | 54 ++++++++++++++++
.../validator/RewriteRuleHandleValidatorTest.java | 71 +++++++++++++++++++++
.../shenyu/common/utils/RewriteHandleUtils.java | 73 ++++++++++++++++++++++
.../common/utils/RewriteHandleUtilsTest.java | 67 ++++++++++++++++++++
.../rewrite/handler/RewritePluginDataHandler.java | 23 ++-----
7 files changed, 336 insertions(+), 17 deletions(-)
diff --git
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/RuleServiceImpl.java
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/RuleServiceImpl.java
index 40e8125b92..a7e2838b4e 100644
---
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/RuleServiceImpl.java
+++
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/RuleServiceImpl.java
@@ -54,6 +54,7 @@ import org.apache.shenyu.admin.transfer.ConditionTransfer;
import org.apache.shenyu.admin.utils.Assert;
import org.apache.shenyu.admin.utils.SessionUtil;
import
org.apache.shenyu.admin.validation.validator.AgentGatewayRuleHandleValidator;
+import org.apache.shenyu.admin.validation.validator.RewriteRuleHandleValidator;
import org.apache.shenyu.common.constant.AdminConstants;
import org.apache.shenyu.common.dto.ConditionData;
import org.apache.shenyu.common.dto.RuleData;
@@ -415,6 +416,8 @@ public class RuleServiceImpl implements RuleService {
}
if (PluginEnum.AGENT_GATEWAY.getName().equals(plugin.getName())) {
AgentGatewayRuleHandleValidator.validate(handle);
+ } else if (PluginEnum.REWRITE.getName().equals(plugin.getName())) {
+ RewriteRuleHandleValidator.validate(handle);
}
}
diff --git
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/validation/validator/RewriteRuleHandleValidator.java
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/validation/validator/RewriteRuleHandleValidator.java
new file mode 100644
index 0000000000..1ac4a189b0
--- /dev/null
+++
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/validation/validator/RewriteRuleHandleValidator.java
@@ -0,0 +1,62 @@
+/*
+ * 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.shenyu.admin.validation.validator;
+
+import com.google.gson.JsonSyntaxException;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.shenyu.admin.exception.ShenyuAdminException;
+import org.apache.shenyu.common.dto.convert.rule.RewriteHandle;
+import org.apache.shenyu.common.utils.GsonUtils;
+import org.apache.shenyu.common.utils.RewriteHandleUtils;
+
+import java.util.Objects;
+
+/**
+ * Validates rewrite rule handles before Admin stores them.
+ */
+public final class RewriteRuleHandleValidator {
+
+ private RewriteRuleHandleValidator() {
+ }
+
+ /**
+ * Validate a rewrite rule handle, rejecting the rule before it is
persisted when the gateway
+ * could never apply it.
+ *
+ * @param handle the rule handle supplied to Admin
+ */
+ public static void validate(final String handle) {
+ if (StringUtils.isBlank(handle)) {
+ // an empty handle means the rule does not rewrite, which the
plugin tolerates as well
+ return;
+ }
+ final RewriteHandle rewriteHandle;
+ try {
+ rewriteHandle = GsonUtils.getInstance().fromJson(handle,
RewriteHandle.class);
+ } catch (JsonSyntaxException ex) {
+ throw new ShenyuAdminException("rewrite rule handle must be a
valid JSON object");
+ }
+ if (Objects.isNull(rewriteHandle)) {
+ throw new ShenyuAdminException("rewrite rule handle must be a
valid JSON object");
+ }
+ final String problem =
RewriteHandleUtils.applicabilityProblem(rewriteHandle.getRegex(),
rewriteHandle.getReplace());
+ if (Objects.nonNull(problem)) {
+ throw new ShenyuAdminException("rewrite rule handle can not be
applied, " + problem);
+ }
+ }
+}
diff --git
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/RuleServiceTest.java
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/RuleServiceTest.java
index 6999eae704..1cfc2ad1ae 100644
---
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/RuleServiceTest.java
+++
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/RuleServiceTest.java
@@ -321,6 +321,60 @@ public final class RuleServiceTest {
verify(ruleMapper, never()).insertSelective(any());
}
+ @Test
+ public void testRewriteCreateRejectsUnapplicableHandleBeforeWrite() {
+ mockRewritePlugin();
+ RuleDTO ruleDTO = buildRuleDTO("");
+ ruleDTO.setHandle("{\"regex\":\"test\",\"replace\":\"/v1/$$\"}");
+
+ assertThrows(ShenyuAdminException.class, () ->
ruleService.createOrUpdate(ruleDTO));
+
+ verify(ruleMapper, never()).insertSelective(any());
+ verify(ruleEventPublisher, never()).onCreated(any(), any());
+ }
+
+ @Test
+ public void testRewriteUpdateRejectsUnapplicableHandleBeforeWrite() {
+ mockRewritePlugin();
+ RuleDTO ruleDTO = buildRuleDTO("123");
+ ruleDTO.setHandle("{\"regex\":\"test\",\"replace\":\"/new/$\"}");
+
+ assertThrows(ShenyuAdminException.class, () ->
ruleService.createOrUpdate(ruleDTO));
+
+ verify(ruleMapper, never()).updateSelective(any());
+ verify(ruleEventPublisher, never()).onUpdated(any(), any(), any(),
any());
+ }
+
+ @Test
+ public void testRewriteCreateAcceptsApplicableHandle() {
+ mockRewritePlugin();
+ RuleDTO ruleDTO = buildRuleDTO("");
+ ruleDTO.setHandle("{\"regex\":\"(test)\",\"replace\":\"/new/$1\"}");
+ given(ruleMapper.insertSelective(any())).willReturn(1);
+
+ assertEquals(1, ruleService.createOrUpdate(ruleDTO));
+ verify(ruleEventPublisher).onCreated(any(), any());
+ }
+
+ @Test
+ public void testRewriteImportRejectsUnapplicableHandleBeforeWrite() {
+ mockRewritePlugin();
+ given(ruleMapper.selectAll()).willReturn(Collections.emptyList());
+ RuleDTO ruleDTO = buildRuleDTO("");
+ ruleDTO.setHandle("{\"regex\":\"test\",\"replace\":\"/new/$x\"}");
+
+ assertThrows(ShenyuAdminException.class, () ->
ruleService.importData(Collections.singletonList(ruleDTO)));
+ verify(ruleMapper, never()).insertSelective(any());
+ }
+
+ private void mockRewritePlugin() {
+ given(selectorMapper.selectById("456")).willReturn(buildSelectorDO());
+ given(pluginMapper.selectById("789")).willReturn(PluginDO.builder()
+ .id("789")
+ .name(PluginEnum.REWRITE.getName())
+ .build());
+ }
+
private void mockAgentGatewayPlugin() {
given(selectorMapper.selectById("456")).willReturn(buildSelectorDO());
given(pluginMapper.selectById("789")).willReturn(PluginDO.builder()
diff --git
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/validation/validator/RewriteRuleHandleValidatorTest.java
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/validation/validator/RewriteRuleHandleValidatorTest.java
new file mode 100644
index 0000000000..95c93957f0
--- /dev/null
+++
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/validation/validator/RewriteRuleHandleValidatorTest.java
@@ -0,0 +1,71 @@
+/*
+ * 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.shenyu.admin.validation.validator;
+
+import org.apache.shenyu.admin.exception.ShenyuAdminException;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Tests for rewrite rule handle validation at the Admin write boundary.
+ */
+public final class RewriteRuleHandleValidatorTest {
+
+ @Test
+ public void testApplicableHandles() {
+ assertDoesNotThrow(() -> RewriteRuleHandleValidator.validate(null));
+ assertDoesNotThrow(() -> RewriteRuleHandleValidator.validate(""));
+ assertDoesNotThrow(() -> RewriteRuleHandleValidator.validate("{}"));
+ assertDoesNotThrow(() ->
RewriteRuleHandleValidator.validate("{\"regex\":\"(test)\",\"replace\":\"/new/$1\"}"));
+ assertDoesNotThrow(() ->
RewriteRuleHandleValidator.validate("{\"regex\":\"/shenyu/test\",\"replace\":\"/new\"}"));
+ assertDoesNotThrow(() ->
RewriteRuleHandleValidator.validate("{\"regex\":\"/shenyu/{id}\",\"replace\":\"/target/{id}\"}"));
+ assertDoesNotThrow(() ->
RewriteRuleHandleValidator.validate("{\"regex\":\"test\",\"replace\":\"/new\",\"percentage\":30}"));
+ // an incomplete expression means the plugin skips the rewrite
+ assertDoesNotThrow(() ->
RewriteRuleHandleValidator.validate("{\"regex\":\"test\"}"));
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {
+ "not-json",
+ "[]",
+ "\"test\"",
+ "null",
+ "{\"regex\":\"test\",\"replace\":\"/new/$1\"}",
+ "{\"regex\":\"test\",\"replace\":\"/v1/$$\"}",
+ "{\"regex\":\"test\",\"replace\":\"/new/$\"}",
+ "{\"regex\":\"test\",\"replace\":\"/new/${name\"}",
+ "{\"regex\":\"(test\",\"replace\":\"/new\"}",
+ "{\"regex\":\"[a-\",\"replace\":\"/new\"}",
+ "{\"regex\":\"test\\\\\",\"replace\":\"/new\"}"
+ })
+ public void testUnapplicableHandles(final String handle) {
+ assertThrows(ShenyuAdminException.class, () ->
RewriteRuleHandleValidator.validate(handle));
+ }
+
+ @Test
+ public void testMessageNamesTheProblem() {
+ final ShenyuAdminException error =
assertThrows(ShenyuAdminException.class,
+ () ->
RewriteRuleHandleValidator.validate("{\"regex\":\"test\",\"replace\":\"/new/$1\"}"));
+ assertTrue(error.getMessage().contains("invalid replacement
template"));
+ }
+}
diff --git
a/shenyu-common/src/main/java/org/apache/shenyu/common/utils/RewriteHandleUtils.java
b/shenyu-common/src/main/java/org/apache/shenyu/common/utils/RewriteHandleUtils.java
new file mode 100644
index 0000000000..b795c4b9e5
--- /dev/null
+++
b/shenyu-common/src/main/java/org/apache/shenyu/common/utils/RewriteHandleUtils.java
@@ -0,0 +1,73 @@
+/*
+ * 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.shenyu.common.utils;
+
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
+
+/**
+ * The utils that tell whether a rewrite expression can be applied.
+ */
+public final class RewriteHandleUtils {
+
+ private static final String INVALID_REGEX = "invalid regular expression: ";
+
+ private static final String INVALID_REPLACEMENT = "invalid replacement
template: ";
+
+ private RewriteHandleUtils() {
+ }
+
+ /**
+ * Explain why a rewrite expression cannot be applied.
+ *
+ * <p>The plugin expands the replacement with {@link
String#replaceAll(String, String)}, so the
+ * replacement is a template in which a dollar sign references a regex
group. The configured
+ * expression is validated on its own first: appending the empty
alternative that makes the
+ * dry-run match would turn a trailing backslash into an escaped pipe and
hide an invalid
+ * expression. The dry-run that follows only validates the replacement
template. An incomplete
+ * expression and the placeholder mode, where both values contain a left
brace, are always
+ * applicable because the plugin skips them or substitutes them
literally.</p>
+ *
+ * @param regex the configured regular expression
+ * @param replace the configured replacement template
+ * @return {@code null} when the plugin can apply the expression,
otherwise the reason why it cannot
+ */
+ public static String applicabilityProblem(final String regex, final String
replace) {
+ if (isSkipped(regex, replace)) {
+ return null;
+ }
+ try {
+ Pattern.compile(regex);
+ } catch (PatternSyntaxException ex) {
+ return INVALID_REGEX + ex.getDescription();
+ }
+ try {
+ // the trailing empty alternative makes the empty input match, so
the replacement is parsed too
+ Pattern.compile(regex + "|").matcher("").replaceAll(replace);
+ return null;
+ } catch (IllegalArgumentException | IndexOutOfBoundsException ex) {
+ return INVALID_REPLACEMENT + ex.getMessage();
+ }
+ }
+
+ private static boolean isSkipped(final String regex, final String replace)
{
+ return StringUtils.isAnyBlank(regex, replace) || replace.contains("{")
&& regex.contains("{");
+ }
+}
diff --git
a/shenyu-common/src/test/java/org/apache/shenyu/common/utils/RewriteHandleUtilsTest.java
b/shenyu-common/src/test/java/org/apache/shenyu/common/utils/RewriteHandleUtilsTest.java
new file mode 100644
index 0000000000..fae5e269ed
--- /dev/null
+++
b/shenyu-common/src/test/java/org/apache/shenyu/common/utils/RewriteHandleUtilsTest.java
@@ -0,0 +1,67 @@
+/*
+ * 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.shenyu.common.utils;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Test case for {@link RewriteHandleUtils}.
+ */
+public final class RewriteHandleUtilsTest {
+
+ @Test
+ public void testApplicableExpressions() {
+ assertNull(RewriteHandleUtils.applicabilityProblem(null, null));
+ assertNull(RewriteHandleUtils.applicabilityProblem("", "/new"));
+ assertNull(RewriteHandleUtils.applicabilityProblem("test", null));
+ assertNull(RewriteHandleUtils.applicabilityProblem("(test)",
"/new/$1"));
+ assertNull(RewriteHandleUtils.applicabilityProblem("/shenyu/test",
"/new"));
+ assertNull(RewriteHandleUtils.applicabilityProblem("/shenyu/{id}",
"/target/{id}"));
+ }
+
+ @Test
+ public void testMissingGroupReferenceIsReported() {
+ final String problem = RewriteHandleUtils.applicabilityProblem("test",
"/new/$1");
+ assertNotNull(problem);
+ assertTrue(problem.startsWith("invalid replacement template"));
+ }
+
+ @Test
+ public void testBareDollarIsReported() {
+ assertNotNull(RewriteHandleUtils.applicabilityProblem("test",
"/v1/$$"));
+ }
+
+ @Test
+ public void testUnclosedGroupIsReported() {
+ final String problem =
RewriteHandleUtils.applicabilityProblem("(test", "/new");
+ assertNotNull(problem);
+ assertTrue(problem.startsWith("invalid regular expression"));
+ }
+
+ @Test
+ public void testTrailingBackslashExpressionIsReported() {
+ // appending the empty alternative would turn the trailing backslash
into an escaped pipe
+ final String problem =
RewriteHandleUtils.applicabilityProblem("test\\", "/new");
+ assertNotNull(problem);
+ assertTrue(problem.startsWith("invalid regular expression"));
+ }
+}
diff --git
a/shenyu-plugin/shenyu-plugin-rewrite/src/main/java/org/apache/shenyu/plugin/rewrite/handler/RewritePluginDataHandler.java
b/shenyu-plugin/shenyu-plugin-rewrite/src/main/java/org/apache/shenyu/plugin/rewrite/handler/RewritePluginDataHandler.java
index 3d203b42bd..8d6d30fc49 100644
---
a/shenyu-plugin/shenyu-plugin-rewrite/src/main/java/org/apache/shenyu/plugin/rewrite/handler/RewritePluginDataHandler.java
+++
b/shenyu-plugin/shenyu-plugin-rewrite/src/main/java/org/apache/shenyu/plugin/rewrite/handler/RewritePluginDataHandler.java
@@ -22,6 +22,7 @@ import org.apache.shenyu.common.dto.RuleData;
import org.apache.shenyu.common.dto.convert.rule.RewriteHandle;
import org.apache.shenyu.common.enums.PluginEnum;
import org.apache.shenyu.common.utils.GsonUtils;
+import org.apache.shenyu.common.utils.RewriteHandleUtils;
import org.apache.shenyu.plugin.base.cache.CommonHandleCache;
import org.apache.shenyu.plugin.base.handler.PluginDataHandler;
import org.apache.shenyu.plugin.base.utils.BeanHolder;
@@ -32,7 +33,6 @@ import org.slf4j.LoggerFactory;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Supplier;
-import java.util.regex.Pattern;
/**
* The type redirect plugin data subscriber.
@@ -78,23 +78,12 @@ public class RewritePluginDataHandler implements
PluginDataHandler {
* @return true when the plugin can apply the handle
*/
private static boolean canRewrite(final RewriteHandle rewriteHandle, final
RuleData ruleData) {
- final String regex = rewriteHandle.getRegex();
- final String replace = rewriteHandle.getReplace();
- if (StringUtils.isAnyBlank(regex, replace)) {
- // an incomplete expression means the plugin does not rewrite,
which is always safe.
+ final String problem =
RewriteHandleUtils.applicabilityProblem(rewriteHandle.getRegex(),
rewriteHandle.getReplace());
+ if (Objects.isNull(problem)) {
return true;
}
- if (replace.contains("{") && regex.contains("{")) {
- // in placeholder mode both values are substituted literally, the
regex is never compiled.
- return true;
- }
- try {
- // the empty alternative makes the empty input match, which also
validates the replacement.
- Pattern.compile(regex + "|").matcher("").replaceAll(replace);
- return true;
- } catch (IllegalArgumentException | IndexOutOfBoundsException ex) {
- LOG.error("invalid rewrite rule [{}] is ignored, regex: {},
replace: {}", ruleData.getId(), regex, replace, ex);
- return false;
- }
+ LOG.error("invalid rewrite rule [{}] is ignored, regex: {}, replace:
{}, reason: {}",
+ ruleData.getId(), rewriteHandle.getRegex(),
rewriteHandle.getReplace(), problem);
+ return false;
}
}
