From cdd4e8cd7f1fb5bfd1927d6023ef6d51b0539737 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Wed, 13 Jul 2022 18:43:51 +0100 Subject: [PATCH] Improve regex support for URL path matching Closes gh-28815 --- .../springframework/util/AntPathMatcher.java | 6 +++--- .../util/AntPathMatcherTests.java | 3 ++- .../pattern/CaptureVariablePathElement.java | 14 ++++---------- .../web/util/pattern/RegexPathElement.java | 10 +++------- .../web/util/pattern/PathPatternTests.java | 3 +++ src/docs/asciidoc/web/webmvc.adoc | 19 ++++++++++++------- 6 files changed, 27 insertions(+), 28 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/util/AntPathMatcher.java b/spring-core/src/main/java/org/springframework/util/AntPathMatcher.java index 93f5af717ef..396b0afe865 100644 --- a/spring-core/src/main/java/org/springframework/util/AntPathMatcher.java +++ b/spring-core/src/main/java/org/springframework/util/AntPathMatcher.java @@ -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. @@ -701,8 +701,8 @@ public class AntPathMatcher implements PathMatcher { 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)); } } diff --git a/spring-core/src/test/java/org/springframework/util/AntPathMatcherTests.java b/spring-core/src/test/java/org/springframework/util/AntPathMatcherTests.java index 8bebf5920ff..ec036e547ce 100644 --- a/spring-core/src/test/java/org/springframework/util/AntPathMatcherTests.java +++ b/spring-core/src/test/java/org/springframework/util/AntPathMatcherTests.java @@ -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. @@ -131,6 +131,7 @@ class AntPathMatcherTests { assertThat(pathMatcher.match("/{bla}.*", "/testing.html")).isTrue(); assertThat(pathMatcher.match("/{bla}", "//x\ny")).isTrue(); + assertThat(pathMatcher.match("/{var:.*}", "/x\ny")).isTrue(); } @Test diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/CaptureVariablePathElement.java b/spring-web/src/main/java/org/springframework/web/util/pattern/CaptureVariablePathElement.java index 856c35af986..b3002295a9f 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/CaptureVariablePathElement.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/CaptureVariablePathElement.java @@ -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. @@ -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)); } } diff --git a/spring-web/src/main/java/org/springframework/web/util/pattern/RegexPathElement.java b/spring-web/src/main/java/org/springframework/web/util/pattern/RegexPathElement.java index c5cd8f588ff..d66e7a7ce2d 100644 --- a/spring-web/src/main/java/org/springframework/web/util/pattern/RegexPathElement.java +++ b/spring-web/src/main/java/org/springframework/web/util/pattern/RegexPathElement.java @@ -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. @@ -108,12 +108,8 @@ class RegexPathElement extends PathElement { } 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 getVariableNames() { diff --git a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java index 0587fc4a8ba..97aad62e622 100644 --- a/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java +++ b/spring-web/src/test/java/org/springframework/web/util/pattern/PathPatternTests.java @@ -300,6 +300,7 @@ public class PathPatternTests { 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 @@ -319,6 +320,8 @@ public class PathPatternTests { 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 diff --git a/src/docs/asciidoc/web/webmvc.adoc b/src/docs/asciidoc/web/webmvc.adoc index 17e90887807..e54d79f8ced 100644 --- a/src/docs/asciidoc/web/webmvc.adoc +++ b/src/docs/asciidoc/web/webmvc.adoc @@ -648,7 +648,7 @@ See <> 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 @@ -657,6 +657,7 @@ declare it as an <> bean or configure it directly on + [[mvc-exceptionhandlers]] === Exceptions [.small]#<># @@ -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/*"); } } ---- @@ -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/*") } } ---- @@ -5392,13 +5391,19 @@ The following example shows how to achieve the same configuration in XML: - - - - ---- +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]]