diff --git a/spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/StompDecoder.java b/spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/StompDecoder.java index 44a083414e1..109d3e1c50f 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/StompDecoder.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/StompDecoder.java @@ -5,7 +5,7 @@ * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * http://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, @@ -56,13 +56,12 @@ public class StompDecoder { /** * Decodes one or more STOMP frames from the given {@code ByteBuffer} into a - * list of {@link Message}s. If the input buffer contains any incplcontains partial STOMP frame content, or additional - * content with a partial STOMP frame, the buffer is reset and {@code null} is - * returned. - * - * @param buffer The buffer to decode the STOMP frame from - * - * @return the decoded messages or an empty list + * list of {@link Message}s. If the input buffer contains partial STOMP frame + * content, or additional content with a partial STOMP frame, the buffer is + * reset and {@code null} is returned. + * @param buffer the buffer to decode the STOMP frame from + * @return the decoded messages, or an empty list if none + * @throws StompConversionException raised in case of decoding issues */ public List> decode(ByteBuffer buffer) { return decode(buffer, new LinkedMultiValueMap()); @@ -71,35 +70,29 @@ public class StompDecoder { /** * Decodes one or more STOMP frames from the given {@code buffer} and returns * a list of {@link Message}s. - * *

If the given ByteBuffer contains only partial STOMP frame content and no * complete STOMP frames, an empty list is returned, and the buffer is reset to * to where it was. - * *

If the buffer contains one ore more STOMP frames, those are returned and * the buffer reset to point to the beginning of the unused partial content. - * *

