Browse Source

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
pull/27107/head
Brian Clozel 5 years ago
parent
commit
9f7c8aea94
  1. 19
      spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceUrlProvider.java
  2. 45
      spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceUrlProviderTests.java
  3. 18
      spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlProvider.java
  4. 51
      spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlProviderTests.java

19
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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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 org.apache.commons.logging.LogFactory;
import reactor.core.publisher.Mono; import reactor.core.publisher.Mono;
import org.springframework.beans.BeansException;
import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;
import org.springframework.context.ApplicationListener; import org.springframework.context.ApplicationListener;
import org.springframework.context.event.ContextRefreshedEvent; import org.springframework.context.event.ContextRefreshedEvent;
import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.core.annotation.AnnotationAwareOrderComparator;
import org.springframework.http.server.PathContainer; import org.springframework.http.server.PathContainer;
import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpRequest;
import org.springframework.lang.Nullable;
import org.springframework.util.StringUtils; import org.springframework.util.StringUtils;
import org.springframework.web.reactive.handler.SimpleUrlHandlerMapping; import org.springframework.web.reactive.handler.SimpleUrlHandlerMapping;
import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.ServerWebExchange;
@ -47,15 +50,23 @@ import org.springframework.web.util.pattern.PathPatternParser;
* {@code ResourceHttpRequestHandler}s to make its decisions. * {@code ResourceHttpRequestHandler}s to make its decisions.
* *
* @author Rossen Stoyanchev * @author Rossen Stoyanchev
* @author Brian Clozel
* @since 5.0 * @since 5.0
*/ */
public class ResourceUrlProvider implements ApplicationListener<ContextRefreshedEvent> { public class ResourceUrlProvider implements ApplicationListener<ContextRefreshedEvent>, ApplicationContextAware {
private static final Log logger = LogFactory.getLog(ResourceUrlProvider.class); private static final Log logger = LogFactory.getLog(ResourceUrlProvider.class);
private final Map<PathPattern, ResourceWebHandler> handlerMap = new LinkedHashMap<>(); private final Map<PathPattern, ResourceWebHandler> 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 * Return a read-only view of the resource handler mappings either manually
@ -83,8 +94,8 @@ public class ResourceUrlProvider implements ApplicationListener<ContextRefreshed
@Override @Override
public void onApplicationEvent(ContextRefreshedEvent event) { public void onApplicationEvent(ContextRefreshedEvent event) {
if (this.handlerMap.isEmpty()) { if (this.applicationContext == event.getApplicationContext() && this.handlerMap.isEmpty()) {
detectResourceHandlers(event.getApplicationContext()); detectResourceHandlers(this.applicationContext);
} }
} }

45
spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceUrlProviderTests.java

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2019 the original author or authors. * Copyright 2002-2021 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -44,6 +44,7 @@ import static org.springframework.web.testfixture.http.server.reactive.MockServe
* Unit tests for {@link ResourceUrlProvider}. * Unit tests for {@link ResourceUrlProvider}.
* *
* @author Rossen Stoyanchev * @author Rossen Stoyanchev
* @author Brian Clozel
*/ */
public class ResourceUrlProviderTests { public class ResourceUrlProviderTests {
@ -62,7 +63,7 @@ public class ResourceUrlProviderTests {
@BeforeEach @BeforeEach
public void setup() throws Exception { void setup() throws Exception {
this.locations.add(new ClassPathResource("test/", getClass())); this.locations.add(new ClassPathResource("test/", getClass()));
this.locations.add(new ClassPathResource("testalternatepath/", getClass())); this.locations.add(new ClassPathResource("testalternatepath/", getClass()));
this.handler.setLocations(this.locations); this.handler.setLocations(this.locations);
@ -73,7 +74,7 @@ public class ResourceUrlProviderTests {
@Test @Test
public void getStaticResourceUrl() { void getStaticResourceUrl() {
String expected = "/resources/foo.css"; String expected = "/resources/foo.css";
String actual = this.urlProvider.getForUriString(expected, this.exchange).block(TIMEOUT); String actual = this.urlProvider.getForUriString(expected, this.exchange).block(TIMEOUT);
@ -81,7 +82,7 @@ public class ResourceUrlProviderTests {
} }
@Test // SPR-13374 @Test // SPR-13374
public void getStaticResourceUrlRequestWithQueryOrHash() { void getStaticResourceUrlRequestWithQueryOrHash() {
String url = "/resources/foo.css?foo=bar&url=https://example.org"; String url = "/resources/foo.css?foo=bar&url=https://example.org";
String resolvedUrl = this.urlProvider.getForUriString(url, this.exchange).block(TIMEOUT); String resolvedUrl = this.urlProvider.getForUriString(url, this.exchange).block(TIMEOUT);
@ -93,7 +94,7 @@ public class ResourceUrlProviderTests {
} }
@Test @Test
public void getVersionedResourceUrl() { void getVersionedResourceUrl() {
VersionResourceResolver versionResolver = new VersionResourceResolver(); VersionResourceResolver versionResolver = new VersionResourceResolver();
versionResolver.setStrategyMap(Collections.singletonMap("/**", new ContentVersionStrategy())); versionResolver.setStrategyMap(Collections.singletonMap("/**", new ContentVersionStrategy()));
List<ResourceResolver> resolvers = new ArrayList<>(); List<ResourceResolver> resolvers = new ArrayList<>();
@ -108,7 +109,7 @@ public class ResourceUrlProviderTests {
} }
@Test // SPR-12647 @Test // SPR-12647
public void bestPatternMatch() { void bestPatternMatch() {
ResourceWebHandler otherHandler = new ResourceWebHandler(); ResourceWebHandler otherHandler = new ResourceWebHandler();
otherHandler.setLocations(this.locations); otherHandler.setLocations(this.locations);
@ -129,7 +130,7 @@ public class ResourceUrlProviderTests {
@Test // SPR-12592 @Test // SPR-12592
@SuppressWarnings("resource") @SuppressWarnings("resource")
public void initializeOnce() { void initializeOnce() {
AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext(); AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext();
context.setServletContext(new MockServletContext()); context.setServletContext(new MockServletContext());
context.register(HandlerMappingConfiguration.class); context.register(HandlerMappingConfiguration.class);
@ -139,6 +140,26 @@ public class ResourceUrlProviderTests {
.hasKeySatisfying(pathPatternStringOf("/resources/**")); .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<PathPattern> pathPatternStringOf(String expected) { private Condition<PathPattern> pathPatternStringOf(String expected) {
return new Condition<PathPattern>( return new Condition<PathPattern>(
@ -161,4 +182,14 @@ public class ResourceUrlProviderTests {
} }
} }
@Configuration
@SuppressWarnings({"unused", "WeakerAccess"})
static class ParentHandlerMappingConfiguration {
@Bean
public ResourceUrlProvider resourceUrlProvider() {
return new ResourceUrlProvider();
}
}
} }

18
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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with 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.Log;
import org.apache.commons.logging.LogFactory; import org.apache.commons.logging.LogFactory;
import org.springframework.beans.BeansException;
import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;
import org.springframework.context.ApplicationListener; import org.springframework.context.ApplicationListener;
import org.springframework.context.event.ContextRefreshedEvent; import org.springframework.context.event.ContextRefreshedEvent;
import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.core.annotation.AnnotationAwareOrderComparator;
@ -48,12 +50,16 @@ import org.springframework.web.util.UrlPathHelper;
* {@code ResourceHttpRequestHandler}s to make its decisions. * {@code ResourceHttpRequestHandler}s to make its decisions.
* *
* @author Rossen Stoyanchev * @author Rossen Stoyanchev
* @author Brian Clozel
* @since 4.1 * @since 4.1
*/ */
public class ResourceUrlProvider implements ApplicationListener<ContextRefreshedEvent> { public class ResourceUrlProvider implements ApplicationListener<ContextRefreshedEvent>, ApplicationContextAware {
protected final Log logger = LogFactory.getLog(getClass()); protected final Log logger = LogFactory.getLog(getClass());
@Nullable
private ApplicationContext applicationContext;
private UrlPathHelper urlPathHelper = UrlPathHelper.defaultInstance; private UrlPathHelper urlPathHelper = UrlPathHelper.defaultInstance;
private PathMatcher pathMatcher = new AntPathMatcher(); private PathMatcher pathMatcher = new AntPathMatcher();
@ -62,6 +68,10 @@ public class ResourceUrlProvider implements ApplicationListener<ContextRefreshed
private boolean autodetect = true; private boolean autodetect = true;
@Override
public void setApplicationContext(ApplicationContext applicationContext) throws BeansException {
this.applicationContext = applicationContext;
}
/** /**
* Configure a {@code UrlPathHelper} to use in * Configure a {@code UrlPathHelper} to use in
@ -127,9 +137,9 @@ public class ResourceUrlProvider implements ApplicationListener<ContextRefreshed
@Override @Override
public void onApplicationEvent(ContextRefreshedEvent event) { public void onApplicationEvent(ContextRefreshedEvent event) {
if (isAutodetect()) { if (event.getApplicationContext() == this.applicationContext && isAutodetect()) {
this.handlerMap.clear(); this.handlerMap.clear();
detectResourceHandlers(event.getApplicationContext()); detectResourceHandlers(this.applicationContext);
if (!this.handlerMap.isEmpty()) { if (!this.handlerMap.isEmpty()) {
this.autodetect = false; this.autodetect = false;
} }

51
spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlProviderTests.java

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2019 the original author or authors. * Copyright 2002-2021 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -44,6 +44,7 @@ import static org.mockito.Mockito.mock;
* *
* @author Jeremy Grelle * @author Jeremy Grelle
* @author Rossen Stoyanchev * @author Rossen Stoyanchev
* @author Brian Clozel
*/ */
public class ResourceUrlProviderTests { public class ResourceUrlProviderTests {
@ -57,7 +58,7 @@ public class ResourceUrlProviderTests {
@BeforeEach @BeforeEach
public void setUp() throws Exception { void setUp() throws Exception {
this.locations.add(new ClassPathResource("test/", getClass())); this.locations.add(new ClassPathResource("test/", getClass()));
this.locations.add(new ClassPathResource("testalternatepath/", getClass())); this.locations.add(new ClassPathResource("testalternatepath/", getClass()));
this.handler.setServletContext(new MockServletContext()); this.handler.setServletContext(new MockServletContext());
@ -69,13 +70,13 @@ public class ResourceUrlProviderTests {
@Test @Test
public void getStaticResourceUrl() { void getStaticResourceUrl() {
String url = this.urlProvider.getForLookupPath("/resources/foo.css"); String url = this.urlProvider.getForLookupPath("/resources/foo.css");
assertThat(url).isEqualTo("/resources/foo.css"); assertThat(url).isEqualTo("/resources/foo.css");
} }
@Test // SPR-13374 @Test // SPR-13374
public void getStaticResourceUrlRequestWithQueryOrHash() { void getStaticResourceUrlRequestWithQueryOrHash() {
MockHttpServletRequest request = new MockHttpServletRequest(); MockHttpServletRequest request = new MockHttpServletRequest();
request.setContextPath("/"); request.setContextPath("/");
request.setRequestURI("/"); request.setRequestURI("/");
@ -90,7 +91,7 @@ public class ResourceUrlProviderTests {
} }
@Test // SPR-16526 @Test // SPR-16526
public void getStaticResourceWithMissingContextPath() { void getStaticResourceWithMissingContextPath() {
MockHttpServletRequest request = new MockHttpServletRequest(); MockHttpServletRequest request = new MockHttpServletRequest();
request.setContextPath("/contextpath-longer-than-request-path"); request.setContextPath("/contextpath-longer-than-request-path");
request.setRequestURI("/contextpath-longer-than-request-path/style.css"); request.setRequestURI("/contextpath-longer-than-request-path/style.css");
@ -100,7 +101,7 @@ public class ResourceUrlProviderTests {
} }
@Test @Test
public void getFingerprintedResourceUrl() { void getFingerprintedResourceUrl() {
Map<String, VersionStrategy> versionStrategyMap = new HashMap<>(); Map<String, VersionStrategy> versionStrategyMap = new HashMap<>();
versionStrategyMap.put("/**", new ContentVersionStrategy()); versionStrategyMap.put("/**", new ContentVersionStrategy());
VersionResourceResolver versionResolver = new VersionResourceResolver(); VersionResourceResolver versionResolver = new VersionResourceResolver();
@ -116,7 +117,7 @@ public class ResourceUrlProviderTests {
} }
@Test // SPR-12647 @Test // SPR-12647
public void bestPatternMatch() throws Exception { void bestPatternMatch() throws Exception {
ResourceHttpRequestHandler otherHandler = new ResourceHttpRequestHandler(); ResourceHttpRequestHandler otherHandler = new ResourceHttpRequestHandler();
otherHandler.setLocations(this.locations); otherHandler.setLocations(this.locations);
Map<String, VersionStrategy> versionStrategyMap = new HashMap<>(); Map<String, VersionStrategy> versionStrategyMap = new HashMap<>();
@ -138,7 +139,7 @@ public class ResourceUrlProviderTests {
@Test // SPR-12592 @Test // SPR-12592
@SuppressWarnings("resource") @SuppressWarnings("resource")
public void initializeOnce() throws Exception { void initializeOnce() throws Exception {
AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext(); AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext();
context.setServletContext(new MockServletContext()); context.setServletContext(new MockServletContext());
context.register(HandlerMappingConfiguration.class); context.register(HandlerMappingConfiguration.class);
@ -149,8 +150,30 @@ public class ResourceUrlProviderTests {
assertThat(urlProviderBean.isAutodetect()).isFalse(); 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 @Test // SPR-16296
public void getForLookupPathShouldNotFailIfPathContainsDoubleSlashes() { void getForLookupPathShouldNotFailIfPathContainsDoubleSlashes() {
// given // given
ResourceResolver mockResourceResolver = mock(ResourceResolver.class); ResourceResolver mockResourceResolver = mock(ResourceResolver.class);
given(mockResourceResolver.resolveUrlPath(any(), any(), any())).willReturn("some-path"); 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();
}
}
} }

Loading…
Cancel
Save