Remove framgent in ResourceUrlEncodingFilter

Closes gh-22552

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;
 		}
 	}
 

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<ResourceResolver> resolvers = Arrays.asList(versionResolver, pathResolver);
+		List<ResourceResolver> resolvers = new ArrayList<>();
+		resolvers.add(versionResolver);
+		resolvers.add(pathResolver);
 
 		this.filter = new ResourceUrlEncodingFilter();
 		this.urlProvider = createResourceUrlProvider(resolvers);
 	}
 
+	private ResourceUrlProvider createResourceUrlProvider(List<ResourceResolver> 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<ResourceResolver> 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);
+		});
 	}
 
 }