From b5327ef60f7cdd3b82d00d36c0775ba0a324f7be Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 1 May 2019 02:59:19 -0400 Subject: [PATCH 1/2] Fix regression in ProducesRequestCondition Closes gh-22853 --- .../condition/ProducesRequestCondition.java | 5 +++- .../ProducesRequestConditionTests.java | 28 +++++++++++++++---- .../condition/ProducesRequestCondition.java | 7 +++-- .../ProducesRequestConditionTests.java | 22 +++++++++++++++ 4 files changed, 52 insertions(+), 10 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java index fd8c00790d..a176fd9dcb 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/condition/ProducesRequestCondition.java @@ -185,9 +185,12 @@ public final class ProducesRequestCondition extends AbstractRequestCondition result = new LinkedHashSet<>(this.expressions); result.removeIf(expression -> !expression.match(exchange)); if (!result.isEmpty()) { diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/ProducesRequestConditionTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/ProducesRequestConditionTests.java index addc705829..3e6f222cd4 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/ProducesRequestConditionTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/condition/ProducesRequestConditionTests.java @@ -21,15 +21,14 @@ import java.util.Collections; import org.junit.Test; +import org.springframework.http.MediaType; import org.springframework.mock.web.test.server.MockServerWebExchange; +import org.springframework.web.reactive.accept.RequestedContentTypeResolver; +import org.springframework.web.reactive.accept.RequestedContentTypeResolverBuilder; import org.springframework.web.server.ServerWebExchange; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; -import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.get; +import static org.junit.Assert.*; +import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.*; /** * Unit tests for {@link ProducesRequestCondition}. @@ -110,6 +109,23 @@ public class ProducesRequestConditionTests { assertNotNull(condition.getMatchingCondition(exchange)); } + @Test // gh-22853 + public void matchAndCompare() { + RequestedContentTypeResolverBuilder builder = new RequestedContentTypeResolverBuilder(); + builder.headerResolver(); + builder.fixedResolver(MediaType.TEXT_HTML); + RequestedContentTypeResolver resolver = builder.build(); + + ProducesRequestCondition none = new ProducesRequestCondition(new String[0], null, resolver); + ProducesRequestCondition html = new ProducesRequestCondition(new String[] {"text/html"}, null, resolver); + + MockServerWebExchange exchange = MockServerWebExchange.from(get("/").header("Accept", "*/*")); + + ProducesRequestCondition noneMatch = none.getMatchingCondition(exchange); + ProducesRequestCondition htmlMatch = html.getMatchingCondition(exchange); + + assertEquals(1, noneMatch.compareTo(htmlMatch, exchange)); + } @Test public void compareTo() { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ProducesRequestCondition.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ProducesRequestCondition.java index eb1a4c2075..33375cdb52 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ProducesRequestCondition.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ProducesRequestCondition.java @@ -185,10 +185,12 @@ public final class ProducesRequestCondition extends AbstractRequestCondition acceptedMediaTypes; try { acceptedMediaTypes = getAcceptedMediaTypes(request); @@ -196,7 +198,6 @@ public final class ProducesRequestCondition extends AbstractRequestCondition result = new LinkedHashSet<>(this.expressions); result.removeIf(expression -> !expression.match(acceptedMediaTypes)); if (!result.isEmpty()) { diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ProducesRequestConditionTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ProducesRequestConditionTests.java index 6556fb7a49..1bcf0d9e41 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ProducesRequestConditionTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/condition/ProducesRequestConditionTests.java @@ -21,7 +21,11 @@ import java.util.Collections; import org.junit.Test; +import org.springframework.http.MediaType; import org.springframework.mock.web.test.MockHttpServletRequest; +import org.springframework.web.accept.ContentNegotiationManager; +import org.springframework.web.accept.FixedContentNegotiationStrategy; +import org.springframework.web.accept.HeaderContentNegotiationStrategy; import org.springframework.web.servlet.mvc.condition.ProducesRequestCondition.ProduceMediaTypeExpression; import static org.junit.Assert.*; @@ -139,6 +143,24 @@ public class ProducesRequestConditionTests { assertNotNull(condition.getMatchingCondition(request)); } + @Test // gh-22853 + public void matchAndCompare() { + ContentNegotiationManager manager = new ContentNegotiationManager( + new HeaderContentNegotiationStrategy(), + new FixedContentNegotiationStrategy(MediaType.TEXT_HTML)); + + ProducesRequestCondition none = new ProducesRequestCondition(new String[0], null, manager); + ProducesRequestCondition html = new ProducesRequestCondition(new String[] {"text/html"}, null, manager); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addHeader("Accept", "*/*"); + + ProducesRequestCondition noneMatch = none.getMatchingCondition(request); + ProducesRequestCondition htmlMatch = html.getMatchingCondition(request); + + assertEquals(1, noneMatch.compareTo(htmlMatch, request)); + } + @Test public void compareTo() { ProducesRequestCondition html = new ProducesRequestCondition("text/html"); From b3982161915490a3cee7022e0905705abe70bc13 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 1 May 2019 03:57:44 -0400 Subject: [PATCH 2/2] Fail proactively if lookupPath not in requestUri Closes gh-22851 --- .../web/servlet/resource/ResourceUrlEncodingFilter.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 e38ddabd1f..96578d5198 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. @@ -30,6 +30,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.lang.Nullable; +import org.springframework.util.Assert; import org.springframework.web.filter.GenericFilterBean; import org.springframework.web.util.UrlPathHelper; @@ -96,6 +97,10 @@ public class ResourceUrlEncodingFilter extends GenericFilterBean { String requestUri = pathHelper.getRequestUri(this); String lookupPath = pathHelper.getLookupPathForRequest(this); this.indexLookupPath = requestUri.lastIndexOf(lookupPath); + Assert.isTrue(this.indexLookupPath != -1, () -> + "Failed to find lookupPath '" + lookupPath + "' within requestUri '" + requestUri + ". " + + "Does the path have invalid encoded characters " + + "for characterEncoding=" + getRequest().getCharacterEncoding() + "?"); this.prefixLookupPath = requestUri.substring(0, this.indexLookupPath); if ("/".equals(lookupPath) && !"/".equals(requestUri)) { String contextPath = pathHelper.getContextPath(this);