The input headers map is used to store successfully parsed headers and * is cleared after ever successfully read message. So when partial content is * read the caller can check if a "content-length" header was read, which helps * to determine how much more content is needed before the next STOMP frame * can be decoded. - * - * @param buffer The buffer to decode the STOMP frame from + * @param buffer the buffer to decode the STOMP frame from * @param headers an empty map that will contain successfully parsed headers * in cases where the partial buffer ended with a partial STOMP frame - * - * @return decoded messages or an empty list + * @return the decoded messages, or an empty list if none * @throws StompConversionException raised in case of decoding issues */ public List> decode(ByteBuffer buffer, MultiValueMap headers) { Assert.notNull(headers, "headers is required"); List> messages = new ArrayList>(); while (buffer.hasRemaining()) { - Message m = decodeMessage(buffer, headers); - if (m != null) { - messages.add(m); - headers.clear(); + Message message = decodeMessage(buffer, headers); + if (message != null) { + messages.add(message); } else { break; @@ -112,20 +105,17 @@ public class StompDecoder { * Decode a single STOMP frame from the given {@code buffer} into a {@link Message}. */ private Message decodeMessage(ByteBuffer buffer, MultiValueMap headers) { - Message decodedMessage = null; skipLeadingEol(buffer); buffer.mark(); String command = readCommand(buffer); if (command.length() > 0) { - readHeaders(buffer, headers); byte[] payload = readPayload(buffer, headers); - if (payload != null) { StompCommand stompCommand = StompCommand.valueOf(command); - if ((payload.length > 0) && (!stompCommand.isBodyAllowed())) { + if (payload.length > 0 && !stompCommand.isBodyAllowed()) { throw new StompConversionException(stompCommand + " shouldn't have but " + "has a payload with length=" + payload.length + ", headers=" + headers); } @@ -137,7 +127,7 @@ public class StompDecoder { } else { if (logger.isTraceEnabled()) { - logger.trace("Received incomplete frame. Resetting buffer."); + logger.trace("Incomplete frame, resetting input buffer..."); } buffer.reset(); } @@ -182,10 +172,10 @@ public class StompDecoder { if (headerStream.size() > 0) { String header = new String(headerStream.toByteArray(), UTF8_CHARSET); int colonIndex = header.indexOf(':'); - if ((colonIndex <= 0) || (colonIndex == header.length() - 1)) { + if (colonIndex <= 0 || colonIndex == header.length() - 1) { if (buffer.remaining() > 0) { - throw new StompConversionException( - "Illegal header: '" + header + "'. A header must be of the form :"); + throw new StompConversionException("Illegal header: '" + header + + "'. A header must be of the form :."); } } else { @@ -205,13 +195,15 @@ public class StompDecoder { * "Value Encoding". */ private String unescape(String inString) { - - StringBuilder sb = new StringBuilder(); - int pos = 0; // position in the old string + StringBuilder sb = new StringBuilder(inString.length()); + int pos = 0; // position in the old string int index = inString.indexOf("\\"); while (index >= 0) { sb.append(inString.substring(pos, index)); + if (index + 1 >= inString.length()) { + throw new StompConversionException("Illegal escape sequence at index " + index + ": " + inString); + } Character c = inString.charAt(index + 1); if (c == 'r') { sb.append('\r'); @@ -282,7 +274,6 @@ public class StompDecoder { /** * Try to read an EOL incrementing the buffer position if successful. - * * @return whether an EOL was consumed */ private boolean tryConsumeEndOfLine(ByteBuffer buffer) { diff --git a/spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/StompEncoder.java b/spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/StompEncoder.java index ae7e54315a8..eaf42681639 100644 --- a/spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/StompEncoder.java +++ b/spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/StompEncoder.java @@ -5,7 +5,7 @@ * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * http://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, @@ -74,8 +74,8 @@ public final class StompEncoder { return baos.toByteArray(); } - catch (IOException e) { - throw new StompConversionException("Failed to encode STOMP frame", e); + catch (IOException ex) { + throw new StompConversionException("Failed to encode STOMP frame", ex); } } @@ -108,8 +108,8 @@ public final class StompEncoder { } private byte[] encodeHeaderString(String input, boolean escape) { - input = escape ? escape(input) : input; - return input.getBytes(UTF8_CHARSET); + String inputToUse = (escape ? escape(input) : input); + return inputToUse.getBytes(UTF8_CHARSET); } /** diff --git a/spring-messaging/src/test/java/org/springframework/messaging/simp/stomp/BufferingStompDecoderTests.java b/spring-messaging/src/test/java/org/springframework/messaging/simp/stomp/BufferingStompDecoderTests.java index 119953cfd8f..136a99acc9f 100644 --- a/spring-messaging/src/test/java/org/springframework/messaging/simp/stomp/BufferingStompDecoderTests.java +++ b/spring-messaging/src/test/java/org/springframework/messaging/simp/stomp/BufferingStompDecoderTests.java @@ -5,7 +5,7 @@ * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * http://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, @@ -16,18 +16,16 @@ package org.springframework.messaging.simp.stomp; -import org.junit.Test; -import org.springframework.messaging.Message; -import org.springframework.messaging.converter.MessageConversionException; - import java.nio.ByteBuffer; import java.nio.charset.Charset; -import java.util.Arrays; +import java.util.Collections; import java.util.List; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.fail; +import org.junit.Test; + +import org.springframework.messaging.Message; + +import static org.junit.Assert.*; /** @@ -38,10 +36,11 @@ import static org.junit.Assert.fail; */ public class BufferingStompDecoderTests { + private final StompDecoder STOMP_DECODER = new StompDecoder(); + @Test public void basic() throws InterruptedException { - BufferingStompDecoder stompDecoder = new BufferingStompDecoder(128); String chunk = "SEND\na:alpha\n\nMessage body\0"; @@ -52,7 +51,7 @@ public class BufferingStompDecoderTests { assertEquals(0, stompDecoder.getBufferSize()); assertNull(stompDecoder.getExpectedContentLength()); } - + @Test public void oneMessageInTwoChunks() throws InterruptedException { @@ -61,7 +60,7 @@ public class BufferingStompDecoderTests { String chunk2 = " body\0"; List> messages = stompDecoder.decode(toByteBuffer(chunk1)); - assertEquals(Arrays.asList(), messages); + assertEquals(Collections.>emptyList(), messages); messages = stompDecoder.decode(toByteBuffer(chunk2)); assertEquals(1, messages.size()); @@ -73,7 +72,6 @@ public class BufferingStompDecoderTests { @Test public void twoMessagesInOneChunk() throws InterruptedException { - BufferingStompDecoder stompDecoder = new BufferingStompDecoder(128); String chunk = "SEND\na:alpha\n\nPayload1\0" + "SEND\na:alpha\n\nPayload2\0"; List> messages = stompDecoder.decode(toByteBuffer(chunk)); @@ -88,7 +86,6 @@ public class BufferingStompDecoderTests { @Test public void oneFullAndOneSplitMessageContentLength() throws InterruptedException { - int contentLength = "Payload2a-Payload2b".getBytes().length; BufferingStompDecoder stompDecoder = new BufferingStompDecoder(128); @@ -119,7 +116,6 @@ public class BufferingStompDecoderTests { @Test public void oneFullAndOneSplitMessageNoContentLength() throws InterruptedException { - BufferingStompDecoder stompDecoder = new BufferingStompDecoder(128); String chunk1 = "SEND\na:alpha\n\nPayload1\0SEND\na:alpha\n"; List> messages = stompDecoder.decode(toByteBuffer(chunk1)); @@ -148,7 +144,6 @@ public class BufferingStompDecoderTests { @Test public void oneFullAndOneSplitWithContentLengthExceedingBufferSize() throws InterruptedException { - BufferingStompDecoder stompDecoder = new BufferingStompDecoder(128); String chunk1 = "SEND\na:alpha\n\nPayload1\0SEND\ncontent-length:129\n"; List> messages = stompDecoder.decode(toByteBuffer(chunk1)); @@ -165,6 +160,7 @@ public class BufferingStompDecoderTests { fail("Expected exception"); } catch (StompConversionException ex) { + // expected } } @@ -175,6 +171,22 @@ public class BufferingStompDecoderTests { stompDecoder.decode(toByteBuffer(payload)); } + @Test + public void incompleteCommand() { + BufferingStompDecoder stompDecoder = new BufferingStompDecoder(128); + String chunk = "MESSAG"; + + List> messages = stompDecoder.decode(toByteBuffer(chunk)); + assertEquals(0, messages.size()); + } + + @Test(expected = StompConversionException.class) // SPR-12418 + public void endingBackslashHeaderValueCheck() { + BufferingStompDecoder stompDecoder = new BufferingStompDecoder(128); + String payload = "SEND\na:alpha\\\n\nMessage body\0"; + stompDecoder.decode(toByteBuffer(payload)); + } + private ByteBuffer toByteBuffer(String chunk) { return ByteBuffer.wrap(chunk.getBytes(Charset.forName("UTF-8")));