From 9f7c8aea9419632aa86e64eb43a9deee71ceaef1 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Wed, 17 Feb 2021 22:10:29 +0100 Subject: [PATCH] Fix ResourceUrlProvider handler auto-detection Prior to this commit, `ResourceUrlProvider` would listen and consider all `ContextRefreshedEvent` and use the given context to detect `SimpleUrlHandlerMapping`. This could lead to situations where a `ResourceUrlProvider` uses another application context than its own (in a parent/child context setup) and detect the wrong set of handlers. Because `ResourceUrlProvider` locks itself once the auto-detection is done, we need to ensure that it considers only events sent by its application context. Fixes gh-26562 --- .../resource/ResourceUrlProvider.java | 19 +++++-- .../resource/ResourceUrlProviderTests.java | 45 +++++++++++++--- .../servlet/resource/ResourceUrlProvider.java | 18 +++++-- .../resource/ResourceUrlProviderTests.java | 51 +++++++++++++++---- 4 files changed, 109 insertions(+), 24 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java index 8bfcb0ccd7d..2146fbdde3f 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 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. @@ -26,12 +26,15 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Mono; +import org.springframework.beans.BeansException; import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; import org.springframework.context.ApplicationListener; import org.springframework.context.event.ContextRefreshedEvent; import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.http.server.PathContainer; import org.springframework.http.server.reactive.ServerHttpRequest; +import org.springframework.lang.Nullable; import org.springframework.util.StringUtils; import org.springframework.web.reactive.handler.SimpleUrlHandlerMapping; import org.springframework.web.server.ServerWebExchange; @@ -47,15 +50,23 @@ import org.springframework.web.util.pattern.PathPatternParser; * {@code ResourceHttpRequestHandler}s to make its decisions. * * @author Rossen Stoyanchev + * @author Brian Clozel * @since 5.0 */ -public class ResourceUrlProvider implements ApplicationListener { +public class ResourceUrlProvider implements ApplicationListener, ApplicationContextAware { private static final Log logger = LogFactory.getLog(ResourceUrlProvider.class); private final Map handlerMap = new LinkedHashMap<>(); + @Nullable + private ApplicationContext applicationContext; + + @Override + public void setApplicationContext(ApplicationContext applicationContext) throws BeansException { + this.applicationContext = applicationContext; + } /** * Return a read-only view of the resource handler mappings either manually @@ -83,8 +94,8 @@ public class ResourceUrlProvider implements ApplicationListener resolvers = new ArrayList<>(); @@ -108,7 +109,7 @@ public class ResourceUrlProviderTests { } @Test // SPR-12647 - public void bestPatternMatch() { + void bestPatternMatch() { ResourceWebHandler otherHandler = new ResourceWebHandler(); otherHandler.setLocations(this.locations); @@ -129,7 +130,7 @@ public class ResourceUrlProviderTests { @Test // SPR-12592 @SuppressWarnings("resource") - public void initializeOnce() { + void initializeOnce() { AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext(); context.setServletContext(new MockServletContext()); context.register(HandlerMappingConfiguration.class); @@ -139,6 +140,26 @@ public class ResourceUrlProviderTests { .hasKeySatisfying(pathPatternStringOf("/resources/**")); } + @Test + void initializeOnCurrentContext() { + AnnotationConfigWebApplicationContext parentContext = new AnnotationConfigWebApplicationContext(); + parentContext.setServletContext(new MockServletContext()); + parentContext.register(ParentHandlerMappingConfiguration.class); + + AnnotationConfigWebApplicationContext childContext = new AnnotationConfigWebApplicationContext(); + childContext.setParent(parentContext); + childContext.setServletContext(new MockServletContext()); + childContext.register(HandlerMappingConfiguration.class); + + parentContext.refresh(); + childContext.refresh(); + + ResourceUrlProvider parentUrlProvider = parentContext.getBean(ResourceUrlProvider.class); + assertThat(parentUrlProvider.getHandlerMap()).isEmpty(); + ResourceUrlProvider childUrlProvider = childContext.getBean(ResourceUrlProvider.class); + assertThat(childUrlProvider.getHandlerMap()).hasKeySatisfying(pathPatternStringOf("/resources/**")); + } + private Condition pathPatternStringOf(String expected) { return new Condition( @@ -161,4 +182,14 @@ public class ResourceUrlProviderTests { } } + @Configuration + @SuppressWarnings({"unused", "WeakerAccess"}) + static class ParentHandlerMappingConfiguration { + + @Bean + public ResourceUrlProvider resourceUrlProvider() { + return new ResourceUrlProvider(); + } + } + } 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 690de8df2ee..28807ac5948 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-2020 the original author or authors. + * Copyright 2002-2021 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. @@ -27,7 +27,9 @@ import javax.servlet.http.HttpServletRequest; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.springframework.beans.BeansException; import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; import org.springframework.context.ApplicationListener; import org.springframework.context.event.ContextRefreshedEvent; import org.springframework.core.annotation.AnnotationAwareOrderComparator; @@ -48,12 +50,16 @@ import org.springframework.web.util.UrlPathHelper; * {@code ResourceHttpRequestHandler}s to make its decisions. * * @author Rossen Stoyanchev + * @author Brian Clozel * @since 4.1 */ -public class ResourceUrlProvider implements ApplicationListener { +public class ResourceUrlProvider implements ApplicationListener, ApplicationContextAware { protected final Log logger = LogFactory.getLog(getClass()); + @Nullable + private ApplicationContext applicationContext; + private UrlPathHelper urlPathHelper = UrlPathHelper.defaultInstance; private PathMatcher pathMatcher = new AntPathMatcher(); @@ -62,6 +68,10 @@ public class ResourceUrlProvider implements ApplicationListener versionStrategyMap = new HashMap<>(); versionStrategyMap.put("/**", new ContentVersionStrategy()); VersionResourceResolver versionResolver = new VersionResourceResolver(); @@ -116,7 +117,7 @@ public class ResourceUrlProviderTests { } @Test // SPR-12647 - public void bestPatternMatch() throws Exception { + void bestPatternMatch() throws Exception { ResourceHttpRequestHandler otherHandler = new ResourceHttpRequestHandler(); otherHandler.setLocations(this.locations); Map versionStrategyMap = new HashMap<>(); @@ -138,7 +139,7 @@ public class ResourceUrlProviderTests { @Test // SPR-12592 @SuppressWarnings("resource") - public void initializeOnce() throws Exception { + void initializeOnce() throws Exception { AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext(); context.setServletContext(new MockServletContext()); context.register(HandlerMappingConfiguration.class); @@ -149,8 +150,30 @@ public class ResourceUrlProviderTests { assertThat(urlProviderBean.isAutodetect()).isFalse(); } + @Test + void initializeOnCurrentContext() { + AnnotationConfigWebApplicationContext parentContext = new AnnotationConfigWebApplicationContext(); + parentContext.setServletContext(new MockServletContext()); + parentContext.register(ParentHandlerMappingConfiguration.class); + + AnnotationConfigWebApplicationContext childContext = new AnnotationConfigWebApplicationContext(); + childContext.setParent(parentContext); + childContext.setServletContext(new MockServletContext()); + childContext.register(HandlerMappingConfiguration.class); + + parentContext.refresh(); + childContext.refresh(); + + ResourceUrlProvider parentUrlProvider = parentContext.getBean(ResourceUrlProvider.class); + assertThat(parentUrlProvider.getHandlerMap()).isEmpty(); + assertThat(parentUrlProvider.isAutodetect()).isTrue(); + ResourceUrlProvider childUrlProvider = childContext.getBean(ResourceUrlProvider.class); + assertThat(childUrlProvider.getHandlerMap()).containsOnlyKeys("/resources/**"); + assertThat(childUrlProvider.isAutodetect()).isFalse(); + } + @Test // SPR-16296 - public void getForLookupPathShouldNotFailIfPathContainsDoubleSlashes() { + void getForLookupPathShouldNotFailIfPathContainsDoubleSlashes() { // given ResourceResolver mockResourceResolver = mock(ResourceResolver.class); given(mockResourceResolver.resolveUrlPath(any(), any(), any())).willReturn("some-path"); @@ -185,4 +208,14 @@ public class ResourceUrlProviderTests { } } + @Configuration + @SuppressWarnings({"unused", "WeakerAccess"}) + static class ParentHandlerMappingConfiguration { + + @Bean + public ResourceUrlProvider resourceUrlProvider() { + return new ResourceUrlProvider(); + } + } + }