Resolve absolute resource links in ResourceTransformers

Prior to this commit, `ResourceTransformer` implementations would
resolve internal links to other resources: both relative and absolute
request paths.
For relative request paths, those transformers would call
`ResourceTransformerSupport.resolveUrlPath` with the resource path,
as provided in the original file. This can cause problems when a
`CachingResourceResolver` is configured in the resolver chain, because
this resolver is caching resources, deriving the cache key from the
given resource path — this can cause collisions for cases like this:

    resources/
    |--foo/
    |  |--foo.css (imports style.css)
    |  |--style.css
    |--bar/
    |  |--bar.css (imports style.css)
    |  |--style.css

The first "style.css" resolved resource is then cached and will be given
to any request asking for "style.css".

To avoid those issues, this commit improves the `ResourceTransformer`
implementations to calculate the absolute request path before asking the
chain to resolve the resource URL, thus avoiding duplications.
The resource chain will be then asked to resolve "/foo/style/css" or
"/bar/style.css".

Issue: SPR-14597
This commit is contained in:
Brian Clozel
2016-10-07 22:25:56 +02:00
parent 9bf8489afd
commit 679b661e19
5 changed files with 76 additions and 37 deletions

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2014 the original author or authors.
* Copyright 2002-2016 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.
@@ -28,14 +28,14 @@ import org.junit.Test;
import org.springframework.core.io.ClassPathResource;
import org.springframework.core.io.Resource;
import org.springframework.mock.web.test.MockHttpServletRequest;
import org.springframework.util.FileCopyUtils;
import static org.junit.Assert.*;
import static org.mockito.BDDMockito.*;
/**
* Unit tests for
* {@link AppCacheManifestTransformer}.
* Unit tests for {@link AppCacheManifestTransformer}.
*
* @author Brian Clozel
*/
@@ -49,13 +49,34 @@ public class AppCacheManifestTransformerTests {
@Before
public void setup() {
ClassPathResource allowedLocation = new ClassPathResource("test/", getClass());
ResourceHttpRequestHandler resourceHandler = new ResourceHttpRequestHandler();
ResourceUrlProvider resourceUrlProvider = new ResourceUrlProvider();
resourceUrlProvider.setHandlerMap(Collections.singletonMap("/static/**", resourceHandler));
VersionResourceResolver versionResolver = new VersionResourceResolver();
versionResolver.setStrategyMap(Collections.singletonMap("/**", new ContentVersionStrategy()));
PathResourceResolver pathResolver = new PathResourceResolver();
pathResolver.setAllowedLocations(allowedLocation);
List<ResourceResolver> resolvers = Arrays.asList(versionResolver, pathResolver);
ResourceResolverChain resolverChain = new DefaultResourceResolverChain(resolvers);
CssLinkResourceTransformer cssLinkResourceTransformer = new CssLinkResourceTransformer();
cssLinkResourceTransformer.setResourceUrlProvider(resourceUrlProvider);
List<ResourceTransformer> transformers = Arrays.asList(cssLinkResourceTransformer);
this.chain = new DefaultResourceTransformerChain(resolverChain, transformers);
this.transformer = new AppCacheManifestTransformer();
this.chain = mock(ResourceTransformerChain.class);
this.request = mock(HttpServletRequest.class);
this.transformer.setResourceUrlProvider(resourceUrlProvider);
resourceHandler.setResourceResolvers(resolvers);
resourceHandler.setResourceTransformers(transformers);
resourceHandler.setLocations(Collections.singletonList(allowedLocation));
}
@Test
public void noTransformIfExtensionNoMatch() throws Exception {
this.chain = mock(ResourceTransformerChain.class);
this.request = mock(HttpServletRequest.class);
Resource resource = mock(Resource.class);
given(resource.getFilename()).willReturn("foobar.file");
given(this.chain.transform(this.request, resource)).willReturn(resource);
@@ -66,6 +87,8 @@ public class AppCacheManifestTransformerTests {
@Test
public void syntaxErrorInManifest() throws Exception {
this.chain = mock(ResourceTransformerChain.class);
this.request = mock(HttpServletRequest.class);
Resource resource = new ClassPathResource("test/error.appcache", getClass());
given(this.chain.transform(this.request, resource)).willReturn(resource);
@@ -75,31 +98,18 @@ public class AppCacheManifestTransformerTests {
@Test
public void transformManifest() throws Exception {
VersionResourceResolver versionResolver = new VersionResourceResolver();
versionResolver.setStrategyMap(Collections.singletonMap("/**", new ContentVersionStrategy()));
PathResourceResolver pathResolver = new PathResourceResolver();
pathResolver.setAllowedLocations(new ClassPathResource("test/", getClass()));
List<ResourceResolver> resolvers = Arrays.asList(versionResolver, pathResolver);
ResourceResolverChain resolverChain = new DefaultResourceResolverChain(resolvers);
List<ResourceTransformer> transformers = new ArrayList<>();
transformers.add(new CssLinkResourceTransformer());
this.chain = new DefaultResourceTransformerChain(resolverChain, transformers);
this.request = new MockHttpServletRequest("GET", "/static/test.appcache");
Resource resource = new ClassPathResource("test/test.appcache", getClass());
Resource result = this.transformer.transform(this.request, resource, this.chain);
byte[] bytes = FileCopyUtils.copyToByteArray(result.getInputStream());
String content = new String(bytes, "UTF-8");
assertThat("should rewrite resource links", content,
Matchers.containsString("foo-e36d2e05253c6c7085a91522ce43a0b4.css"));
Matchers.containsString("/static/foo-e36d2e05253c6c7085a91522ce43a0b4.css"));
assertThat("should rewrite resource links", content,
Matchers.containsString("bar-11e16cf79faee7ac698c805cf28248d2.css"));
Matchers.containsString("/static/bar-11e16cf79faee7ac698c805cf28248d2.css"));
assertThat("should rewrite resource links", content,
Matchers.containsString("js/bar-bd508c62235b832d960298ca6c0b7645.js"));
Matchers.containsString("/static/js/bar-bd508c62235b832d960298ca6c0b7645.js"));
assertThat("should not rewrite external resources", content,
Matchers.containsString("//example.org/style.css"));

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2014 the original author or authors.
* Copyright 2002-2016 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.
@@ -36,6 +36,7 @@ import static org.junit.Assert.*;
* {@link org.springframework.web.servlet.resource.CssLinkResourceTransformer}.
*
* @author Rossen Stoyanchev
* @author Brian Clozel
* @since 4.1
*/
public class CssLinkResourceTransformerTests {
@@ -47,34 +48,44 @@ public class CssLinkResourceTransformerTests {
@Before
public void setUp() {
ClassPathResource allowedLocation = new ClassPathResource("test/", getClass());
ResourceHttpRequestHandler resourceHandler = new ResourceHttpRequestHandler();
VersionResourceResolver versionResolver = new VersionResourceResolver();
versionResolver.setStrategyMap(Collections.singletonMap("/**", new ContentVersionStrategy()));
PathResourceResolver pathResolver = new PathResourceResolver();
pathResolver.setAllowedLocations(new ClassPathResource("test/", getClass()));
pathResolver.setAllowedLocations(allowedLocation);
List<ResourceResolver> resolvers = Arrays.asList(versionResolver, pathResolver);
List<ResourceTransformer> transformers = Arrays.asList(new CssLinkResourceTransformer());
ResourceUrlProvider resourceUrlProvider = new ResourceUrlProvider();
resourceUrlProvider.setHandlerMap(Collections.singletonMap("/static/**", resourceHandler));
CssLinkResourceTransformer cssLinkResourceTransformer = new CssLinkResourceTransformer();
cssLinkResourceTransformer.setResourceUrlProvider(resourceUrlProvider);
List<ResourceTransformer> transformers = Arrays.asList(cssLinkResourceTransformer);
resourceHandler.setResourceResolvers(resolvers);
resourceHandler.setResourceTransformers(transformers);
resourceHandler.setLocations(Collections.singletonList(allowedLocation));
ResourceResolverChain resolverChain = new DefaultResourceResolverChain(resolvers);
this.transformerChain = new DefaultResourceTransformerChain(resolverChain, transformers);
this.request = new MockHttpServletRequest();
}
@Test
public void transform() throws Exception {
this.request = new MockHttpServletRequest("GET", "/static/main.css");
Resource css = new ClassPathResource("test/main.css", getClass());
TransformedResource actual = (TransformedResource) this.transformerChain.transform(this.request, css);
String expected = "\n" +
"@import url(\"bar-11e16cf79faee7ac698c805cf28248d2.css\");\n" +
"@import url('bar-11e16cf79faee7ac698c805cf28248d2.css');\n" +
"@import url(bar-11e16cf79faee7ac698c805cf28248d2.css);\n\n" +
"@import \"foo-e36d2e05253c6c7085a91522ce43a0b4.css\";\n" +
"@import 'foo-e36d2e05253c6c7085a91522ce43a0b4.css';\n\n" +
"body { background: url(\"images/image-f448cd1d5dba82b774f3202c878230b3.png\") }\n";
"@import url(\"/static/bar-11e16cf79faee7ac698c805cf28248d2.css\");\n" +
"@import url('/static/bar-11e16cf79faee7ac698c805cf28248d2.css');\n" +
"@import url(/static/bar-11e16cf79faee7ac698c805cf28248d2.css);\n\n" +
"@import \"/static/foo-e36d2e05253c6c7085a91522ce43a0b4.css\";\n" +
"@import '/static/foo-e36d2e05253c6c7085a91522ce43a0b4.css';\n\n" +
"body { background: url(\"/static/images/image-f448cd1d5dba82b774f3202c878230b3.png\") }\n";
String result = new String(actual.getByteArray(), "UTF-8");
result = StringUtils.deleteAny(result, "\r");
@@ -83,6 +94,7 @@ public class CssLinkResourceTransformerTests {
@Test
public void transformNoLinks() throws Exception {
this.request = new MockHttpServletRequest("GET", "/static/foo.css");
Resource expected = new ClassPathResource("test/foo.css", getClass());
Resource actual = this.transformerChain.transform(this.request, expected);
assertSame(expected, actual);
@@ -90,6 +102,7 @@ public class CssLinkResourceTransformerTests {
@Test
public void transformExtLinksNotAllowed() throws Exception {
this.request = new MockHttpServletRequest("GET", "/static/external.css");
ResourceResolverChain resolverChain = Mockito.mock(DefaultResourceResolverChain.class);
ResourceTransformerChain transformerChain = new DefaultResourceTransformerChain(resolverChain,
Arrays.asList(new CssLinkResourceTransformer()));
@@ -115,6 +128,7 @@ public class CssLinkResourceTransformerTests {
@Test
public void transformWithNonCssResource() throws Exception {
this.request = new MockHttpServletRequest("GET", "/static/images/image.png");
Resource expected = new ClassPathResource("test/images/image.png", getClass());
Resource actual = this.transformerChain.transform(this.request, expected);
assertSame(expected, actual);