Validate if STOMP frame is allowed to have a body

Issue: SPR-10890

Author: rstoyanchev

spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/StompCommand.java

@@ -17,10 +17,9 @@
 package org.springframework.messaging.simp.stomp;
 
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Map;
-import java.util.Set;
 
 import org.springframework.messaging.simp.SimpMessageType;
 
@@ -51,36 +50,38 @@ public enum StompCommand {
 	RECEIPT,
 	ERROR;
 
+	private static Map<StompCommand, SimpMessageType> messageTypes = new HashMap<StompCommand, SimpMessageType>();
 
-	private static Map<StompCommand, SimpMessageType> messageTypeLookup = new HashMap<StompCommand, SimpMessageType>();
-
-	private static Set<StompCommand> destinationRequiredLookup =
-			new HashSet<StompCommand>(Arrays.asList(SEND, SUBSCRIBE, MESSAGE));
-
-	private static Set<StompCommand> subscriptionIdRequiredLookup =
-			new HashSet<StompCommand>(Arrays.asList(SUBSCRIBE, UNSUBSCRIBE, MESSAGE));
+	private static Collection<StompCommand> destinationRequired = Arrays.asList(SEND, SUBSCRIBE, MESSAGE);
+	private static Collection<StompCommand> subscriptionIdRequired = Arrays.asList(SUBSCRIBE, UNSUBSCRIBE, MESSAGE);
+	private static Collection<StompCommand> bodyAllowed = Arrays.asList(SEND, MESSAGE, ERROR);
 
 	static {
-		messageTypeLookup.put(StompCommand.CONNECT, SimpMessageType.CONNECT);
-		messageTypeLookup.put(StompCommand.STOMP, SimpMessageType.CONNECT);
-		messageTypeLookup.put(StompCommand.SEND, SimpMessageType.MESSAGE);
-		messageTypeLookup.put(StompCommand.MESSAGE, SimpMessageType.MESSAGE);
-		messageTypeLookup.put(StompCommand.SUBSCRIBE, SimpMessageType.SUBSCRIBE);
-		messageTypeLookup.put(StompCommand.UNSUBSCRIBE, SimpMessageType.UNSUBSCRIBE);
-		messageTypeLookup.put(StompCommand.DISCONNECT, SimpMessageType.DISCONNECT);
+		messageTypes.put(StompCommand.CONNECT, SimpMessageType.CONNECT);
+		messageTypes.put(StompCommand.STOMP, SimpMessageType.CONNECT);
+		messageTypes.put(StompCommand.SEND, SimpMessageType.MESSAGE);
+		messageTypes.put(StompCommand.MESSAGE, SimpMessageType.MESSAGE);
+		messageTypes.put(StompCommand.SUBSCRIBE, SimpMessageType.SUBSCRIBE);
+		messageTypes.put(StompCommand.UNSUBSCRIBE, SimpMessageType.UNSUBSCRIBE);
+		messageTypes.put(StompCommand.DISCONNECT, SimpMessageType.DISCONNECT);
 	}
 
 	public SimpMessageType getMessageType() {
-		SimpMessageType type = messageTypeLookup.get(this);
+		SimpMessageType type = messageTypes.get(this);
 		return (type != null) ? type : SimpMessageType.OTHER;
 	}
 
 	public boolean requiresDestination() {
-		return destinationRequiredLookup.contains(this);
+		return destinationRequired.contains(this);
 	}
 
 	public boolean requiresSubscriptionId() {
-		return subscriptionIdRequiredLookup.contains(this);
+		return subscriptionIdRequired.contains(this);
+	}
+
+	public boolean isBodyAllowed() {
+		return bodyAllowed.contains(this);
 	}
 
 }
+

spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/StompDecoder.java

@@ -29,9 +29,9 @@
 import org.springframework.util.MultiValueMap;
 
 /**
- * A decoder for STOMP frames
+ * A decoder for STOMP frames.
  *
- * @author awilkinson
+ * @author Andy Wilkinson
  * @since 4.0
  */
 public class StompDecoder {
@@ -60,8 +60,15 @@ public Message<byte[]> decode(ByteBuffer buffer) {
 			MultiValueMap<String, String> headers = readHeaders(buffer);
 			byte[] payload = readPayload(buffer, headers);
 
-			decodedMessage = MessageBuilder.withPayload(payload).setHeaders(
-					StompHeaderAccessor.create(StompCommand.valueOf(command), headers)).build();
+			StompCommand stompCommand = StompCommand.valueOf(command);
+			if ((payload.length > 0) && (!stompCommand.isBodyAllowed())) {
+				throw new StompConversionException(stompCommand +
+						" isn't allowed to have a body but has payload length=" + payload.length +
+						", headers=" + headers);
+			}
+
+			decodedMessage = MessageBuilder.withPayload(payload)
+					.setHeaders(StompHeaderAccessor.create(stompCommand, headers)).build();
 		}
 		else {
 			decodedMessage = MessageBuilder.withPayload(HEARTBEAT_PAYLOAD).setHeaders(

spring-messaging/src/main/java/org/springframework/messaging/simp/stomp/StompEncoder.java

@@ -29,7 +29,7 @@
 import org.springframework.messaging.simp.SimpMessageType;
 
 /**
- * An encoder for STOMP frames
+ * An encoder for STOMP frames.
  *
  * @author Andy Wilkinson
  * @since 4.0

spring-messaging/src/main/java/org/springframework/messaging/support/converter/MessageConversionException.java

@@ -20,12 +20,15 @@
 import org.springframework.messaging.MessagingException;
 
 /**
+ * An exception raised by {@link MessageConverter} implementations.
+ *
  * @author Mark Fisher
  * @since 4.0
  */
 @SuppressWarnings("serial")
 public class MessageConversionException extends MessagingException {
 
+
 	public MessageConversionException(String description, Throwable cause) {
 		super(description, cause);
 	}

spring-messaging/src/test/java/org/springframework/messaging/simp/stomp/StompCodecTests.java

@@ -133,6 +133,11 @@ public void decodeFrameWithEscapedHeaders() {
 		assertEquals("alpha:bravo\r\n\\", headers.getFirstNativeHeader("a:\r\n\\b"));
 	}
 
+	@Test(expected=StompConversionException.class)
+	public void decodeFrameBodyNotAllowed() {
+		decode("CONNECT\naccept-version:1.2\n\nThe body of the message\0");
+	}
+
 	@Test
 	public void decodeMultipleFramesFromSameBuffer() {
 		String frame1 = "SEND\ndestination:test\n\nThe body of the message\0";