Repository: netty/netty
Issue: StompSubframeDecoder.redHeaders no any notification when parsed line that contains multiple colon

### Expected behavior
By STOMP 1.2 specification - header name or value include any octet except CR or LF or ":". 

I understand that we can not just change the current behavior because of compatibility, but maybe we can add some kind of error notification ?

### Actual behavior
We can built stomp frame that contain invalid header. And also when decoder parsed line it simply skips it without any notification error.

### Steps to reproduce
Build DefaultStompFrame for any command  with invalid header (time:2017-01-01T00:00:00) and then decoded message, header missing.

### Minimal yet complete reproducer code (or URL to code)

### Netty version
4.1.13

### JVM version (e.g. `java -version`)
1.8.0_131

### OS version (e.g. `uname -a`)
ProductName:	Mac OS X
ProductVersion:	10.12.6

Issue comments:
@amizurov maybe you want to submit a PR and just make it configurable via a constructor argument (and using the current behaviour by default) ?
@normanmaurer Ok, with great pleasure.
Fixed by https://github.com/netty/netty/pull/7160

Pull request review comments:
For splitting with one-char String better use `String#split` instead of `Pattern`, [there is an optimization for this](http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/687fd7c7986d/src/share/classes/java/lang/String.java#l2316).
@fenik17 Thanks, I agree it is better approach for this case.
nit: you can remove the final here.
nit: remove the final to make it consistent
call `channel.finish()` and validate the return value
`getBytes(CharsetUtil.US_ASCII)`
`getBytes(CharsetUtil.US_ASCII)`
assert return value
assert return value
@After
    public void teardown() throws Exception {
        assertFalse(channel.finish());
    }
ah sorry missed this 👍 
sorry for be a pita but just merge the two lines above^^
sorry for be a pita but just merge the two lines above^^
Did miss this before but just do:

```java
ByteBuf invalidIncoming = Unpooled.copiedBuffer(StompTestConstants.FRAME_WITH_INVALID_HEADER, CharsetUtil.US_ASCII);
```
Did miss this before but just do:

```java
ByteBuf invalidIncoming = Unpooled.copiedBuffer(StompTestConstants.FRAME_WITH_INVALID_HEADER, CharsetUtil.US_ASCII);
```
remove `EmbeddedChannel` here to ensure `channel.finish()` is called on the correct instance later on.
can you move this to the other final fields and so group these together.

Patch that resolved the issue:
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 e25c15447ac..3ce55f21cbe 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.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 @@ enum State {
 
     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 StompSubframeDecoder() {
         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 StompSubframeDecoder(int maxLineLength, int maxChunkSize) {
         }
         this.maxChunkSize = maxChunkSize;
         this.maxLineLength = maxLineLength;
+        this.validateHeaders = validateHeaders;
     }
 
     @Override
@@ -134,7 +144,7 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) t
                     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 @@ private State readHeaders(ByteBuf buffer, StompHeaders headers) {
                 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;
                     }
                 }


Test patch:
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 5b9b97e32c8..fe772126a55 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 @@
 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 void testSingleFrameWithBodyAndContentLength() {
 
         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 void testSingleFrameWithBodyWithoutContentLength() {
 
         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 void testSingleFrameChunked() {
         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 void testMultipleFramesDecoding() {
 
         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 7d0de429122..0d89a5c3cc1 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() { }
 }