Merge branch '5.1.x'

spring-web/src/main/java/org/springframework/http/server/PathContainer.java

@@ -36,7 +36,7 @@ import org.springframework.util.MultiValueMap;
 public interface PathContainer {
 
 	/**
-	 * The original path that was parsed.
+	 * The original (raw, encoded) path that this instance was parsed from.
 	 */
 	String value();
 
@@ -83,7 +83,7 @@ public interface PathContainer {
 	interface Element {
 
 		/**
-		 * Return the original, raw (encoded) value for the path component.
+		 * Return the original (raw, encoded) value of this path element.
 		 */
 		String value();
 	}

spring-webflux/src/main/java/org/springframework/web/reactive/resource/PathResourceResolver.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.
@@ -19,6 +19,7 @@ package org.springframework.web.reactive.resource;
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.net.URLDecoder;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.List;
 
@@ -31,6 +32,7 @@ import org.springframework.core.io.UrlResource;
 import org.springframework.lang.Nullable;
 import org.springframework.util.StringUtils;
 import org.springframework.web.server.ServerWebExchange;
+import org.springframework.web.util.UriUtils;
 
 /**
  * A simple {@code ResourceResolver} that tries to find a resource under the given
@@ -108,6 +110,9 @@ public class PathResourceResolver extends AbstractResourceResolver {
 	 */
 	protected Mono<Resource> getResource(String resourcePath, Resource location) {
 		try {
+			if (location instanceof ClassPathResource) {
+				resourcePath = UriUtils.decode(resourcePath, StandardCharsets.UTF_8);
+			}
 			Resource resource = location.createRelative(resourcePath);
 			if (resource.isReadable()) {
 				if (checkResource(resource, location)) {

spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceResolver.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.
@@ -21,6 +21,7 @@ import java.util.List;
 import reactor.core.publisher.Mono;
 
 import org.springframework.core.io.Resource;
+import org.springframework.http.server.RequestPath;
 import org.springframework.lang.Nullable;
 import org.springframework.web.server.ServerWebExchange;
 
@@ -40,7 +41,8 @@ public interface ResourceResolver {
 	 * Resolve the supplied request and request path to a {@link Resource} that
 	 * exists under one of the given resource locations.
 	 * @param exchange the current exchange
-	 * @param requestPath the portion of the request path to use
+	 * @param requestPath the portion of the request path to use. This is
+	 * expected to be the encoded path, i.e. {@link RequestPath#value()}.
 	 * @param locations the locations to search in when looking up resources
 	 * @param chain the chain of remaining resolvers to delegate to
 	 * @return the resolved resource or an empty {@code Mono} if unresolved
@@ -53,7 +55,8 @@ public interface ResourceResolver {
 	 * to access the resource that is located at the given <em>internal</em>
 	 * resource path.
 	 * <p>This is useful when rendering URL links to clients.
-	 * @param resourcePath the internal resource path
+	 * @param resourcePath the "internal" resource path to resolve a path for
+	 * public use. This is expected to be the encoded path.
 	 * @param locations the locations to search in when looking up resources
 	 * @param chain the chain of resolvers to delegate to
 	 * @return the resolved public URL path or an empty {@code Mono} if unresolved

spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractMessageReaderArgumentResolver.java

@@ -35,6 +35,7 @@ import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.core.codec.DecodingException;
 import org.springframework.core.codec.Hints;
 import org.springframework.core.io.buffer.DataBuffer;
+import org.springframework.core.io.buffer.DataBufferUtils;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.MediaType;
 import org.springframework.http.codec.HttpMessageReader;
@@ -205,7 +206,8 @@ public abstract class AbstractMessageReaderArgumentResolver extends HandlerMetho
 
 		HttpMethod method = request.getMethod();
 		if (contentType == null && method != null && SUPPORTED_METHODS.contains(method)) {
-			Flux<DataBuffer> body = request.getBody().doOnNext(o -> {
+			Flux<DataBuffer> body = request.getBody().doOnNext(buffer -> {
+				DataBufferUtils.release(buffer);
 				// Body not empty, back to 415..
 				throw new UnsupportedMediaTypeStatusException(mediaType, this.supportedMediaTypes, elementType);
 			});

spring-webflux/src/main/java/org/springframework/web/reactive/result/view/freemarker/FreeMarkerView.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 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.
@@ -32,7 +32,6 @@ import freemarker.template.ObjectWrapper;
 import freemarker.template.SimpleHash;
 import freemarker.template.Template;
 import freemarker.template.Version;
-import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 
 import org.springframework.beans.BeansException;
@@ -42,6 +41,7 @@ import org.springframework.context.ApplicationContextException;
 import org.springframework.context.i18n.LocaleContextHolder;
 import org.springframework.core.io.buffer.DataBuffer;
 import org.springframework.core.io.buffer.DataBufferUtils;
+import org.springframework.core.io.buffer.PooledDataBuffer;
 import org.springframework.http.MediaType;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
@@ -184,30 +184,34 @@ public class FreeMarkerView extends AbstractUrlBasedView {
 	protected Mono<Void> renderInternal(Map<String, Object> renderAttributes,
 			@Nullable MediaType contentType, ServerWebExchange exchange) {
 
-		// Expose all standard FreeMarker hash models.
+		return exchange.getResponse().writeWith(Mono
-		SimpleHash freeMarkerModel = getTemplateModel(renderAttributes, exchange);
+				.fromCallable(() -> {
+					// Expose all standard FreeMarker hash models.
+					SimpleHash freeMarkerModel = getTemplateModel(renderAttributes, exchange);
 
-		if (logger.isDebugEnabled()) {
+					if (logger.isDebugEnabled()) {
-			logger.debug(exchange.getLogPrefix() + "Rendering [" + getUrl() + "]");
+						logger.debug(exchange.getLogPrefix() + "Rendering [" + getUrl() + "]");
-		}
+					}
 
-		Locale locale = LocaleContextHolder.getLocale(exchange.getLocaleContext());
+					Locale locale = LocaleContextHolder.getLocale(exchange.getLocaleContext());
-		DataBuffer dataBuffer = exchange.getResponse().bufferFactory().allocateBuffer();
+					DataBuffer dataBuffer = exchange.getResponse().bufferFactory().allocateBuffer();
-		try {
+					try {
-			Charset charset = getCharset(contentType);
+						Charset charset = getCharset(contentType);
-			Writer writer = new OutputStreamWriter(dataBuffer.asOutputStream(), charset);
+						Writer writer = new OutputStreamWriter(dataBuffer.asOutputStream(), charset);
-			getTemplate(locale).process(freeMarkerModel, writer);
+						getTemplate(locale).process(freeMarkerModel, writer);
-		}
+						return dataBuffer;
-		catch (IOException ex) {
+					}
-			DataBufferUtils.release(dataBuffer);
+					catch (IOException ex) {
-			String message = "Could not load FreeMarker template for URL [" + getUrl() + "]";
+						DataBufferUtils.release(dataBuffer);
-			return Mono.error(new IllegalStateException(message, ex));
+						String message = "Could not load FreeMarker template for URL [" + getUrl() + "]";
-		}
+						throw new IllegalStateException(message, ex);
-		catch (Throwable ex) {
+					}
-			DataBufferUtils.release(dataBuffer);
+					catch (Throwable ex) {
-			return Mono.error(ex);
+						DataBufferUtils.release(dataBuffer);
-		}
+						throw ex;
-		return exchange.getResponse().writeWith(Flux.just(dataBuffer));
+					}
+				})
+				.doOnDiscard(PooledDataBuffer.class, DataBufferUtils::release));
 	}
 
 	private Charset getCharset(@Nullable MediaType mediaType) {

spring-webflux/src/main/java/org/springframework/web/reactive/result/view/script/ScriptTemplateView.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.
@@ -37,9 +37,7 @@ import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextException;
 import org.springframework.context.i18n.LocaleContextHolder;
 import org.springframework.core.io.Resource;
-import org.springframework.core.io.buffer.DataBuffer;
 import org.springframework.http.MediaType;
-import org.springframework.http.server.reactive.ServerHttpResponse;
 import org.springframework.lang.Nullable;
 import org.springframework.scripting.support.StandardScriptEvalException;
 import org.springframework.scripting.support.StandardScriptUtils;
@@ -301,8 +299,7 @@ public class ScriptTemplateView extends AbstractUrlBasedView {
 	protected Mono<Void> renderInternal(
 			Map<String, Object> model, @Nullable MediaType contentType, ServerWebExchange exchange) {
 
-		return Mono.defer(() -> {
+		return exchange.getResponse().writeWith(Mono.fromCallable(() -> {
-			ServerHttpResponse response = exchange.getResponse();
 			try {
 				ScriptEngine engine = getEngine();
 				String url = getUrl();
@@ -338,8 +335,7 @@ public class ScriptTemplateView extends AbstractUrlBasedView {
 				}
 
 				byte[] bytes = String.valueOf(html).getBytes(StandardCharsets.UTF_8);
-				DataBuffer buffer = response.bufferFactory().allocateBuffer(bytes.length).write(bytes);
+				return exchange.getResponse().bufferFactory().wrap(bytes); // just wrapping, no allocation
-				return response.writeWith(Mono.just(buffer));
 			}
 			catch (ScriptException ex) {
 				throw new IllegalStateException("Failed to render script template", new StandardScriptEvalException(ex));
@@ -347,7 +343,7 @@ public class ScriptTemplateView extends AbstractUrlBasedView {
 			catch (Exception ex) {
 				throw new IllegalStateException("Failed to render script template", ex);
 			}
-		});
+		}));
 	}
 
 	protected String getTemplate(String path) throws IOException {

spring-webflux/src/test/java/org/springframework/web/reactive/resource/PathResourceResolverTests.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.
@@ -22,6 +22,7 @@ import java.util.List;
 import org.junit.Test;
 
 import org.springframework.core.io.ClassPathResource;
+import org.springframework.core.io.FileUrlResource;
 import org.springframework.core.io.Resource;
 import org.springframework.core.io.UrlResource;
 
@@ -64,6 +65,22 @@ public class PathResourceResolverTests {
 		assertNotNull(actual);
 	}
 
+	@Test // gh-22272
+	public void resolveWithEncodedPath() throws IOException {
+		Resource classpathLocation = new ClassPathResource("test/", PathResourceResolver.class);
+		testWithEncodedPath(classpathLocation);
+		testWithEncodedPath(new FileUrlResource(classpathLocation.getURL()));
+	}
+
+	private void testWithEncodedPath(Resource location) throws IOException {
+		String path = "foo%20foo.txt";
+		List<Resource> locations = singletonList(location);
+		Resource actual = this.resolver.resolveResource(null, path, locations, null).block(TIMEOUT);
+
+		assertNotNull(actual);
+		assertEquals("foo foo.txt", actual.getFile().getName());
+	}
+
 	@Test
 	public void checkResource() throws IOException {
 		Resource location = new ClassPathResource("test/", PathResourceResolver.class);

spring-webflux/src/test/java/org/springframework/web/reactive/result/view/ZeroDemandResponse.java

@@ -0,0 +1,129 @@
+/*
+ * 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.web.reactive.result.view;
+
+import java.util.function.Supplier;
+
+import io.netty.buffer.PooledByteBufAllocator;
+import org.reactivestreams.Publisher;
+import org.reactivestreams.Subscription;
+import reactor.core.publisher.BaseSubscriber;
+import reactor.core.publisher.Mono;
+
+import org.springframework.core.io.buffer.DataBuffer;
+import org.springframework.core.io.buffer.DataBufferFactory;
+import org.springframework.core.io.buffer.LeakAwareDataBufferFactory;
+import org.springframework.core.io.buffer.NettyDataBufferFactory;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseCookie;
+import org.springframework.http.server.reactive.ServerHttpResponse;
+import org.springframework.util.MultiValueMap;
+
+/**
+ * Response that subscribes to the writes source but never posts demand and also
+ * offers method to then cancel the subscription, and check of leaks in the end.
+ *
+ * @author Rossen Stoyanchev
+ */
+public class ZeroDemandResponse implements ServerHttpResponse {
+
+	private final LeakAwareDataBufferFactory bufferFactory;
+
+	private final ZeroDemandSubscriber writeSubscriber = new ZeroDemandSubscriber();
+
+
+	public ZeroDemandResponse() {
+		NettyDataBufferFactory delegate = new NettyDataBufferFactory(PooledByteBufAllocator.DEFAULT);
+		this.bufferFactory = new LeakAwareDataBufferFactory(delegate);
+	}
+
+
+	public void checkForLeaks() {
+		this.bufferFactory.checkForLeaks();
+	}
+
+	public void cancelWrite() {
+		this.writeSubscriber.cancel();
+	}
+
+
+	@Override
+	public DataBufferFactory bufferFactory() {
+		return this.bufferFactory;
+	}
+
+	@Override
+	public Mono<Void> writeWith(Publisher<? extends DataBuffer> body) {
+		body.subscribe(this.writeSubscriber);
+		return Mono.never();
+	}
+
+
+	@Override
+	public boolean setStatusCode(HttpStatus status) {
+		throw new UnsupportedOperationException();
+	}
+
+	@Override
+	public HttpStatus getStatusCode() {
+		throw new UnsupportedOperationException();
+	}
+
+	@Override
+	public MultiValueMap<String, ResponseCookie> getCookies() {
+		throw new UnsupportedOperationException();
+	}
+
+	@Override
+	public void addCookie(ResponseCookie cookie) {
+		throw new UnsupportedOperationException();
+	}
+
+	@Override
+	public void beforeCommit(Supplier<? extends Mono<Void>> action) {
+		throw new UnsupportedOperationException();
+	}
+
+	@Override
+	public boolean isCommitted() {
+		throw new UnsupportedOperationException();
+	}
+
+	@Override
+	public Mono<Void> writeAndFlushWith(Publisher<? extends Publisher<? extends DataBuffer>> body) {
+		throw new UnsupportedOperationException();
+	}
+
+	@Override
+	public Mono<Void> setComplete() {
+		throw new UnsupportedOperationException();
+	}
+
+	@Override
+	public HttpHeaders getHeaders() {
+		throw new UnsupportedOperationException();
+	}
+
+
+	private static class ZeroDemandSubscriber extends BaseSubscriber<DataBuffer> {
+
+		@Override
+		protected void hookOnSubscribe(Subscription subscription) {
+			// Just subscribe without requesting
+		}
+	}
+}

spring-webflux/src/test/java/org/springframework/web/reactive/result/view/freemarker/FreeMarkerViewTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 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.
@@ -31,20 +31,26 @@ import reactor.test.StepVerifier;
 import org.springframework.context.ApplicationContextException;
 import org.springframework.context.support.GenericApplicationContext;
 import org.springframework.core.io.buffer.DataBuffer;
+import org.springframework.http.codec.ServerCodecConfigurer;
 import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
 import org.springframework.mock.web.test.server.MockServerWebExchange;
 import org.springframework.ui.ExtendedModelMap;
 import org.springframework.ui.ModelMap;
+import org.springframework.web.reactive.result.view.ZeroDemandResponse;
+import org.springframework.web.server.ServerWebExchange;
+import org.springframework.web.server.adapter.DefaultServerWebExchange;
+import org.springframework.web.server.i18n.AcceptHeaderLocaleContextResolver;
+import org.springframework.web.server.session.DefaultWebSessionManager;
 
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.*;
-import static org.junit.Assert.assertTrue;
 
 /**
  * @author Rossen Stoyanchev
  */
 public class FreeMarkerViewTests {
 
-	private static final String TEMPLATE_PATH = "classpath*:org/springframework/web/reactive/view/freemarker/";
+	private static final String TEMPLATE_PATH =
+			"classpath*:org/springframework/web/reactive/view/freemarker/";
 
 
 	private final MockServerWebExchange exchange =
@@ -101,7 +107,7 @@ public class FreeMarkerViewTests {
 	}
 
 	@Test
-	public void render() throws Exception {
+	public void render() {
 		FreeMarkerView view = new FreeMarkerView();
 		view.setConfiguration(this.freeMarkerConfig);
 		view.setUrl("test.ftl");
@@ -116,6 +122,26 @@ public class FreeMarkerViewTests {
 				.verify();
 	}
 
+	@Test // gh-22754
+	public void subscribeWithoutDemand() {
+		ZeroDemandResponse response = new ZeroDemandResponse();
+		ServerWebExchange exchange = new DefaultServerWebExchange(
+				MockServerHttpRequest.get("/path").build(), response,
+				new DefaultWebSessionManager(), ServerCodecConfigurer.create(),
+				new AcceptHeaderLocaleContextResolver());
+
+		FreeMarkerView view = new FreeMarkerView();
+		view.setConfiguration(this.freeMarkerConfig);
+		view.setUrl("test.ftl");
+
+		ModelMap model = new ExtendedModelMap();
+		model.addAttribute("hello", "hi FreeMarker");
+		view.render(model, null, exchange).subscribe();
+
+		response.cancelWrite();
+		response.checkForLeaks();
+	}
+
 
 	private static String asString(DataBuffer dataBuffer) {
 		ByteBuffer byteBuffer = dataBuffer.asByteBuffer();

spring-webflux/src/test/java/org/springframework/web/reactive/result/view/script/NashornScriptTemplateTests.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.
@@ -25,9 +25,15 @@ import org.springframework.context.annotation.AnnotationConfigApplicationContext
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.http.MediaType;
+import org.springframework.http.codec.ServerCodecConfigurer;
 import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
 import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse;
 import org.springframework.mock.web.test.server.MockServerWebExchange;
+import org.springframework.web.reactive.result.view.ZeroDemandResponse;
+import org.springframework.web.server.ServerWebExchange;
+import org.springframework.web.server.adapter.DefaultServerWebExchange;
+import org.springframework.web.server.i18n.AcceptHeaderLocaleContextResolver;
+import org.springframework.web.server.session.DefaultWebSessionManager;
 
 import static org.junit.Assert.assertEquals;
 
@@ -58,6 +64,25 @@ public class NashornScriptTemplateTests {
 				response.getBodyAsString().block());
 	}
 
+	@Test // gh-22754
+	public void subscribeWithoutDemand() throws Exception {
+		ZeroDemandResponse response = new ZeroDemandResponse();
+		ServerWebExchange exchange = new DefaultServerWebExchange(
+				MockServerHttpRequest.get("/path").build(), response,
+				new DefaultWebSessionManager(), ServerCodecConfigurer.create(),
+				new AcceptHeaderLocaleContextResolver());
+
+		Map<String, Object> model = new HashMap<>();
+		model.put("title", "Layout example");
+		model.put("body", "This is the body");
+		String viewUrl = "org/springframework/web/reactive/result/view/script/nashorn/template.html";
+		ScriptTemplateView view = createViewWithUrl(viewUrl, ScriptTemplatingConfiguration.class);
+		view.render(model, null, exchange).subscribe();
+
+		response.cancelWrite();
+		response.checkForLeaks();
+	}
+
 	private MockServerHttpResponse render(String viewUrl, Map<String, Object> model,
 			Class<?> configuration) throws Exception {
 

spring-webflux/src/test/resources/org/springframework/web/reactive/resource/test/foo foo.txt

@@ -0,0 +1 @@
+Also some text.