Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AntPathPatternMatcher hardcoded "/" separator in variables matching and comparator #33085

Conversation

tafjwr
Copy link
Contributor

@tafjwr tafjwr commented Jun 22, 2024

Related: #26264

When AntPathMatcher is constructed with a path separator other than default character "/", the functionality of extraction of URI template variables doesn’t work properly. If the matcher is specified with a non-default character, and a part of URI variable template includes default separator as a variable name, the variable name key mapping leads to exceptions or unexpected results. This is because the regex for URI template variables is hard-coded with "/", which is not considered as a part of variable key, although it can be placed if the matcher has a non-default path separator. To fix this, I updated it to pass a path separator to that regex.

Currently, for example, the matcher with non-default path separator seems to be leveraged on WebSocketConfiguration, in default, to map a "."-separated resource, which is a part of URI segment. Thus, for instance, if the path separator is specified as ".", extracting “price.stock.aaa“ with the template "price.stock.{ticker/symbol}" leads to IllegalStateException.

NOTE: A hard-coded variable pattern is still remaining. It seemed to be indirectly used in PathMatcher#compare implementation, but I’m not sure if this erroneous code causes some bug behaviors or not and is tracked by the team. And for isolation reasons, I would like to exclude it from the scope of code fix in this PR.

private static final Pattern VARIABLE_PATTERN = Pattern.compile("\\{[^/]+?\\}");

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 22, 2024
@bclozel bclozel added type: bug A general bug in: core Issues in core modules (aop, beans, core, context, expression) and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jun 22, 2024
@bclozel bclozel self-assigned this Jun 22, 2024
@bclozel bclozel added this to the 6.2.x milestone Jun 22, 2024
@bclozel bclozel changed the title Fix AntPathMatcher URI template variable extractor AntPathPatternMatcher hardcoded "/" separator in variables matching and comparator Jun 24, 2024
@bclozel bclozel modified the milestones: 6.2.x, 6.2.0-M5 Jun 24, 2024
bclozel pushed a commit that referenced this pull request Jun 24, 2024
@bclozel bclozel closed this in 6d1f117 Jun 24, 2024
bclozel added a commit that referenced this pull request Jun 24, 2024
As pointed out in gh-33085, the `AntPatternComparator` hardcodes the "/"
separator when checking for "catch all" patterns like "/**".
This commit ensures that the custom path separator is used for those
checks, in order to guarantee consistent comparator results.

See gh-33085
@bclozel
Copy link
Member

bclozel commented Jun 24, 2024

Thanks for your contribution @tafjwr , this is now in main and should be available in SNAPSHOTs soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants