From 58b17bff2280c1c504fb6bdc68356dbd110cebf1 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 19 Mar 2019 12:06:45 -0400 Subject: [PATCH] Remove framgent in ResourceUrlEncodingFilter Closes gh-22552 --- .../resource/ResourceUrlEncodingFilter.java | 17 ++- .../ResourceUrlEncodingFilterTests.java | 130 ++++++++---------- 2 files changed, 73 insertions(+), 74 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlEncodingFilter.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlEncodingFilter.java index 64dfed2f7c9..297911806c9 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlEncodingFilter.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlEncodingFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2019 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. @@ -111,7 +111,7 @@ public class ResourceUrlEncodingFilter extends GenericFilterBean { return null; } if (this.indexLookupPath != null && url.startsWith(this.prefixLookupPath)) { - int suffixIndex = getQueryParamsIndex(url); + int suffixIndex = getEndPathIndex(url); String suffix = url.substring(suffixIndex); String lookupPath = url.substring(this.indexLookupPath, suffixIndex); lookupPath = this.resourceUrlProvider.getForLookupPath(lookupPath); @@ -122,9 +122,16 @@ public class ResourceUrlEncodingFilter extends GenericFilterBean { return null; } - private int getQueryParamsIndex(String url) { - int index = url.indexOf('?'); - return (index > 0 ? index : url.length()); + private int getEndPathIndex(String path) { + int end = path.indexOf('?'); + int fragmentIndex = path.indexOf('#'); + if (fragmentIndex != -1 && (end == -1 || fragmentIndex < end)) { + end = fragmentIndex; + } + if (end == -1) { + end = path.length(); + } + return end; } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlEncodingFilterTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlEncodingFilterTests.java index 7722f763198..a7b1f431613 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlEncodingFilterTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlEncodingFilterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2019 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. @@ -15,9 +15,11 @@ */ package org.springframework.web.servlet.resource; -import java.util.Arrays; +import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; import java.util.List; +import javax.servlet.ServletException; import javax.servlet.http.HttpServletResponse; import org.junit.Before; @@ -27,7 +29,7 @@ import org.springframework.core.io.ClassPathResource; import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.mock.web.test.MockHttpServletResponse; -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.*; /** * Unit tests for {@code ResourceUrlEncodingFilter}. @@ -40,41 +42,45 @@ public class ResourceUrlEncodingFilterTests { private ResourceUrlProvider urlProvider; + @Before - public void createFilter() throws Exception { + public void createFilter() { VersionResourceResolver versionResolver = new VersionResourceResolver(); versionResolver.setStrategyMap(Collections.singletonMap("/**", new ContentVersionStrategy())); PathResourceResolver pathResolver = new PathResourceResolver(); pathResolver.setAllowedLocations(new ClassPathResource("test/", getClass())); - List resolvers = Arrays.asList(versionResolver, pathResolver); + List resolvers = new ArrayList<>(); + resolvers.add(versionResolver); + resolvers.add(pathResolver); this.filter = new ResourceUrlEncodingFilter(); this.urlProvider = createResourceUrlProvider(resolvers); } + private ResourceUrlProvider createResourceUrlProvider(List resolvers) { + ResourceHttpRequestHandler handler = new ResourceHttpRequestHandler(); + handler.setLocations(Collections.singletonList(new ClassPathResource("test/", getClass()))); + handler.setResourceResolvers(resolvers); + + ResourceUrlProvider urlProvider = new ResourceUrlProvider(); + urlProvider.setHandlerMap(Collections.singletonMap("/resources/**", handler)); + return urlProvider; + } + + @Test public void encodeURL() throws Exception { - MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); - MockHttpServletResponse response = new MockHttpServletResponse(); - - this.filter.doFilter(request, response, (req, res) -> { - req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider); - String result = ((HttpServletResponse) res).encodeURL("/resources/bar.css"); - assertEquals("/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result); - }); + testEncodeUrl(new MockHttpServletRequest("GET", "/"), + "/resources/bar.css", "/resources/bar-11e16cf79faee7ac698c805cf28248d2.css"); } @Test - public void encodeURLWithContext() throws Exception { + public void encodeUrlWithContext() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/context/foo"); request.setContextPath("/context"); - MockHttpServletResponse response = new MockHttpServletResponse(); - this.filter.doFilter(request, response, (req, res) -> { - req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider); - String result = ((HttpServletResponse) res).encodeURL("/context/resources/bar.css"); - assertEquals("/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result); - }); + testEncodeUrl(request, "/context/resources/bar.css", + "/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css"); } @@ -82,9 +88,8 @@ public class ResourceUrlEncodingFilterTests { public void encodeUrlWithContextAndForwardedRequest() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/context/foo"); request.setContextPath("/context"); - MockHttpServletResponse response = new MockHttpServletResponse(); - this.filter.doFilter(request, response, (req, res) -> { + this.filter.doFilter(request, new MockHttpServletResponse(), (req, res) -> { req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider); request.setRequestURI("/forwarded"); request.setContextPath("/"); @@ -93,84 +98,71 @@ public class ResourceUrlEncodingFilterTests { }); } - // SPR-13757 - @Test + @Test // SPR-13757 public void encodeContextPathUrlWithoutSuffix() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/context"); request.setContextPath("/context"); - MockHttpServletResponse response = new MockHttpServletResponse(); - this.filter.doFilter(request, response, (req, res) -> { - req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider); - String result = ((HttpServletResponse) res).encodeURL("/context/resources/bar.css"); - assertEquals("/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result); - }); + testEncodeUrl(request, "/context/resources/bar.css", + "/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css"); } @Test public void encodeContextPathUrlWithSuffix() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/context/"); request.setContextPath("/context"); - MockHttpServletResponse response = new MockHttpServletResponse(); - this.filter.doFilter(request, response, (req, res) -> { - req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider); - String result = ((HttpServletResponse) res).encodeURL("/context/resources/bar.css"); - assertEquals("/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result); - }); + testEncodeUrl(request, "/context/resources/bar.css", + "/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css"); } - // SPR-13018 - @Test - public void encodeEmptyURLWithContext() throws Exception { + @Test // SPR-13018 + public void encodeEmptyUrlWithContext() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/context/foo"); request.setContextPath("/context"); - MockHttpServletResponse response = new MockHttpServletResponse(); - this.filter.doFilter(request, response, (req, res) -> { - req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider); - String result = ((HttpServletResponse) res).encodeURL("?foo=1"); - assertEquals("?foo=1", result); - }); + testEncodeUrl(request, "?foo=1", "?foo=1"); } - // SPR-13374 - @Test - public void encodeURLWithRequestParams() throws Exception { + @Test // SPR-13374 + public void encodeUrlWithRequestParams() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo"); request.setContextPath("/"); - MockHttpServletResponse response = new MockHttpServletResponse(); - this.filter.doFilter(request, response, (req, res) -> { - req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider); - String result = ((HttpServletResponse) res).encodeURL("/resources/bar.css?foo=bar&url=http://example.org"); - assertEquals("/resources/bar-11e16cf79faee7ac698c805cf28248d2.css?foo=bar&url=http://example.org", result); - }); + testEncodeUrl(request, "/resources/bar.css?foo=bar&url=http://example.org", + "/resources/bar-11e16cf79faee7ac698c805cf28248d2.css?foo=bar&url=http://example.org"); } - // SPR-13847 - @Test + @Test // SPR-13847 public void encodeUrlPreventStringOutOfBounds() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/context-path/index"); request.setContextPath("/context-path"); request.setServletPath(""); - MockHttpServletResponse response = new MockHttpServletResponse(); - this.filter.doFilter(request, response, (req, res) -> { - req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider); - String result = ((HttpServletResponse) res).encodeURL("index?key=value"); - assertEquals("index?key=value", result); - }); + testEncodeUrl(request, "index?key=value", "index?key=value"); + } + + @Test // gh-22552 + public void encodeUrlWithFragment() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo"); + request.setContextPath("/"); + + testEncodeUrl(request, "/resources/bar.css#something", + "/resources/bar-11e16cf79faee7ac698c805cf28248d2.css#something"); + + testEncodeUrl(request, + "/resources/bar.css?foo=bar&url=http://example.org#something", + "/resources/bar-11e16cf79faee7ac698c805cf28248d2.css?foo=bar&url=http://example.org#something"); } + private void testEncodeUrl(MockHttpServletRequest request, String url, String expected) + throws ServletException, IOException { - protected ResourceUrlProvider createResourceUrlProvider(List resolvers) { - ResourceHttpRequestHandler handler = new ResourceHttpRequestHandler(); - handler.setLocations(Arrays.asList(new ClassPathResource("test/", getClass()))); - handler.setResourceResolvers(resolvers); - ResourceUrlProvider urlProvider = new ResourceUrlProvider(); - urlProvider.setHandlerMap(Collections.singletonMap("/resources/**", handler)); - return urlProvider; + this.filter.doFilter(request, new MockHttpServletResponse(), (req, res) -> { + req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider); + String result = ((HttpServletResponse) res).encodeURL(url); + assertEquals(expected, result); + }); } }