From 828c5b49076868d78930f6d8ac0c440d6de74c15 Mon Sep 17 00:00:00 2001 From: Ondrej Kraus Date: Fri, 23 Nov 2018 18:34:37 +0100 Subject: [PATCH] Sanitize request fragment in ResourceUrlEncodingFilter Prior to this change, ResourceUrlEncodingFilter would try to resolve the resource path using request URL without removing fragment first, whereas only paths should be used. This commit synchronizes behavior of ResourceUrlEncodingFilter with behavior of ResourceUrlProvider. Issue: SPR-17535 --- .../resource/ResourceUrlEncodingFilter.java | 16 +++++++++--- .../ResourceUrlEncodingFilterTests.java | 26 +++++++++++++++++++ 2 files changed, 38 insertions(+), 4 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 e50a48e8120d..f36ac7b649d6 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 @@ -115,7 +115,7 @@ public String resolveUrlPath(String url) { 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); @@ -126,9 +126,17 @@ public String resolveUrlPath(String url) { return null; } - private int getQueryParamsIndex(String url) { - int index = url.indexOf('?'); - return (index > 0 ? index : url.length()); + private int getEndPathIndex(String lookupPath) { + int suffixIndex = lookupPath.length(); + int queryIndex = lookupPath.indexOf('?'); + if (queryIndex > 0) { + suffixIndex = queryIndex; + } + int hashIndex = lookupPath.indexOf('#'); + if (hashIndex > 0) { + suffixIndex = Math.min(suffixIndex, hashIndex); + } + return suffixIndex; } } 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 b704445c42a6..c24b830bc31b 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 @@ -173,4 +173,30 @@ public void encodeUrlPreventStringOutOfBounds() throws Exception { }); } + @Test // SPR-17535 + public void encodeURLWitFragment() 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#something"); + assertEquals("/resources/bar-11e16cf79faee7ac698c805cf28248d2.css#something", result); + }); + } + + @Test // SPR-13374 and SPR-17535 combined + public void encodeURLWitFragmentAndRequestParams() 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#something"); + assertEquals("/resources/bar-11e16cf79faee7ac698c805cf28248d2.css?foo=bar&url=http://example.org#something", result); + }); + } + }