Skip to content

[ISSUE 6334] : Add uri validation for regex and other operators.#6335

Open
hengyuss wants to merge 4 commits intoapache:masterfrom
hengyuss:fix/fix_uri_validate
Open

[ISSUE 6334] : Add uri validation for regex and other operators.#6335
hengyuss wants to merge 4 commits intoapache:masterfrom
hengyuss:fix/fix_uri_validate

Conversation

@hengyuss
Copy link
Copy Markdown

@hengyuss hengyuss commented Apr 24, 2026

Fixes #6334
Currently, when creating or updating a Rule in ShenYu Admin, the URI condition validation only checks the PATH_PATTERN operator.
If a user selects other operators like regex, =, startsWith, or endsWith and inputs an invalid format (e.g., an unclosed regex like [a-z+), the Admin console skips the validation and successfully saves the dirty data to the database.

When this dirty data is synced to the Gateway, it causes severe runtime exceptions (e.g., PatternSyntaxException) during route matching, potentially crashing the routing process.

I create a new class UriConditionValidator to modify or add condition

Make sure that:

  • You have read the contribution guidelines.
  • You submit test cases (unit or integration tests) that back your changes.
  • Your local test passed ./mvnw clean install -Dmaven.javadoc.skip=true.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses issue #6334 by extending ShenYu Admin’s URI condition validation during rule creation/update so that invalid URI patterns (not just pathPattern) are rejected before being persisted and synced to the Gateway, preventing runtime failures (e.g., PatternSyntaxException) during route matching.

Changes:

  • Introduces a new UriConditionValidator utility to validate URI condition values per operator (e.g., pathPattern, regex, =, startsWith, endsWith).
  • Updates RuleService#createOrUpdate to validate all ParamType=uri conditions via the new validator.
  • Minor javadoc alignment adjustments in RuleService.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
shenyu-admin/src/main/java/org/apache/shenyu/admin/validation/validator/UriConditionValidator.java Adds centralized operator-based validation logic for URI rule conditions.
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/RuleService.java Applies the new URI condition validation during rule create/update.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@hengyuss
Copy link
Copy Markdown
Author

Hi @Aias00 , I have updated the branch and resolved the previous comments. Please take another look when you have time. PTAL, thanks!

static {
VALIDATOR_MAP.put(OperatorEnum.PATH_PATTERN.getAlias(),
PathPatternParser.defaultInstance::parse);
VALIDATOR_MAP.put(OperatorEnum.REGEX.getAlias(), Pattern::compile);
@Aias00
Copy link
Copy Markdown
Contributor

Aias00 commented Apr 27, 2026

@hengyuss hey bro, you have to fix the problem

@hengyuss
Copy link
Copy Markdown
Author

ok, i will fix it soon

@hengyuss
Copy link
Copy Markdown
Author

Hi, @Aias00
Regarding the ReDoS CodeQL alert: The scanner still flags Pattern.compile even with a length limit. We have two ways to handle this:

Add com.google.re2j:re2j: A DFA regex engine that guarantees linear time and passes CodeQL. (License is compatible).

Dismiss the alert: If we consider the regex input from ShenYu Admin strictly trusted, we can mark this alert as a false positive.

Which approach does the community prefer? I'm happy to update the PR accordingly.

@Aias00
Copy link
Copy Markdown
Contributor

Aias00 commented Apr 28, 2026

Hi, @Aias00 Regarding the ReDoS CodeQL alert: The scanner still flags Pattern.compile even with a length limit. We have two ways to handle this:

Add com.google.re2j:re2j: A DFA regex engine that guarantees linear time and passes CodeQL. (License is compatible).

Dismiss the alert: If we consider the regex input from ShenYu Admin strictly trusted, we can mark this alert as a false positive.

Which approach does the community prefer? I'm happy to update the PR accordingly.

how about refering this: https://github.com/apache/shenyu/security/code-scanning/77

@hengyuss
Copy link
Copy Markdown
Author

Hi, @Aias00 Regarding the ReDoS CodeQL alert: The scanner still flags Pattern.compile even with a length limit. We have two ways to handle this:

Add com.google.re2j:re2j: A DFA regex engine that guarantees linear time and passes CodeQL. (License is compatible).

Dismiss the alert: If we consider the regex input from ShenYu Admin strictly trusted, we can mark this alert as a false positive.

Which approach does the community prefer? I'm happy to update the PR accordingly.

how about refering this: https://github.com/apache/shenyu/security/code-scanning/77

I'm getting a 404 Not Found on that link

@damingqicai
Copy link
Copy Markdown

damingqicai commented Apr 28, 2026 via email

@Aias00
Copy link
Copy Markdown
Contributor

Aias00 commented Apr 28, 2026

Hi, @Aias00 Regarding the ReDoS CodeQL alert: The scanner still flags Pattern.compile even with a length limit. We have two ways to handle this:

Add com.google.re2j:re2j: A DFA regex engine that guarantees linear time and passes CodeQL. (License is compatible).

Dismiss the alert: If we consider the regex input from ShenYu Admin strictly trusted, we can mark this alert as a false positive.

Which approach does the community prefer? I'm happy to update the PR accordingly.

how about refering this: Regular expression injection

I'm getting a 404 Not Found on that link

add my wechat: aias00

@hengyuss
Copy link
Copy Markdown
Author

Hi, @Aias00 Regarding the ReDoS CodeQL alert: The scanner still flags Pattern.compile even with a length limit. We have two ways to handle this:

Add com.google.re2j:re2j: A DFA regex engine that guarantees linear time and passes CodeQL. (License is compatible).

Dismiss the alert: If we consider the regex input from ShenYu Admin strictly trusted, we can mark this alert as a false positive.

Which approach does the community prefer? I'm happy to update the PR accordingly.

how about refering this: https://github.com/apache/shenyu/security/code-scanning/77

Hi, @Aias00 Regarding the ReDoS CodeQL alert: The scanner still flags Pattern.compile even with a length limit. We have two ways to handle this:

Add com.google.re2j:re2j: A DFA regex engine that guarantees linear time and passes CodeQL. (License is compatible).

Dismiss the alert: If we consider the regex input from ShenYu Admin strictly trusted, we can mark this alert as a false positive.

Which approach does the community prefer? I'm happy to update the PR accordingly.

how about refering this: https://github.com/apache/shenyu/security/code-scanning/77

I looked into the standard CodeQL recommendation as you suggested. CodeQL advises using Pattern.quote() to sanitize the input.

However, if we apply Pattern.quote() in UriConditionValidator, it will escape all regex meta-characters into literal strings. This means our gateway would lose the ability to perform actual regex-based routing.

Since we must evaluate real regex patterns, relying on java.util.regex will always trigger the ReDoS alert due to its underlying NFA backtracking mechanism.

Therefore, may be we should introduce com.google.re2j:re2j to resolve it

@Aias00
Copy link
Copy Markdown
Contributor

Aias00 commented Apr 30, 2026

Hi, @Aias00 Regarding the ReDoS CodeQL alert: The scanner still flags Pattern.compile even with a length limit. We have two ways to handle this:

Add com.google.re2j:re2j: A DFA regex engine that guarantees linear time and passes CodeQL. (License is compatible).

Dismiss the alert: If we consider the regex input from ShenYu Admin strictly trusted, we can mark this alert as a false positive.

Which approach does the community prefer? I'm happy to update the PR accordingly.

how about refering this: Regular expression injection

Hi, @Aias00 Regarding the ReDoS CodeQL alert: The scanner still flags Pattern.compile even with a length limit. We have two ways to handle this:

Add com.google.re2j:re2j: A DFA regex engine that guarantees linear time and passes CodeQL. (License is compatible).

Dismiss the alert: If we consider the regex input from ShenYu Admin strictly trusted, we can mark this alert as a false positive.

Which approach does the community prefer? I'm happy to update the PR accordingly.

how about refering this: Regular expression injection

I looked into the standard CodeQL recommendation as you suggested. CodeQL advises using Pattern.quote() to sanitize the input.

However, if we apply Pattern.quote() in UriConditionValidator, it will escape all regex meta-characters into literal strings. This means our gateway would lose the ability to perform actual regex-based routing.

Since we must evaluate real regex patterns, relying on java.util.regex will always trigger the ReDoS alert due to its underlying NFA backtracking mechanism.

Therefore, may be we should introduce com.google.re2j:re2j to resolve it

fair enough

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Resolve TODO: Add URI validation for other operators in Rule configuration

5 participants