From cc336ef2f898db83de46e8d48879b03c1a4895d1 Mon Sep 17 00:00:00 2001 From: amizurov Date: Mon, 28 Aug 2017 23:58:26 +0300 Subject: [PATCH] Fix StompSubframeDecoder.readHeaders produce not any notification when parsed line that contains multiple colon Motivation: By STOMP 1.2 specification - header name or value include any octet except CR or LF or ":". Modification: Add constructor argument that allows to enable / disable validation. Result: Fixes [#7083] --- .../codec/stomp/StompSubframeDecoder.java | 25 +++++++-- .../codec/stomp/StompSubframeDecoderTest.java | 55 ++++++++++++++++--- .../codec/stomp/StompTestConstants.java | 8 ++- 3 files changed, 73 insertions(+), 15 deletions(-) diff --git a/codec-stomp/src/main/java/io/netty/handler/codec/stomp/StompSubframeDecoder.java b/codec-stomp/src/main/java/io/netty/handler/codec/stomp/StompSubframeDecoder.java index e25c15447a..3ce55f21cb 100644 --- a/codec-stomp/src/main/java/io/netty/handler/codec/stomp/StompSubframeDecoder.java +++ b/codec-stomp/src/main/java/io/netty/handler/codec/stomp/StompSubframeDecoder.java @@ -15,6 +15,9 @@ */ package io.netty.handler.codec.stomp; +import java.util.List; +import java.util.Locale; + import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; import io.netty.channel.ChannelHandlerContext; @@ -25,9 +28,6 @@ import io.netty.handler.codec.TooLongFrameException; import io.netty.handler.codec.stomp.StompSubframeDecoder.State; import io.netty.util.internal.AppendableCharSequence; -import java.util.List; -import java.util.Locale; - import static io.netty.buffer.ByteBufUtil.indexOf; import static io.netty.buffer.ByteBufUtil.readBytes; @@ -71,6 +71,7 @@ public class StompSubframeDecoder extends ReplayingDecoder { private final int maxLineLength; private final int maxChunkSize; + private final boolean validateHeaders; private int alreadyReadChunkSize; private LastStompContentSubframe lastContent; private long contentLength = -1; @@ -79,7 +80,15 @@ public class StompSubframeDecoder extends ReplayingDecoder { this(DEFAULT_MAX_LINE_LENGTH, DEFAULT_CHUNK_SIZE); } + public StompSubframeDecoder(boolean validateHeaders) { + this(DEFAULT_MAX_LINE_LENGTH, DEFAULT_CHUNK_SIZE, validateHeaders); + } + public StompSubframeDecoder(int maxLineLength, int maxChunkSize) { + this(maxLineLength, maxChunkSize, false); + } + + public StompSubframeDecoder(int maxLineLength, int maxChunkSize, boolean validateHeaders) { super(State.SKIP_CONTROL_CHARACTERS); if (maxLineLength <= 0) { throw new IllegalArgumentException( @@ -93,6 +102,7 @@ public class StompSubframeDecoder extends ReplayingDecoder { } this.maxChunkSize = maxChunkSize; this.maxLineLength = maxLineLength; + this.validateHeaders = validateHeaders; } @Override @@ -134,7 +144,7 @@ public class StompSubframeDecoder extends ReplayingDecoder { if (toRead > maxChunkSize) { toRead = maxChunkSize; } - if (this.contentLength >= 0) { + if (contentLength >= 0) { int remainingLength = (int) (contentLength - alreadyReadChunkSize); if (toRead > remainingLength) { toRead = remainingLength; @@ -214,11 +224,14 @@ public class StompSubframeDecoder extends ReplayingDecoder { String[] split = line.split(":"); if (split.length == 2) { headers.add(split[0], split[1]); + } else if (validateHeaders) { + throw new IllegalArgumentException("a header value or name contains a prohibited character ':'" + + ", " + line); } } else { if (headers.contains(StompHeaders.CONTENT_LENGTH)) { - this.contentLength = getContentLength(headers, 0); - if (this.contentLength == 0) { + contentLength = getContentLength(headers, 0); + if (contentLength == 0) { return State.FINALIZE_FRAME_READ; } } diff --git a/codec-stomp/src/test/java/io/netty/handler/codec/stomp/StompSubframeDecoderTest.java b/codec-stomp/src/test/java/io/netty/handler/codec/stomp/StompSubframeDecoderTest.java index 5b9b97e32c..fe772126a5 100644 --- a/codec-stomp/src/test/java/io/netty/handler/codec/stomp/StompSubframeDecoderTest.java +++ b/codec-stomp/src/test/java/io/netty/handler/codec/stomp/StompSubframeDecoderTest.java @@ -18,12 +18,19 @@ package io.netty.handler.codec.stomp; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; import io.netty.channel.embedded.EmbeddedChannel; -import io.netty.util.CharsetUtil; import org.junit.After; import org.junit.Before; import org.junit.Test; -import static org.junit.Assert.*; +import static io.netty.handler.codec.stomp.StompTestConstants.FRAME_WITH_INVALID_HEADER; +import static io.netty.util.CharsetUtil.US_ASCII; +import static io.netty.util.CharsetUtil.UTF_8; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; public class StompSubframeDecoderTest { @@ -69,7 +76,7 @@ public class StompSubframeDecoderTest { StompContentSubframe content = channel.readInbound(); assertTrue(content instanceof LastStompContentSubframe); - String s = content.content().toString(CharsetUtil.UTF_8); + String s = content.content().toString(UTF_8); assertEquals("hello, queue a!!!", s); content.release(); @@ -88,7 +95,7 @@ public class StompSubframeDecoderTest { StompContentSubframe content = channel.readInbound(); assertTrue(content instanceof LastStompContentSubframe); - String s = content.content().toString(CharsetUtil.UTF_8); + String s = content.content().toString(UTF_8); assertEquals("hello, queue a!", s); content.release(); @@ -108,22 +115,22 @@ public class StompSubframeDecoderTest { assertEquals(StompCommand.SEND, frame.command()); StompContentSubframe content = channel.readInbound(); - String s = content.content().toString(CharsetUtil.UTF_8); + String s = content.content().toString(UTF_8); assertEquals("hello", s); content.release(); content = channel.readInbound(); - s = content.content().toString(CharsetUtil.UTF_8); + s = content.content().toString(UTF_8); assertEquals(", que", s); content.release(); content = channel.readInbound(); - s = content.content().toString(CharsetUtil.UTF_8); + s = content.content().toString(UTF_8); assertEquals("ue a!", s); content.release(); content = channel.readInbound(); - s = content.content().toString(CharsetUtil.UTF_8); + s = content.content().toString(UTF_8); assertEquals("!!", s); content.release(); @@ -155,4 +162,36 @@ public class StompSubframeDecoderTest { assertNull(channel.readInbound()); } + + @Test + public void testValidateHeadersDecodingDisabled() { + ByteBuf invalidIncoming = Unpooled.copiedBuffer(FRAME_WITH_INVALID_HEADER.getBytes(US_ASCII)); + assertTrue(channel.writeInbound(invalidIncoming)); + + StompHeadersSubframe frame = channel.readInbound(); + assertNotNull(frame); + assertEquals(StompCommand.SEND, frame.command()); + assertTrue(frame.headers().contains("destination")); + assertTrue(frame.headers().contains("content-type")); + assertFalse(frame.headers().contains("current-time")); + + StompContentSubframe content = channel.readInbound(); + String s = content.content().toString(UTF_8); + assertEquals("some body", s); + content.release(); + } + + @Test + public void testValidateHeadersDecodingEnabled() { + channel = new EmbeddedChannel(new StompSubframeDecoder(true)); + + ByteBuf invalidIncoming = Unpooled.copiedBuffer(FRAME_WITH_INVALID_HEADER.getBytes(US_ASCII)); + assertTrue(channel.writeInbound(invalidIncoming)); + + StompHeadersSubframe frame = channel.readInbound(); + assertNotNull(frame); + assertTrue(frame.decoderResult().isFailure()); + assertEquals("a header value or name contains a prohibited character ':', current-time:2000-01-01T00:00:00", + frame.decoderResult().cause().getMessage()); + } } diff --git a/codec-stomp/src/test/java/io/netty/handler/codec/stomp/StompTestConstants.java b/codec-stomp/src/test/java/io/netty/handler/codec/stomp/StompTestConstants.java index 7d0de42912..0d89a5c3cc 100644 --- a/codec-stomp/src/test/java/io/netty/handler/codec/stomp/StompTestConstants.java +++ b/codec-stomp/src/test/java/io/netty/handler/codec/stomp/StompTestConstants.java @@ -57,6 +57,12 @@ public final class StompTestConstants { '\n' + "body\0"; - private StompTestConstants() { } + public static final String FRAME_WITH_INVALID_HEADER = "SEND\n" + + "destination:/some-destination\n" + + "content-type:text/plain\n" + + "current-time:2000-01-01T00:00:00\n" + + '\n' + + "some body\0"; + private StompTestConstants() { } }