Skip to content

Commit

Permalink
Fix ResourceUrlProvider handler auto-detection
Browse files Browse the repository at this point in the history
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-26561
  • Loading branch information
bclozel committed Feb 17, 2021
1 parent 49ccd7a commit 947387b
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 24 deletions.
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -26,12 +26,15 @@
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;
Expand All @@ -47,15 +50,23 @@
* {@code ResourceHttpRequestHandler}s to make its decisions.
*
* @author Rossen Stoyanchev
* @author Brian Clozel
* @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 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
Expand Down Expand Up @@ -83,8 +94,8 @@ public void registerHandlers(Map<String, ResourceWebHandler> handlerMap) {

@Override
public void onApplicationEvent(ContextRefreshedEvent event) {
if (this.handlerMap.isEmpty()) {
detectResourceHandlers(event.getApplicationContext());
if (this.applicationContext == event.getApplicationContext() && this.handlerMap.isEmpty()) {
detectResourceHandlers(this.applicationContext);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -44,6 +44,7 @@
* Unit tests for {@link ResourceUrlProvider}.
*
* @author Rossen Stoyanchev
* @author Brian Clozel
*/
public class ResourceUrlProviderTests {

Expand All @@ -62,7 +63,7 @@ public class ResourceUrlProviderTests {


@BeforeEach
public void setup() throws Exception {
void setup() throws Exception {
this.locations.add(new ClassPathResource("test/", getClass()));
this.locations.add(new ClassPathResource("testalternatepath/", getClass()));
this.handler.setLocations(this.locations);
Expand All @@ -73,15 +74,15 @@ public void setup() throws Exception {


@Test
public void getStaticResourceUrl() {
void getStaticResourceUrl() {
String expected = "/resources/foo.css";
String actual = this.urlProvider.getForUriString(expected, this.exchange).block(TIMEOUT);

assertThat(actual).isEqualTo(expected);
}

@Test // SPR-13374
public void getStaticResourceUrlRequestWithQueryOrHash() {
void getStaticResourceUrlRequestWithQueryOrHash() {

String url = "/resources/foo.css?foo=bar&url=https://example.org";
String resolvedUrl = this.urlProvider.getForUriString(url, this.exchange).block(TIMEOUT);
Expand All @@ -93,7 +94,7 @@ public void getStaticResourceUrlRequestWithQueryOrHash() {
}

@Test
public void getVersionedResourceUrl() {
void getVersionedResourceUrl() {
VersionResourceResolver versionResolver = new VersionResourceResolver();
versionResolver.setStrategyMap(Collections.singletonMap("/**", new ContentVersionStrategy()));
List<ResourceResolver> resolvers = new ArrayList<>();
Expand All @@ -108,7 +109,7 @@ public void getVersionedResourceUrl() {
}

@Test // SPR-12647
public void bestPatternMatch() {
void bestPatternMatch() {
ResourceWebHandler otherHandler = new ResourceWebHandler();
otherHandler.setLocations(this.locations);

Expand All @@ -129,7 +130,7 @@ public void bestPatternMatch() {

@Test // SPR-12592
@SuppressWarnings("resource")
public void initializeOnce() {
void initializeOnce() {
AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext();
context.setServletContext(new MockServletContext());
context.register(HandlerMappingConfiguration.class);
Expand All @@ -139,6 +140,26 @@ public void initializeOnce() {
.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) {
return new Condition<PathPattern>(
Expand All @@ -161,4 +182,14 @@ public ResourceUrlProvider resourceUrlProvider() {
}
}

@Configuration
@SuppressWarnings({"unused", "WeakerAccess"})
static class ParentHandlerMappingConfiguration {

@Bean
public ResourceUrlProvider resourceUrlProvider() {
return new ResourceUrlProvider();
}
}

}
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -27,7 +27,9 @@
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;
Expand All @@ -47,12 +49,16 @@
* {@code ResourceHttpRequestHandler}s to make its decisions.
*
* @author Rossen Stoyanchev
* @author Brian Clozel
* @since 4.1
*/
public class ResourceUrlProvider implements ApplicationListener<ContextRefreshedEvent> {
public class ResourceUrlProvider implements ApplicationListener<ContextRefreshedEvent>, ApplicationContextAware {

protected final Log logger = LogFactory.getLog(getClass());

@Nullable
private ApplicationContext applicationContext;

private UrlPathHelper urlPathHelper = UrlPathHelper.defaultInstance;

private PathMatcher pathMatcher = new AntPathMatcher();
Expand All @@ -61,6 +67,10 @@ public class ResourceUrlProvider implements ApplicationListener<ContextRefreshed

private boolean autodetect = true;

@Override
public void setApplicationContext(ApplicationContext applicationContext) throws BeansException {
this.applicationContext = applicationContext;
}

/**
* Configure a {@code UrlPathHelper} to use in
Expand Down Expand Up @@ -126,9 +136,9 @@ public boolean isAutodetect() {

@Override
public void onApplicationEvent(ContextRefreshedEvent event) {
if (isAutodetect()) {
if (event.getApplicationContext() == this.applicationContext && isAutodetect()) {
this.handlerMap.clear();
detectResourceHandlers(event.getApplicationContext());
detectResourceHandlers(this.applicationContext);
if (!this.handlerMap.isEmpty()) {
this.autodetect = false;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -44,6 +44,7 @@
*
* @author Jeremy Grelle
* @author Rossen Stoyanchev
* @author Brian Clozel
*/
public class ResourceUrlProviderTests {

Expand All @@ -57,7 +58,7 @@ public class ResourceUrlProviderTests {


@BeforeEach
public void setUp() throws Exception {
void setUp() throws Exception {
this.locations.add(new ClassPathResource("test/", getClass()));
this.locations.add(new ClassPathResource("testalternatepath/", getClass()));
this.handler.setServletContext(new MockServletContext());
Expand All @@ -69,13 +70,13 @@ public void setUp() throws Exception {


@Test
public void getStaticResourceUrl() {
void getStaticResourceUrl() {
String url = this.urlProvider.getForLookupPath("/resources/foo.css");
assertThat(url).isEqualTo("/resources/foo.css");
}

@Test // SPR-13374
public void getStaticResourceUrlRequestWithQueryOrHash() {
void getStaticResourceUrlRequestWithQueryOrHash() {
MockHttpServletRequest request = new MockHttpServletRequest();
request.setContextPath("/");
request.setRequestURI("/");
Expand All @@ -90,7 +91,7 @@ public void getStaticResourceUrlRequestWithQueryOrHash() {
}

@Test // SPR-16526
public void getStaticResourceWithMissingContextPath() {
void getStaticResourceWithMissingContextPath() {
MockHttpServletRequest request = new MockHttpServletRequest();
request.setContextPath("/contextpath-longer-than-request-path");
request.setRequestURI("/contextpath-longer-than-request-path/style.css");
Expand All @@ -100,7 +101,7 @@ public void getStaticResourceWithMissingContextPath() {
}

@Test
public void getFingerprintedResourceUrl() {
void getFingerprintedResourceUrl() {
Map<String, VersionStrategy> versionStrategyMap = new HashMap<>();
versionStrategyMap.put("/**", new ContentVersionStrategy());
VersionResourceResolver versionResolver = new VersionResourceResolver();
Expand All @@ -116,7 +117,7 @@ public void getFingerprintedResourceUrl() {
}

@Test // SPR-12647
public void bestPatternMatch() throws Exception {
void bestPatternMatch() throws Exception {
ResourceHttpRequestHandler otherHandler = new ResourceHttpRequestHandler();
otherHandler.setLocations(this.locations);
Map<String, VersionStrategy> versionStrategyMap = new HashMap<>();
Expand All @@ -138,7 +139,7 @@ public void bestPatternMatch() throws Exception {

@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);
Expand All @@ -149,8 +150,30 @@ public void initializeOnce() throws Exception {
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");
Expand Down Expand Up @@ -185,4 +208,14 @@ public ResourceUrlProvider resourceUrlProvider() {
}
}

@Configuration
@SuppressWarnings({"unused", "WeakerAccess"})
static class ParentHandlerMappingConfiguration {

@Bean
public ResourceUrlProvider resourceUrlProvider() {
return new ResourceUrlProvider();
}
}

}

0 comments on commit 947387b

Please sign in to comment.