diff --git a/build.gradle b/build.gradle index 310a9d4faf..ae73dc09a8 100644 --- a/build.gradle +++ b/build.gradle @@ -24,7 +24,7 @@ ext { groovyVersion = "2.5.7" hsqldbVersion = "2.5.0" jackson2Version = "2.9.9" - jettyVersion = "9.4.19.v20190610" + jettyVersion = "9.4.20.v20190813" junit5Version = "5.5.1" kotlinVersion = "1.3.50" log4jVersion = "2.12.0" diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractListenerReadPublisher.java b/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractListenerReadPublisher.java index 401e6c867a..b28b6e47a0 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractListenerReadPublisher.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractListenerReadPublisher.java @@ -224,6 +224,23 @@ public abstract class AbstractListenerReadPublisher implements Publisher { } } + private void handleCompletionOrErrorBeforeDemand() { + State state = this.state.get(); + if (!state.equals(State.UNSUBSCRIBED) && !state.equals(State.SUBSCRIBING)) { + if (this.completionBeforeDemand) { + rsReadLogger.trace(getLogPrefix() + "Completed before demand"); + this.state.get().onAllDataRead(this); + } + Throwable ex = this.errorBeforeDemand; + if (ex != null) { + if (rsReadLogger.isTraceEnabled()) { + rsReadLogger.trace(getLogPrefix() + "Completed with error before demand: " + ex); + } + this.state.get().onError(this, ex); + } + } + } + private Subscription createSubscription() { return new ReadSubscription(); } @@ -283,7 +300,7 @@ public abstract class AbstractListenerReadPublisher implements Publisher { publisher.subscriber = subscriber; subscriber.onSubscribe(subscription); publisher.changeState(SUBSCRIBING, NO_DEMAND); - handleCompletionOrErrorBeforeDemand(publisher); + publisher.handleCompletionOrErrorBeforeDemand(); } else { throw new IllegalStateException("Failed to transition to SUBSCRIBING, " + @@ -294,30 +311,13 @@ public abstract class AbstractListenerReadPublisher implements Publisher { @Override void onAllDataRead(AbstractListenerReadPublisher publisher) { publisher.completionBeforeDemand = true; - handleCompletionOrErrorBeforeDemand(publisher); + publisher.handleCompletionOrErrorBeforeDemand(); } @Override void onError(AbstractListenerReadPublisher publisher, Throwable ex) { publisher.errorBeforeDemand = ex; - handleCompletionOrErrorBeforeDemand(publisher); - } - - private void handleCompletionOrErrorBeforeDemand(AbstractListenerReadPublisher publisher) { - if (publisher.state.get().equals(NO_DEMAND)) { - if (publisher.completionBeforeDemand) { - rsReadLogger.trace(publisher.getLogPrefix() + "Completed before demand"); - publisher.state.get().onAllDataRead(publisher); - } - Throwable ex = publisher.errorBeforeDemand; - if (ex != null) { - if (rsReadLogger.isTraceEnabled()) { - String prefix = publisher.getLogPrefix(); - rsReadLogger.trace(prefix + "Completed with error before demand: " + ex); - } - publisher.state.get().onError(publisher, ex); - } - } + publisher.handleCompletionOrErrorBeforeDemand(); } }, @@ -337,11 +337,13 @@ public abstract class AbstractListenerReadPublisher implements Publisher { @Override void onAllDataRead(AbstractListenerReadPublisher publisher) { publisher.completionBeforeDemand = true; + publisher.handleCompletionOrErrorBeforeDemand(); } @Override void onError(AbstractListenerReadPublisher publisher, Throwable ex) { publisher.errorBeforeDemand = ex; + publisher.handleCompletionOrErrorBeforeDemand(); } }, 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 77b71215b4..c2a4486a3f 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 @@ -96,10 +96,7 @@ public class ResourceUrlEncodingFilter extends GenericFilterBean { String lookupPath = pathHelper.getLookupPathForRequest(this); this.indexLookupPath = requestUri.lastIndexOf(lookupPath); if (this.indexLookupPath == -1) { - throw new IllegalStateException( - "Failed to find lookupPath '" + lookupPath + "' within requestUri '" + requestUri + "'. " + - "Does the path have invalid encoded characters for characterEncoding '" + - getRequest().getCharacterEncoding() + "'?"); + throw new LookupPathIndexException(lookupPath, requestUri); } this.prefixLookupPath = requestUri.substring(0, this.indexLookupPath); if ("/".equals(lookupPath) && !"/".equals(requestUri)) { @@ -164,4 +161,14 @@ public class ResourceUrlEncodingFilter extends GenericFilterBean { } } + + @SuppressWarnings("serial") + static class LookupPathIndexException extends IllegalArgumentException { + + LookupPathIndexException(String lookupPath, String requestUri) { + super("Failed to find lookupPath '" + lookupPath + "' within requestUri '" + requestUri + "'. " + + "This could be because the path has invalid encoded characters or isn't normalized."); + } + } + } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlProviderExposingInterceptor.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlProviderExposingInterceptor.java index 9572843c43..49ec8e25d8 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlProviderExposingInterceptor.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlProviderExposingInterceptor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 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. @@ -20,6 +20,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.springframework.util.Assert; +import org.springframework.web.bind.ServletRequestBindingException; import org.springframework.web.servlet.handler.HandlerInterceptorAdapter; /** @@ -48,7 +49,12 @@ public class ResourceUrlProviderExposingInterceptor extends HandlerInterceptorAd public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception { - request.setAttribute(RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); + try { + request.setAttribute(RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); + } + catch (ResourceUrlEncodingFilter.LookupPathIndexException ex) { + throw new ServletRequestBindingException(ex.getMessage(), ex); + } return true; } 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 1da6808385..71ce5d50c9 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 @@ -19,7 +19,9 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import javax.servlet.FilterChain; import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.junit.jupiter.api.BeforeEach; @@ -28,8 +30,10 @@ import org.junit.jupiter.api.Test; import org.springframework.core.io.ClassPathResource; import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.mock.web.test.MockHttpServletResponse; +import org.springframework.web.bind.ServletRequestBindingException; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; /** * Unit tests for {@code ResourceUrlEncodingFilter}. @@ -155,14 +159,31 @@ public class ResourceUrlEncodingFilterTests { "/resources/bar-11e16cf79faee7ac698c805cf28248d2.css?foo=bar&url=https://example.org#something"); } - private void testEncodeUrl(MockHttpServletRequest request, String url, String expected) - throws ServletException, IOException { + @Test // gh-23508 + public void invalidLookupPath() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRequestURI("/a/b/../logo.png"); + request.setServletPath("/a/logo.png"); this.filter.doFilter(request, new MockHttpServletResponse(), (req, res) -> { - req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider); - String result = ((HttpServletResponse) res).encodeURL(url); - assertThat(result).isEqualTo(expected); + ResourceUrlProviderExposingInterceptor interceptor = + new ResourceUrlProviderExposingInterceptor(this.urlProvider); + + assertThatThrownBy(() -> interceptor.preHandle((HttpServletRequest) req, (HttpServletResponse) res, null)) + .isInstanceOf(ServletRequestBindingException.class); + }); } + private void testEncodeUrl(MockHttpServletRequest request, String url, String expected) + throws ServletException, IOException { + + FilterChain chain = (req, res) -> { + req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider); + String result = ((HttpServletResponse) res).encodeURL(url); + assertThat(result).isEqualTo(expected); + }; + this.filter.doFilter(request, new MockHttpServletResponse(), chain); + } + } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/server/jetty/JettyRequestUpgradeStrategy.java b/spring-websocket/src/main/java/org/springframework/web/socket/server/jetty/JettyRequestUpgradeStrategy.java index 0f89b01e0a..904b477aec 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/server/jetty/JettyRequestUpgradeStrategy.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/server/jetty/JettyRequestUpgradeStrategy.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. @@ -17,6 +17,7 @@ package org.springframework.web.socket.server.jetty; import java.io.IOException; +import java.lang.reflect.Method; import java.security.Principal; import java.util.ArrayList; import java.util.List; @@ -28,6 +29,7 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.websocket.api.WebSocketPolicy; import org.eclipse.jetty.websocket.api.extensions.ExtensionConfig; +import org.eclipse.jetty.websocket.api.extensions.ExtensionFactory; import org.eclipse.jetty.websocket.server.HandshakeRFC6455; import org.eclipse.jetty.websocket.server.WebSocketServerFactory; @@ -38,7 +40,9 @@ import org.springframework.http.server.ServerHttpResponse; import org.springframework.http.server.ServletServerHttpRequest; import org.springframework.http.server.ServletServerHttpResponse; import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; import org.springframework.util.CollectionUtils; +import org.springframework.util.ReflectionUtils; import org.springframework.web.context.ServletContextAware; import org.springframework.web.socket.WebSocketExtension; import org.springframework.web.socket.WebSocketHandler; @@ -167,7 +171,7 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Serv } private List buildWebSocketExtensions() { - Set names = this.factory.getExtensionFactory().getExtensionNames(); + Set names = getExtensionNames(); List result = new ArrayList<>(names.size()); for (String name : names) { result.add(new WebSocketExtension(name)); @@ -175,6 +179,18 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy, Serv return result; } + @SuppressWarnings({"unchecked"}) + private Set getExtensionNames() { + try { + return this.factory.getExtensionFactory().getExtensionNames(); + } + catch (IncompatibleClassChangeError ex) { + // 9.4.20.v20190813: ExtensionFactory (abstract class -> interface) + Method method = ClassUtils.getMethod(ExtensionFactory.class, "getExtensionNames"); + return (Set) ReflectionUtils.invokeMethod(method, this.factory.getExtensionFactory()); + } + } + @Override public void upgrade(ServerHttpRequest request, ServerHttpResponse response, String selectedProtocol, List selectedExtensions, Principal user,