Skip to content

Commit

Permalink
Fix StompSubframeDecoder.readHeaders produce not any notification whe…
Browse files Browse the repository at this point in the history 
…n 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 [netty#7083]
  • Loading branch information
@kiril-me
amizurov authored and kiril-me committed Feb 28, 2018
1 parent c339224 commit cc0dd50
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 15 deletions.
25 changes: 19 additions & 6 deletions codec-stomp/src/main/java/io/netty/handler/codec/stomp/StompSubframeDecoder.java
View file
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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;
Expand All @@ -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(
Expand All @@ -93,6 +102,7 @@ public StompSubframeDecoder(int maxLineLength, int maxChunkSize) {
}
this.maxChunkSize = maxChunkSize;
this.maxLineLength = maxLineLength;
this.validateHeaders = validateHeaders;
}

@Override
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
Expand Down
55 changes: 47 additions & 8 deletions codec-stomp/src/test/java/io/netty/handler/codec/stomp/StompSubframeDecoderTest.java
View file
Expand Up @@ -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 {

Expand Down Expand Up @@ -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();

Expand All @@ -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();

Expand All @@ -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();

Expand Down Expand Up @@ -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());
}
}
8 changes: 7 additions & 1 deletion codec-stomp/src/test/java/io/netty/handler/codec/stomp/StompTestConstants.java
View file
Expand Up @@ -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() { }
}

0 comments on commit cc0dd50

Please sign in to comment.