From b86c11cc9b00693fc3446724ffd37ae379189b7f Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 21 Aug 2019 02:28:19 +0300 Subject: [PATCH 1/2] Respect existing content-length for HTTP HEAD Closes gh-23484 --- .../reactive/HttpHeadResponseDecorator.java | 9 ++- .../HttpHeadResponseDecoratorTests.java | 73 +++++++++++++++++++ 2 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 spring-web/src/test/java/org/springframework/http/server/reactive/HttpHeadResponseDecoratorTests.java diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/HttpHeadResponseDecorator.java b/spring-web/src/main/java/org/springframework/http/server/reactive/HttpHeadResponseDecorator.java index 4c8d0b0f06c..d387abd329b 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/HttpHeadResponseDecorator.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/HttpHeadResponseDecorator.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. @@ -24,6 +24,7 @@ import reactor.core.publisher.Mono; import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferUtils; +import org.springframework.http.HttpHeaders; /** * {@link ServerHttpResponse} decorator for HTTP HEAD requests. @@ -52,7 +53,11 @@ public class HttpHeadResponseDecorator extends ServerHttpResponseDecorator { DataBufferUtils.release(buffer); return next; }) - .doOnNext(count -> getHeaders().setContentLength(count)) + .doOnNext(length -> { + if (length > 0 || getHeaders().getFirst(HttpHeaders.CONTENT_LENGTH) == null) { + getHeaders().setContentLength(length); + } + }) .then(); } diff --git a/spring-web/src/test/java/org/springframework/http/server/reactive/HttpHeadResponseDecoratorTests.java b/spring-web/src/test/java/org/springframework/http/server/reactive/HttpHeadResponseDecoratorTests.java new file mode 100644 index 00000000000..8231bc4789c --- /dev/null +++ b/spring-web/src/test/java/org/springframework/http/server/reactive/HttpHeadResponseDecoratorTests.java @@ -0,0 +1,73 @@ +/* + * 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.http.server.reactive; + +import java.nio.charset.StandardCharsets; + +import io.netty.buffer.PooledByteBufAllocator; +import org.junit.After; +import org.junit.Test; +import reactor.core.publisher.Flux; + +import org.springframework.core.io.buffer.DataBuffer; +import org.springframework.core.io.buffer.LeakAwareDataBufferFactory; +import org.springframework.core.io.buffer.NettyDataBufferFactory; +import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse; + +import static org.junit.Assert.assertEquals; + +/** + * Unit tests for {@link HttpHeadResponseDecorator}. + * @author Rossen Stoyanchev + */ +public class HttpHeadResponseDecoratorTests { + + private final LeakAwareDataBufferFactory bufferFactory = + new LeakAwareDataBufferFactory(new NettyDataBufferFactory(PooledByteBufAllocator.DEFAULT)); + + private final ServerHttpResponse response = + new HttpHeadResponseDecorator(new MockServerHttpResponse(this.bufferFactory)); + + + @After + public void tearDown() { + this.bufferFactory.checkForLeaks(); + } + + + @Test + public void write() { + Flux body = Flux.just(toDataBuffer("data1"), toDataBuffer("data2")); + response.writeWith(body).block(); + assertEquals(10, response.getHeaders().getContentLength()); + } + + @Test // gh-23484 + public void writeWithGivenContentLength() { + int length = 15; + this.response.getHeaders().setContentLength(length); + this.response.writeWith(Flux.empty()).block(); + assertEquals(length, this.response.getHeaders().getContentLength()); + } + + + private DataBuffer toDataBuffer(String s) { + DataBuffer buffer = this.bufferFactory.allocateBuffer(); + buffer.write(s.getBytes(StandardCharsets.UTF_8)); + return buffer; + } + +} From 6d8bf3466caf01841a7c9155b65e8d3ab65d26a0 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 21 Aug 2019 10:02:15 +0300 Subject: [PATCH 2/2] Suppress decoding error for resource path Closes gh-23463 --- .../resource/PathResourceResolver.java | 3 +++ .../reactive/resource/ResourceWebHandler.java | 7 +++++-- .../resource/PathResourceResolverTests.java | 7 +++++++ .../web/reactive/resource/test/test%file.txt | 0 .../resource/PathResourceResolver.java | 5 ++++- .../resource/ResourceHttpRequestHandler.java | 7 +++++-- .../resource/PathResourceResolverTests.java | 21 +++++++++++-------- .../web/servlet/resource/test/test%file.txt | 0 8 files changed, 36 insertions(+), 14 deletions(-) create mode 100644 spring-webflux/src/test/resources/org/springframework/web/reactive/resource/test/test%file.txt create mode 100644 spring-webmvc/src/test/resources/org/springframework/web/servlet/resource/test/test%file.txt diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/PathResourceResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/PathResourceResolver.java index 17cda85be2a..beafe65d0d2 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/PathResourceResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/PathResourceResolver.java @@ -203,6 +203,9 @@ public class PathResourceResolver extends AbstractResourceResolver { return true; } } + catch (IllegalArgumentException ex) { + // May not be possible to decode... + } catch (UnsupportedEncodingException ex) { // Should never happen... } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java index 2735663812a..87d59e7cd7b 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.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. @@ -468,7 +468,10 @@ public class ResourceWebHandler implements WebHandler, InitializingBean { return true; } } - catch (IllegalArgumentException | UnsupportedEncodingException ex) { + catch (IllegalArgumentException ex) { + // May not be possible to decode... + } + catch (UnsupportedEncodingException ex) { // Should never happen... } } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/PathResourceResolverTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/PathResourceResolverTests.java index 4afd4c07659..bf2aff79673 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/PathResourceResolverTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/PathResourceResolverTests.java @@ -109,6 +109,13 @@ public class PathResourceResolverTests { assertNull(actual); } + @Test // gh-23463 + public void ignoreInvalidEscapeSequence() throws IOException { + UrlResource location = new UrlResource(getClass().getResource("./test/")); + Resource resource = location.createRelative("test%file.txt"); + assertTrue(this.resolver.checkResource(resource, location)); + } + @Test public void checkResourceWithAllowedLocations() { this.resolver.setAllowedLocations( diff --git a/spring-webflux/src/test/resources/org/springframework/web/reactive/resource/test/test%file.txt b/spring-webflux/src/test/resources/org/springframework/web/reactive/resource/test/test%file.txt new file mode 100644 index 00000000000..e69de29bb2d diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PathResourceResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PathResourceResolver.java index db8e7c3c28a..b1ebfe94237 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PathResourceResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/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. @@ -288,6 +288,9 @@ public class PathResourceResolver extends AbstractResourceResolver { return true; } } + catch (IllegalArgumentException ex) { + // May not be possible to decode... + } catch (UnsupportedEncodingException ex) { // Should never happen... } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java index 587cffeb74d..4ef71cc5bfb 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.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. @@ -605,7 +605,10 @@ public class ResourceHttpRequestHandler extends WebContentGenerator return true; } } - catch (IllegalArgumentException | UnsupportedEncodingException ex) { + catch (IllegalArgumentException ex) { + // May not be possible to decode... + } + catch (UnsupportedEncodingException ex) { // Should never happen... } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/PathResourceResolverTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/PathResourceResolverTests.java index 61958655bbc..689bcda1e10 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/PathResourceResolverTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/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. @@ -90,6 +90,13 @@ public class PathResourceResolverTests { assertNull(actual); } + @Test // gh-23463 + public void ignoreInvalidEscapeSequence() throws IOException { + UrlResource location = new UrlResource(getClass().getResource("./test/")); + Resource resource = location.createRelative("test%file.txt"); + assertTrue(this.resolver.checkResource(resource, location)); + } + @Test public void checkResourceWithAllowedLocations() { this.resolver.setAllowedLocations( @@ -103,8 +110,7 @@ public class PathResourceResolverTests { assertEquals("../testalternatepath/bar.css", actual); } - // SPR-12432 - @Test + @Test // SPR-12432 public void checkServletContextResource() throws Exception { Resource classpathLocation = new ClassPathResource("test/", PathResourceResolver.class); MockServletContext context = new MockServletContext(); @@ -116,8 +122,7 @@ public class PathResourceResolverTests { assertTrue(this.resolver.checkResource(resource, servletContextLocation)); } - // SPR-12624 - @Test + @Test // SPR-12624 public void checkRelativeLocation() throws Exception { String locationUrl= new UrlResource(getClass().getResource("./test/")).getURL().toExternalForm(); Resource location = new UrlResource(locationUrl.replace("/springframework","/../org/springframework")); @@ -125,15 +130,13 @@ public class PathResourceResolverTests { assertNotNull(this.resolver.resolveResource(null, "main.css", Collections.singletonList(location), null)); } - // SPR-12747 - @Test + @Test // SPR-12747 public void checkFileLocation() throws Exception { Resource resource = getResource("main.css"); assertTrue(this.resolver.checkResource(resource, resource)); } - // SPR-13241 - @Test + @Test // SPR-13241 public void resolvePathRootResource() { Resource webjarsLocation = new ClassPathResource("/META-INF/resources/webjars/", PathResourceResolver.class); String path = this.resolver.resolveUrlPathInternal("", Collections.singletonList(webjarsLocation), null); diff --git a/spring-webmvc/src/test/resources/org/springframework/web/servlet/resource/test/test%file.txt b/spring-webmvc/src/test/resources/org/springframework/web/servlet/resource/test/test%file.txt new file mode 100644 index 00000000000..e69de29bb2d