Skip to content

fix(sdk): quote path-variable replacement values in the Spring SDK (#7712) - #7729

Merged
Aias00 merged 1 commit into
apache:masterfrom
konlue:fix/sdk-pathvar-regex-replacement
Oct 19, 2026
Merged

Aias00 merged 1 commit into
apache:masterfrom
konlue:fix/sdk-pathvar-regex-replacement

Conversation

@konlue

@konlue konlue commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Fixes #7712.

Summary

PathVariableParameterProcessor#processArgument passes the path-variable value directly as the replacement string to RegExUtils.replaceAll. Java replacement syntax interprets $n, ${name} and backslashes, so a valid identifier such as $1 fails request construction with IndexOutOfBoundsException: No group 1, while a\b is silently rewritten to ab.

Modifications

  • Quote the replacement value with Matcher.quoteReplacement(...) in both the single-string branch and the Map<String, String> branch. The pattern is unchanged, so matching behaviour stays the same; only the user-supplied value is treated literally.
  • Add two parameterized tests (three request templates each) covering the dollar-sign and backslash values on both branches.

Verifying this change

  • Red-green: with the original unquoted replacement restored, the new tests fail with IndexOutOfBoundsException: No group 1 (the symptom reported in the issue); with the fix, all cases pass.

  • Targeted tests pass:

    ./mvnw -B -ntp -pl shenyu-sdk/shenyu-sdk-spring test -Dtest=PathVariableParameterProcessorTest

    16/16 green.

  • Checkstyle passes:

    ./mvnw -B -ntp -pl shenyu-sdk/shenyu-sdk-spring checkstyle:check

    clean.

Notes

Percent-encoding of path segments is a separate concern and is intentionally not changed here; this PR only stops regex replacement syntax from mangling values.

@Aias00 Aias00 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.

LGTM. Matcher.quoteReplacement is exactly the right primitive here, and both branches are covered.

Evidence

  • RegExUtils.replaceAll(url, pattern, replacement) delegates to Matcher.replaceAll, so the replacement is parsed as regex syntax: $1 / ${name} become group references and \ escapes the next character. Quoting only the replacement keeps the pattern untouched, which is why URL matching behaviour is provably unchanged — you pinned that in the PR body and the diff is limited to the replacement argument.
  • Both the single-string branch (:55) and the Map branch (:67) are fixed; leaving one of them unquoted would have kept the same bug for the map form of @PathVariable.
  • The tests are red-green in the way that matters: $1 throws IndexOutOfBoundsException: No group 1 on the old code, and a\b silently becomes ab, so both new assertions genuinely fail without the fix.

Non-blocking

  1. I grepped the Spring SDK for other replaceAll-with-user-value sites and found none, so this closes the class of bug rather than one instance of it. If a future processor adds one, a shared quoteReplacement helper would be a better default than remembering to quote.
  2. The tests assert url.contains("/$1") and url.contains("/a\\b"); asserting the full construction result (or that no exception is thrown for a value that used to blow up) would be slightly stronger, but this is enough to prevent a regression.

CI: pr_build green.

@Aias00
Aias00 merged commit dc46e10 into apache:master Oct 19, 2026
29 checks passed
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] Spring SDK path-variable values are interpreted as regex replacement syntax

2 participants