From 942b993f2b9781ff2126ff92a6be5b975dfc72ed Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Wed, 13 Dec 2017 20:26:35 +0100 Subject: [PATCH] Only enable validation of headers if original headers were validating as well. Motiviation: In our replace(...) methods we always used validation for the newly created headers while the original headers may not use validation at all. Modifications: - Only use validation if the original headers used validation as well. - Ensure we create a copy of the headers in replace(...). Result: Fixes [#5226] --- .../handler/codec/http/DefaultFullHttpRequest.java | 5 ++++- .../handler/codec/http/DefaultFullHttpResponse.java | 5 ++++- .../netty/handler/codec/http/DefaultHttpHeaders.java | 5 +++++ .../java/io/netty/handler/codec/http/HttpHeaders.java | 7 +++++++ .../netty/handler/codec/http/HttpObjectAggregator.java | 10 ++++------ .../codec/http2/InboundHttp2ToHttpAdapterTest.java | 4 ++++ .../java/io/netty/handler/codec/DefaultHeaders.java | 10 ++++++++++ 7 files changed, 38 insertions(+), 8 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/DefaultFullHttpRequest.java b/codec-http/src/main/java/io/netty/handler/codec/http/DefaultFullHttpRequest.java index ba55d9657e..117e6dbf64 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/DefaultFullHttpRequest.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/DefaultFullHttpRequest.java @@ -142,7 +142,10 @@ public class DefaultFullHttpRequest extends DefaultHttpRequest implements FullHt @Override public FullHttpRequest replace(ByteBuf content) { - return new DefaultFullHttpRequest(protocolVersion(), method(), uri(), content, headers(), trailingHeaders()); + FullHttpRequest request = new DefaultFullHttpRequest(protocolVersion(), method(), uri(), content, + headers().copy(), trailingHeaders().copy()); + request.setDecoderResult(decoderResult()); + return request; } @Override diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/DefaultFullHttpResponse.java b/codec-http/src/main/java/io/netty/handler/codec/http/DefaultFullHttpResponse.java index 134865cabb..012b996a24 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/DefaultFullHttpResponse.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/DefaultFullHttpResponse.java @@ -149,7 +149,10 @@ public class DefaultFullHttpResponse extends DefaultHttpResponse implements Full @Override public FullHttpResponse replace(ByteBuf content) { - return new DefaultFullHttpResponse(protocolVersion(), status(), content, headers(), trailingHeaders()); + FullHttpResponse response = new DefaultFullHttpResponse(protocolVersion(), status(), content, + headers().copy(), trailingHeaders().copy()); + response.setDecoderResult(decoderResult()); + return response; } @Override diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java b/codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java index e8fcf3e7ad..88af27f738 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/DefaultHttpHeaders.java @@ -348,6 +348,11 @@ public class DefaultHttpHeaders extends HttpHeaders { return headers.hashCode(CASE_SENSITIVE_HASHER); } + @Override + public HttpHeaders copy() { + return new DefaultHttpHeaders(headers.copy()); + } + private static void validateHeaderNameElement(byte value) { switch (value) { case 0x00: diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpHeaders.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpHeaders.java index 7df00eac2b..35cfc5c5f1 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpHeaders.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpHeaders.java @@ -1693,4 +1693,11 @@ public abstract class HttpHeaders implements Iterable> public String toString() { return HeadersUtils.toString(getClass(), iteratorCharSequence(), size()); } + + /** + * Returns a deap copy of the passed in {@link HttpHeaders}. + */ + public HttpHeaders copy() { + return new DefaultHttpHeaders().set(this); + } } diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectAggregator.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectAggregator.java index ace7e4642b..257581fe23 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectAggregator.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectAggregator.java @@ -424,9 +424,8 @@ public class HttpObjectAggregator @Override public FullHttpRequest replace(ByteBuf content) { - DefaultFullHttpRequest dup = new DefaultFullHttpRequest(protocolVersion(), method(), uri(), content); - dup.headers().set(headers()); - dup.trailingHeaders().set(trailingHeaders()); + DefaultFullHttpRequest dup = new DefaultFullHttpRequest(protocolVersion(), method(), uri(), content, + headers().copy(), trailingHeaders().copy()); dup.setDecoderResult(decoderResult()); return dup; } @@ -523,9 +522,8 @@ public class HttpObjectAggregator @Override public FullHttpResponse replace(ByteBuf content) { - DefaultFullHttpResponse dup = new DefaultFullHttpResponse(getProtocolVersion(), getStatus(), content); - dup.headers().set(headers()); - dup.trailingHeaders().set(trailingHeaders()); + DefaultFullHttpResponse dup = new DefaultFullHttpResponse(getProtocolVersion(), getStatus(), content, + headers().copy(), trailingHeaders().copy()); dup.setDecoderResult(decoderResult()); return dup; } diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java index b29c0966d7..8fb4ebfc67 100644 --- a/codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java +++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java @@ -62,6 +62,7 @@ import static io.netty.handler.codec.http2.Http2TestUtil.of; import static io.netty.handler.codec.http2.Http2TestUtil.runInChannel; import static java.util.concurrent.TimeUnit.SECONDS; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -607,6 +608,9 @@ public class InboundHttp2ToHttpAdapterTest { verify(serverListener, times(2)).messageReceived(requestCaptor.capture()); capturedRequests = requestCaptor.getAllValues(); assertEquals(2, capturedRequests.size()); + // We do not expect to have this header in the captured request so remove it now. + assertNotNull(request.headers().remove("x-http2-stream-weight")); + assertEquals(request, capturedRequests.get(0)); assertEquals(request2, capturedRequests.get(1)); diff --git a/codec/src/main/java/io/netty/handler/codec/DefaultHeaders.java b/codec/src/main/java/io/netty/handler/codec/DefaultHeaders.java index 5664706b98..01b64e8562 100644 --- a/codec/src/main/java/io/netty/handler/codec/DefaultHeaders.java +++ b/codec/src/main/java/io/netty/handler/codec/DefaultHeaders.java @@ -952,6 +952,16 @@ public class DefaultHeaders> implements Headers return (T) this; } + /** + * Returns a deep copy of this instance. + */ + public DefaultHeaders copy() { + DefaultHeaders copy = new DefaultHeaders( + hashingStrategy, valueConverter, nameValidator, entries.length); + copy.addImpl(this); + return copy; + } + private final class HeaderIterator implements Iterator> { private HeaderEntry current = head;