Repository navigation
fix: preserve resource names in general rule commands - #3666
Open
hutiefang76 wants to merge 1 commit into
Open
hutiefang76 wants to merge 1 commit into
hutiefang76 wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe what this PR does / why we need it
A once-encoded
setRulesrequest can silently change a resource such asapi+v1intoapi v1, turn a literal%2Binto+, or reject a resource containing a lone%. The changed value also reaches writable data sources.Does this pull request fix one issue?
Fixes #3665.
Describe how you did it
Remove the second URL decode from
ModifyRulesCommandHandler; command transports already provide decoded parameters. Keep URL decoding at the transport boundary.Add regressions for flow and authority rules through the handler, simple HTTP socket requests and the Netty HTTP pipeline. They check loaded rules and writable-data-source values for
+,%2Band%, with ASCII, space and Unicode controls. Each test restores rule managers, data sources and the command registration it replaces.Describe how to verify it
On the unmodified
1.8base (a3f40ba8), 19 of the 33 regression cases fail. With the fix, all 33 pass on JDK 17.mvn -B -pl sentinel-transport/sentinel-transport-common,sentinel-transport/sentinel-transport-simple-http,sentinel-transport/sentinel-transport-netty-http -am -Dsurefire.jdk-toolchain-version=17 testThe affected-module reactor and its dependencies passed 285 tests on macOS arm64 / Microsoft JDK 17.0.18, with no failures, errors or skips.
verify -DskipTestsalso passed, including PMD checks and Java 8 compilation compatibility.After the final test-cleanup changes, the 33 regressions passed again on JDK 17 and on Zulu JDK 8 (1.8.0_492), with no failures, errors or skips. The final
verify -DskipTestspassed again; it does not count as another test run.Special notes for reviews
The general handler is separate from the parameter-flow handler in #3628. These tests exercise both supported HTTP transports; a running Spring MVC application was not part of this validation. Implementation and review were AI-assisted. Remote CI has not run for this new PR yet.