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

Revisit AntPathMatcher's handling of patterns with file extensions [SPR-7336] #11995

Closed
spring-projects-issues opened this issue Jun 28, 2010 · 7 comments
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

marc schipperheyn opened SPR-7336 and commented

The AntPathMatchers in some cases prefers less specific matches over specific matches. This examples is from the case of
WebContentInterceptor

 
<property name="cacheMappings">
	<props>
		<prop key="/**/">1800</prop>
		<prop key="/**/*.html">1800</prop>
		<prop key="/**/offerform/**/*.html">-1</prop>
		<prop key="/**/offerform/*.html">-1</prop>
		<prop key="/**/cart/">-1</prop>
		<prop key="**/cart/**/*.html">-1</prop>
	</props>
</property>

The order doesn't matter. The issue occurs also if we put /**/ at the bottom

Example urlPath

/en/offerform/step4.html?offerId=17
matches /**/offerform/**/*.html
matches /**/offerform/*.html
matches /**/ [shouldn't match at all AFAIC]
matches /**/*.html

result: 1800. Should be -1

Example urlPath

/en/cart/
matches /**/
matches /**/cart/

result: -1 correct

It seems that the issue doesn't occur when the path ends with a / in stead of with an extension *.html


Affects: 3.0.3

2 votes, 4 watchers

@spring-projects-issues
Copy link
Collaborator Author

Luke Taylor commented

Did you mean to register this against Spring?

@spring-projects-issues
Copy link
Collaborator Author

marc schipperheyn commented

yeah, sorry.

@spring-projects-issues
Copy link
Collaborator Author

John Latham commented

Hmmm, this has gone from Bug to Improvement. Is behaviour of AntPathMatcher supposed to be consistent between 2.5 and 3.0? It isn't consistent in my experience. Is that known? Please advise whether I should log details of differences on this issue or create another.

@spring-projects-issues
Copy link
Collaborator Author

marc schipperheyn commented

Yeah, AFAIC it's a bug

@spring-projects-issues
Copy link
Collaborator Author

marc schipperheyn commented

As a solution direction you could offer an overridable isBetterMatch(String currentPath,String otherMatchingPath) method that retains current functionality and that you potentially implement later in a different manner

protected Integer lookupCacheSeconds(String urlPath) {
	// direct match?
	Integer cacheSeconds = this.cacheMappings.get(urlPath);
	if (cacheSeconds == null) {
		// pattern match?
		//This determines the best pattern bij simply looking if the matching patterns is the longest
		String match = "";
		for (String registeredPath : this.cacheMappings.keySet()) {
			if (this.pathMatcher.match(registeredPath, urlPath)) {
				match = isBetterMatch(match,urlPath);
			}
		}
		cacheSeconds = this.cacheMappings.get(match);
	}
	return cacheSeconds;
}

public String isBetterMatch(String currentPath,String otherMatchingPath){
	return true;
}

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Although I'm not sure what we should do yet, it clearly looks like something needs to be done. The cacheMappings in WebContentInterceptor are not sorted with AntPathMatcher PatternComparator. The order in which patterns are listed is not preserved either.

Furthermore we keep going and return the last match where in fact any match is as random as the next. I'm actually not sure why the original description states the result of specific URLs where the result can be different each time as far as I can see. Am I missing anything?

If the result is truly random as it seems to be, then we can take our picks for the fix without backwards compatibility issues. The use of AntPathMatcher/Comparator with some protected method to override might make the most sense.

@spring-projects-issues spring-projects-issues added type: enhancement A general enhancement in: core Issues in core modules (aop, beans, core, context, expression) labels Jan 11, 2019
@rstoyanchev
Copy link
Contributor

Closing as outdated. We've deprecated and turned off support for suffix pattern match in #23915 and related issues. We've added support for pre-parsed PathPattern, created specifically for Web paths, and see that as the way forward. The PathPattern syntax also does not allow "**" in the middle of a path.

@rstoyanchev rstoyanchev removed this from the General Backlog milestone Dec 9, 2021
@rstoyanchev rstoyanchev added the status: declined A suggestion or change that we don't feel we should currently apply label Dec 9, 2021
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) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants