Skip to content

Commit

Permalink
Improve regex support for URL path matching
Browse files Browse the repository at this point in the history
Closes gh-28815
  • Loading branch information
rstoyanchev committed Jul 13, 2022
1 parent 02b7ddb commit cdd4e8c
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 28 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2021 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -701,8 +701,8 @@ else if (match.startsWith("{") && match.endsWith("}")) {
else {
this.exactMatch = false;
patternBuilder.append(quote(pattern, end, pattern.length()));
this.pattern = (this.caseSensitive ? Pattern.compile(patternBuilder.toString()) :
Pattern.compile(patternBuilder.toString(), Pattern.CASE_INSENSITIVE));
this.pattern = Pattern.compile(patternBuilder.toString(),
Pattern.DOTALL | (this.caseSensitive ? 0 : Pattern.CASE_INSENSITIVE));
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2020 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -131,6 +131,7 @@ void match() {

assertThat(pathMatcher.match("/{bla}.*", "/testing.html")).isTrue();
assertThat(pathMatcher.match("/{bla}", "//x\ny")).isTrue();
assertThat(pathMatcher.match("/{var:.*}", "/x\ny")).isTrue();
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2021 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -59,15 +59,9 @@ class CaptureVariablePathElement extends PathElement {
}
else {
this.variableName = new String(captureDescriptor, 1, colon - 1);
if (caseSensitive) {
this.constraintPattern = Pattern.compile(
new String(captureDescriptor, colon + 1, captureDescriptor.length - colon - 2));
}
else {
this.constraintPattern = Pattern.compile(
new String(captureDescriptor, colon + 1, captureDescriptor.length - colon - 2),
Pattern.CASE_INSENSITIVE);
}
this.constraintPattern = Pattern.compile(
new String(captureDescriptor, colon + 1, captureDescriptor.length - colon - 2),
Pattern.DOTALL | (caseSensitive ? 0 : Pattern.CASE_INSENSITIVE));
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2021 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -108,12 +108,8 @@ else if (match.startsWith("{") && match.endsWith("}")) {
}

patternBuilder.append(quote(text, end, text.length()));
if (this.caseSensitive) {
return Pattern.compile(patternBuilder.toString());
}
else {
return Pattern.compile(patternBuilder.toString(), Pattern.CASE_INSENSITIVE);
}
return Pattern.compile(patternBuilder.toString(),
Pattern.DOTALL | (this.caseSensitive ? 0 : Pattern.CASE_INSENSITIVE));
}

public List<String> getVariableNames() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ public void encodingAndBoundVariablesCapturePathElement() {
checkCapture("{var:f o}","f%20o","var","f o"); // constraint is expressed in non encoded form
checkCapture("{var:f.o}","f%20o","var","f o");
checkCapture("{var:f\\|o}","f%7co","var","f|o");
checkCapture("{var:.*}","x\ny","var","x\ny");
}

@Test
Expand All @@ -319,6 +320,8 @@ public void encodingAndBoundVariablesRegexPathElement() {
checkCapture("/{var1}_ _{var2}","/f%20o_%20_f%7co","var1","f o","var2","f|o");
checkCapture("/{var1}_ _{var2:f\\|o}","/f%20o_%20_f%7co","var1","f o","var2","f|o");
checkCapture("/{var1:f o}_ _{var2:f\\|o}","/f%20o_%20_f%7co","var1","f o","var2","f|o");
checkCapture("/{var1:f o}_ _{var2:f\\|o}","/f%20o_%20_f%7co","var1","f o","var2","f|o");
checkCapture("/{var1}_{var2}","/f\noo_foo","var1","f\noo","var2","foo");
}

@Test
Expand Down
19 changes: 12 additions & 7 deletions src/docs/asciidoc/web/webmvc.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ See <<mvc-config-interceptors>> in the section on MVC configuration for examples
configure interceptors. You can also register them directly by using setters on individual
`HandlerMapping` implementations.

Note that `postHandle` is less useful with `@ResponseBody` and `ResponseEntity` methods for
`postHandle` method is less useful with `@ResponseBody` and `ResponseEntity` methods for
which the response is written and committed within the `HandlerAdapter` and before
`postHandle`. That means it is too late to make any changes to the response, such as adding
an extra header. For such scenarios, you can implement `ResponseBodyAdvice` and either
Expand All @@ -657,6 +657,7 @@ declare it as an <<mvc-ann-controller-advice>> bean or configure it directly on




[[mvc-exceptionhandlers]]
=== Exceptions
[.small]#<<web-reactive.adoc#webflux-dispatcher-exceptions, WebFlux>>#
Expand Down Expand Up @@ -5362,7 +5363,6 @@ the following example shows:
public void addInterceptors(InterceptorRegistry registry) {
registry.addInterceptor(new LocaleChangeInterceptor());
registry.addInterceptor(new ThemeChangeInterceptor()).addPathPatterns("/**").excludePathPatterns("/admin/**");
registry.addInterceptor(new SecurityInterceptor()).addPathPatterns("/secure/*");
}
}
----
Expand All @@ -5376,7 +5376,6 @@ the following example shows:
override fun addInterceptors(registry: InterceptorRegistry) {
registry.addInterceptor(LocaleChangeInterceptor())
registry.addInterceptor(ThemeChangeInterceptor()).addPathPatterns("/**").excludePathPatterns("/admin/**")
registry.addInterceptor(SecurityInterceptor()).addPathPatterns("/secure/*")
}
}
----
Expand All @@ -5392,13 +5391,19 @@ The following example shows how to achieve the same configuration in XML:
<mvc:exclude-mapping path="/admin/**"/>
<bean class="org.springframework.web.servlet.theme.ThemeChangeInterceptor"/>
</mvc:interceptor>
<mvc:interceptor>
<mvc:mapping path="/secure/*"/>
<bean class="org.example.SecurityInterceptor"/>
</mvc:interceptor>
</mvc:interceptors>
----

NOTE: Mapped interceptors are not ideally suited as a security layer due to the potential
for a mismatch with annotated controller path matching, which can also match trailing
slashes and path extensions transparently, along with other path matching options. Many
of these options have been deprecated but the potential for a mismatch remains.
Generally, we recommend using Spring Security which includes a dedicated
https://docs.spring.io/spring-security/reference/servlet/integrations/mvc.html#mvc-requestmatcher[MvcRequestMatcher]
to align with Spring MVC path matching and also has a security firewall that blocks many
unwanted characters in URL paths.




[[mvc-config-content-negotiation]]
Expand Down

0 comments on commit cdd4e8c

Please sign in to comment.