Skip to content

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

Merged
Aias00 merged 12 commits into
apache:masterfrom
hengyuss:fix/fix_uri_validate
Jun 11, 2026
Merged

[ISSUE 6334] : Add uri validation for regex and other operators.#6335
Aias00 merged 12 commits into
apache:masterfrom
hengyuss:fix/fix_uri_validate

Conversation

@hengyuss

@hengyuss hengyuss commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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
Contributor Author

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

@Aias00

Aias00 commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

@hengyuss hey bro, you have to fix the problem

@hengyuss

Copy link
Copy Markdown
Contributor Author

ok, i will fix it soon

@hengyuss

Copy link
Copy Markdown
Contributor 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

Aias00 commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

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
Contributor 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

damingqicai commented Apr 28, 2026 via email

Copy link
Copy Markdown

@Aias00

Aias00 commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

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
Contributor 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

Aias00 commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

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

@hengyuss

hengyuss commented May 6, 2026

Copy link
Copy Markdown
Contributor 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: 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

I have added the re2j dependency to resolve the problem. Please review, thanks!

@hengyuss

hengyuss commented May 7, 2026

Copy link
Copy Markdown
Contributor Author

@Aias00 I have fixed the failing tests and updated the PR. Could you please approve and rerun the workflow for me? Thanks!

@hengyuss

hengyuss commented May 7, 2026

Copy link
Copy Markdown
Contributor Author
图片 @Aias00 is my fault? it looks like a ci machine error other pr has the seem error "could not acquire locks" this is #6336 ci error 图片

@Aias00

Aias00 commented May 20, 2026

Copy link
Copy Markdown
Contributor

pls add my wechat: aias00

@Aias00 Aias00 merged commit 1ecc77c into apache:master Jun 11, 2026
47 of 53 checks passed
@Aias00 Aias00 added this to the 2.7.1 milestone Jun 11, 2026
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

6 participants