From f88759c3c9dc048f4a0d98eab973cd65c99fabc1 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Thu, 3 Sep 2020 20:21:00 +0100 Subject: [PATCH] Shared read-only instances of UrlPathHelper UrlPathHelper is often created and used without customizations or with the same customizations. This commit introduces re-usable, instances. Effectively a backport of commit 23233c. See gh-25690 --- .../MockHttpServletRequestBuilder.java | 5 +- .../setup/PatternMappingFilterProxy.java | 6 +- .../web/filter/ForwardedHeaderFilter.java | 26 ++------- .../web/util/UrlPathHelper.java | 55 +++++++++++++++++++ .../condition/PatternsRequestCondition.java | 2 +- ...stractMessageConverterMethodProcessor.java | 16 +----- ...vletCookieValueMethodArgumentResolver.java | 4 +- .../servlet/resource/ResourceUrlProvider.java | 4 +- .../support/AbstractFlashMapManager.java | 4 +- .../support/ServletUriComponentsBuilder.java | 4 +- 10 files changed, 76 insertions(+), 50 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java b/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java index e315c948a8..8b844e5b1d 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/request/MockHttpServletRequestBuilder.java @@ -83,9 +83,6 @@ import org.springframework.web.util.UrlPathHelper; public class MockHttpServletRequestBuilder implements ConfigurableSmartRequestBuilder, Mergeable { - private static final UrlPathHelper urlPathHelper = new UrlPathHelper(); - - private final String method; private final URI url; @@ -781,7 +778,7 @@ public class MockHttpServletRequestBuilder } String extraPath = requestUri.substring(this.contextPath.length() + this.servletPath.length()); this.pathInfo = (StringUtils.hasText(extraPath) ? - urlPathHelper.decodeRequestString(request, extraPath) : null); + UrlPathHelper.defaultInstance.decodeRequestString(request, extraPath) : null); } request.setPathInfo(this.pathInfo); } diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/setup/PatternMappingFilterProxy.java b/spring-test/src/main/java/org/springframework/test/web/servlet/setup/PatternMappingFilterProxy.java index 7ea42b8c7d..1966016813 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/setup/PatternMappingFilterProxy.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/setup/PatternMappingFilterProxy.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -45,8 +45,6 @@ final class PatternMappingFilterProxy implements Filter { private static final String PATH_MAPPING_PATTERN = "/*"; - private static final UrlPathHelper urlPathHelper = new UrlPathHelper(); - private final Filter delegate; /** Patterns that require an exact match, e.g. "/test" */ @@ -96,7 +94,7 @@ final class PatternMappingFilterProxy implements Filter { throws IOException, ServletException { HttpServletRequest httpRequest = (HttpServletRequest) request; - String requestPath = urlPathHelper.getPathWithinApplication(httpRequest); + String requestPath = UrlPathHelper.defaultInstance.getPathWithinApplication(httpRequest); if (matches(requestPath)) { this.delegate.doFilter(request, response, filterChain); diff --git a/spring-web/src/main/java/org/springframework/web/filter/ForwardedHeaderFilter.java b/spring-web/src/main/java/org/springframework/web/filter/ForwardedHeaderFilter.java index 8b5f6a6d6a..b9c4a4c301 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/ForwardedHeaderFilter.java +++ b/spring-web/src/main/java/org/springframework/web/filter/ForwardedHeaderFilter.java @@ -79,20 +79,11 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { } - private final UrlPathHelper pathHelper; - private boolean removeOnly; private boolean relativeRedirects; - public ForwardedHeaderFilter() { - this.pathHelper = new UrlPathHelper(); - this.pathHelper.setUrlDecode(false); - this.pathHelper.setRemoveSemicolonContent(false); - } - - /** * Enables mode in which any "Forwarded" or "X-Forwarded-*" headers are * removed only and the information in them ignored. @@ -149,7 +140,7 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { } else { HttpServletRequest wrappedRequest = - new ForwardedHeaderExtractingRequest(request, this.pathHelper); + new ForwardedHeaderExtractingRequest(request); HttpServletResponse wrappedResponse = this.relativeRedirects ? RelativeRedirectResponseWrapper.wrapIfNecessary(response, HttpStatus.SEE_OTHER) : @@ -230,7 +221,7 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { private final ForwardedPrefixExtractor forwardedPrefixExtractor; - ForwardedHeaderExtractingRequest(HttpServletRequest request, UrlPathHelper pathHelper) { + ForwardedHeaderExtractingRequest(HttpServletRequest request) { super(request); HttpRequest httpRequest = new ServletServerHttpRequest(request); @@ -244,7 +235,7 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { String baseUrl = this.scheme + "://" + this.host + (port == -1 ? "" : ":" + port); Supplier delegateRequest = () -> (HttpServletRequest) getRequest(); - this.forwardedPrefixExtractor = new ForwardedPrefixExtractor(delegateRequest, pathHelper, baseUrl); + this.forwardedPrefixExtractor = new ForwardedPrefixExtractor(delegateRequest, baseUrl); } @@ -296,8 +287,6 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { private final Supplier delegate; - private final UrlPathHelper pathHelper; - private final String baseUrl; private String actualRequestUri; @@ -316,14 +305,10 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { * @param delegateRequest supplier for the current * {@link HttpServletRequestWrapper#getRequest() delegate request} which * may change during a forward (e.g. Tomcat. - * @param pathHelper the path helper instance * @param baseUrl the host, scheme, and port based on forwarded headers */ - public ForwardedPrefixExtractor( - Supplier delegateRequest, UrlPathHelper pathHelper, String baseUrl) { - + public ForwardedPrefixExtractor(Supplier delegateRequest, String baseUrl) { this.delegate = delegateRequest; - this.pathHelper = pathHelper; this.baseUrl = baseUrl; this.actualRequestUri = delegateRequest.get().getRequestURI(); @@ -353,7 +338,8 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter { @Nullable private String initRequestUri() { if (this.forwardedPrefix != null) { - return this.forwardedPrefix + this.pathHelper.getPathWithinApplication(this.delegate.get()); + return this.forwardedPrefix + + UrlPathHelper.rawPathInstance.getPathWithinApplication(this.delegate.get()); } return null; } diff --git a/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java b/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java index fcac7db744..79673637db 100644 --- a/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java +++ b/spring-web/src/main/java/org/springframework/web/util/UrlPathHelper.java @@ -28,6 +28,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.lang.Nullable; +import org.springframework.util.Assert; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.util.StringUtils; @@ -70,6 +71,8 @@ public class UrlPathHelper { private String defaultEncoding = WebUtils.DEFAULT_CHARACTER_ENCODING; + private boolean readOnly = false; + /** * Whether URL lookups should always use the full path within the current @@ -81,6 +84,7 @@ public class UrlPathHelper { *

By default this is set to "false". */ public void setAlwaysUseFullPath(boolean alwaysUseFullPath) { + checkReadOnly(); this.alwaysUseFullPath = alwaysUseFullPath; } @@ -103,6 +107,7 @@ public class UrlPathHelper { * @see java.net.URLDecoder#decode(String, String) */ public void setUrlDecode(boolean urlDecode) { + checkReadOnly(); this.urlDecode = urlDecode; } @@ -119,6 +124,7 @@ public class UrlPathHelper { *

Default is "true". */ public void setRemoveSemicolonContent(boolean removeSemicolonContent) { + checkReadOnly(); this.removeSemicolonContent = removeSemicolonContent; } @@ -126,6 +132,7 @@ public class UrlPathHelper { * Whether configured to remove ";" (semicolon) content from the request URI. */ public boolean shouldRemoveSemicolonContent() { + checkReadOnly(); return this.removeSemicolonContent; } @@ -143,6 +150,7 @@ public class UrlPathHelper { * @see WebUtils#DEFAULT_CHARACTER_ENCODING */ public void setDefaultEncoding(String defaultEncoding) { + checkReadOnly(); this.defaultEncoding = defaultEncoding; } @@ -153,6 +161,17 @@ public class UrlPathHelper { return this.defaultEncoding; } + /** + * Switch to read-only mode where further configuration changes are not allowed. + */ + private void setReadOnly() { + this.readOnly = true; + } + + private void checkReadOnly() { + Assert.isTrue(!this.readOnly, "This instance cannot be modified"); + } + /** * Return the mapping lookup path for the given request, within the current @@ -640,4 +659,40 @@ public class UrlPathHelper { return !flagToUse; } + + + /** + * Shared, read-only instance with defaults. The following apply: + *

    + *
  • {@code alwaysUseFullPath=false} + *
  • {@code urlDecode=true} + *
  • {@code removeSemicolon=true} + *
  • {@code defaultEncoding=}{@link WebUtils#DEFAULT_CHARACTER_ENCODING} + *
+ */ + public static final UrlPathHelper defaultInstance = new UrlPathHelper(); + + static { + defaultInstance.setReadOnly(); + } + + + /** + * Shared, read-only instance for the full, encoded path. The following apply: + *
    + *
  • {@code alwaysUseFullPath=true} + *
  • {@code urlDecode=false} + *
  • {@code removeSemicolon=false} + *
  • {@code defaultEncoding=}{@link WebUtils#DEFAULT_CHARACTER_ENCODING} + *
+ */ + public static final UrlPathHelper rawPathInstance = new UrlPathHelper(); + + static { + rawPathInstance.setAlwaysUseFullPath(true); + rawPathInstance.setUrlDecode(false); + rawPathInstance.setRemoveSemicolonContent(false); + rawPathInstance.setReadOnly(); + } + } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java index 3616aa33a9..4062a05340 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/PatternsRequestCondition.java @@ -121,7 +121,7 @@ public class PatternsRequestCondition extends AbstractRequestCondition fileExtensions) { this.patterns = initPatterns(patterns); - this.pathHelper = urlPathHelper != null ? urlPathHelper : new UrlPathHelper(); + this.pathHelper = urlPathHelper != null ? urlPathHelper : UrlPathHelper.defaultInstance; this.pathMatcher = pathMatcher != null ? pathMatcher : new AntPathMatcher(); this.useSuffixPatternMatch = useSuffixPatternMatch; this.useTrailingSlashMatch = useTrailingSlashMatch; diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java index dbda3378df..932ff78a05 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/AbstractMessageConverterMethodProcessor.java @@ -91,16 +91,6 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe new ParameterizedTypeReference>() { }.getType(); - private static final UrlPathHelper decodingUrlPathHelper = new UrlPathHelper(); - - private static final UrlPathHelper rawUrlPathHelper = new UrlPathHelper(); - - static { - rawUrlPathHelper.setRemoveSemicolonContent(false); - rawUrlPathHelper.setUrlDecode(false); - } - - private final ContentNegotiationManager contentNegotiationManager; private final Set safeExtensions = new HashSet<>(); @@ -430,7 +420,7 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe } HttpServletRequest servletRequest = request.getServletRequest(); - String requestUri = rawUrlPathHelper.getOriginatingRequestUri(servletRequest); + String requestUri = UrlPathHelper.rawPathInstance.getOriginatingRequestUri(servletRequest); int index = requestUri.lastIndexOf('/') + 1; String filename = requestUri.substring(index); @@ -442,10 +432,10 @@ public abstract class AbstractMessageConverterMethodProcessor extends AbstractMe filename = filename.substring(0, index); } - filename = decodingUrlPathHelper.decodeRequestString(servletRequest, filename); + filename = UrlPathHelper.defaultInstance.decodeRequestString(servletRequest, filename); String ext = StringUtils.getFilenameExtension(filename); - pathParams = decodingUrlPathHelper.decodeRequestString(servletRequest, pathParams); + pathParams = UrlPathHelper.defaultInstance.decodeRequestString(servletRequest, pathParams); String extInPathParams = StringUtils.getFilenameExtension(pathParams); if (!safeExtension(servletRequest, ext) || !safeExtension(servletRequest, extInPathParams)) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletCookieValueMethodArgumentResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletCookieValueMethodArgumentResolver.java index d1928fb9f6..97c66a581a 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletCookieValueMethodArgumentResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletCookieValueMethodArgumentResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2020 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. @@ -37,7 +37,7 @@ import org.springframework.web.util.WebUtils; */ public class ServletCookieValueMethodArgumentResolver extends AbstractCookieValueMethodArgumentResolver { - private UrlPathHelper urlPathHelper = new UrlPathHelper(); + private UrlPathHelper urlPathHelper = UrlPathHelper.defaultInstance; public ServletCookieValueMethodArgumentResolver(@Nullable ConfigurableBeanFactory beanFactory) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlProvider.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlProvider.java index 8abd1b5ef9..690de8df2e 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlProvider.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlProvider.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -54,7 +54,7 @@ public class ResourceUrlProvider implements ApplicationListener