From cdd4e8cd7f1fb5bfd1927d6023ef6d51b0539737 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Wed, 13 Jul 2022 18:43:51 +0100 Subject: [PATCH 1/2] 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 93f5af717e..396b0afe86 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 8bebf5920f..ec036e547c 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 856c35af98..b3002295a9 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 c5cd8f588f..d66e7a7ce2 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 0587fc4a8b..97aad62e62 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 17e9088780..e54d79f8ce 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]] From e50131d454248b9eca26edb4765aaeae3f204107 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Wed, 13 Jul 2022 19:10:51 +0100 Subject: [PATCH 2/2] Add Consumer methods to configure Jackson modules Closes gh-28633 --- .../json/Jackson2ObjectMapperBuilder.java | 47 +++++++++++++++---- .../Jackson2ObjectMapperBuilderTests.java | 21 ++++++++- 2 files changed, 57 insertions(+), 11 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/converter/json/Jackson2ObjectMapperBuilder.java b/spring-web/src/main/java/org/springframework/http/converter/json/Jackson2ObjectMapperBuilder.java index bdd1e57728..655a3084de 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/json/Jackson2ObjectMapperBuilder.java +++ b/spring-web/src/main/java/org/springframework/http/converter/json/Jackson2ObjectMapperBuilder.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. @@ -521,7 +521,7 @@ public class Jackson2ObjectMapperBuilder { } /** - * Specify one or more modules to be registered with the {@link ObjectMapper}. + * Specify the modules to be registered with the {@link ObjectMapper}. *

Multiple invocations are not additive, the last one defines the modules to * register. *

Note: If this is set, no finding of modules is going to happen - not by @@ -538,15 +538,9 @@ public class Jackson2ObjectMapperBuilder { } /** - * Set a complete list of modules to be registered with the {@link ObjectMapper}. - *

Multiple invocations are not additive, the last one defines the modules to - * register. - *

Note: If this is set, no finding of modules is going to happen - not by - * Jackson, and not by Spring either (see {@link #findModulesViaServiceLoader}). - * As a consequence, specifying an empty list here will suppress any kind of - * module detection. - *

Specify either this or {@link #modulesToInstall}, not both. + * Variant of {@link #modules(Module...)} with a {@link List}. * @see #modules(Module...) + * @see #modules(Consumer) * @see com.fasterxml.jackson.databind.Module */ public Jackson2ObjectMapperBuilder modules(List modules) { @@ -556,6 +550,22 @@ public class Jackson2ObjectMapperBuilder { return this; } + /** + * Variant of {@link #modules(Module...)} with a {@link Consumer} for full + * control over the underlying list of modules. + * @since 6.0 + * @see #modules(Module...) + * @see #modules(List) + * @see com.fasterxml.jackson.databind.Module + */ + public Jackson2ObjectMapperBuilder modules(Consumer> consumer) { + this.modules = (this.modules != null ? this.modules : new ArrayList<>()); + this.findModulesViaServiceLoader = false; + this.findWellKnownModules = false; + consumer.accept(this.modules); + return this; + } + /** * Specify one or more modules to be registered with the {@link ObjectMapper}. *

Multiple invocations are not additive, the last one defines the modules @@ -566,6 +576,7 @@ public class Jackson2ObjectMapperBuilder { * allowing to eventually override their configuration. *

Specify either this or {@link #modules(Module...)}, not both. * @since 4.1.5 + * @see #modulesToInstall(Consumer) * @see #modulesToInstall(Class...) * @see com.fasterxml.jackson.databind.Module */ @@ -575,6 +586,21 @@ public class Jackson2ObjectMapperBuilder { return this; } + /** + * Variant of {@link #modulesToInstall(Module...)} with a {@link Consumer} + * for full control over the underlying list of modules. + * @since 6.0 + * @see #modulesToInstall(Module...) + * @see #modulesToInstall(Class...) + * @see com.fasterxml.jackson.databind.Module + */ + public Jackson2ObjectMapperBuilder modulesToInstall(Consumer> consumer) { + this.modules = (this.modules != null ? this.modules : new ArrayList<>()); + this.findWellKnownModules = true; + consumer.accept(this.modules); + return this; + } + /** * Specify one or more modules by class to be registered with * the {@link ObjectMapper}. @@ -586,6 +612,7 @@ public class Jackson2ObjectMapperBuilder { * allowing to eventually override their configuration. *

Specify either this or {@link #modules(Module...)}, not both. * @see #modulesToInstall(Module...) + * @see #modulesToInstall(Consumer) * @see com.fasterxml.jackson.databind.Module */ @SafeVarargs diff --git a/spring-web/src/test/java/org/springframework/http/converter/json/Jackson2ObjectMapperBuilderTests.java b/spring-web/src/test/java/org/springframework/http/converter/json/Jackson2ObjectMapperBuilderTests.java index 32a93851ba..f1a5748d27 100644 --- a/spring-web/src/test/java/org/springframework/http/converter/json/Jackson2ObjectMapperBuilderTests.java +++ b/spring-web/src/test/java/org/springframework/http/converter/json/Jackson2ObjectMapperBuilderTests.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. @@ -241,6 +241,16 @@ class Jackson2ObjectMapperBuilderTests { assertThat(serializers.findSerializer(null, SimpleType.construct(Integer.class), null)).isSameAs(serializer1); } + @Test + void modulesWithConsumer() { + NumberSerializer serializer1 = new NumberSerializer(Integer.class); + SimpleModule module = new SimpleModule(); + module.addSerializer(Integer.class, serializer1); + ObjectMapper objectMapper = Jackson2ObjectMapperBuilder.json().modules(list -> list.add(module) ).build(); + Serializers serializers = getSerializerFactoryConfig(objectMapper).serializers().iterator().next(); + assertThat(serializers.findSerializer(null, SimpleType.construct(Integer.class), null)).isSameAs(serializer1); + } + @Test void modulesToInstallByClass() { ObjectMapper objectMapper = Jackson2ObjectMapperBuilder.json() @@ -259,6 +269,15 @@ class Jackson2ObjectMapperBuilderTests { assertThat(serializers.findSerializer(null, SimpleType.construct(Integer.class), null).getClass()).isSameAs(CustomIntegerSerializer.class); } + @Test + void modulesToInstallWithConsumer() { + ObjectMapper objectMapper = Jackson2ObjectMapperBuilder.json() + .modulesToInstall(list -> list.add(new CustomIntegerModule())) + .build(); + Serializers serializers = getSerializerFactoryConfig(objectMapper).serializers().iterator().next(); + assertThat(serializers.findSerializer(null, SimpleType.construct(Integer.class), null).getClass()).isSameAs(CustomIntegerSerializer.class); + } + @Test void wellKnownModules() throws JsonProcessingException, UnsupportedEncodingException { ObjectMapper objectMapper = Jackson2ObjectMapperBuilder.json().build();