From 8fb92dc7e21f07f3135e60e256bb3d716387973f Mon Sep 17 00:00:00 2001
From: Derek Collison <derek.collison@gmail.com>
Date: Wed, 26 Aug 2015 23:04:10 -0700
Subject: [PATCH] Fix parser bug around MSG protocol.

Make sure we do the right thing when no args are presented for an MSG, e.g. MSG <spc>.
Also do not parse at all of this is a client, only valid for routes.
---
 server/client.go      |  8 +++++---
 server/parser.go      |  8 ++++++--
 server/parser_test.go | 22 +++++++++++++++++++++-
 server/split_test.go  |  4 +++-
 4 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/server/client.go b/server/client.go
index 078b9a9b..69b303aa 100644
--- a/server/client.go
+++ b/server/client.go
@@ -374,9 +374,6 @@ func (c *client) processMsgArgs(arg []byte) error {
 		args = append(args, arg[start:])
 	}
 
-	c.pa.subject = args[0]
-	c.pa.sid = args[1]
-
 	switch len(args) {
 	case 3:
 		c.pa.reply = nil
@@ -392,6 +389,11 @@ func (c *client) processMsgArgs(arg []byte) error {
 	if c.pa.size < 0 {
 		return fmt.Errorf("processMsgArgs Bad or Missing Size: '%s'", arg)
 	}
+
+	// Common ones processed after check for arg length
+	c.pa.subject = args[0]
+	c.pa.sid = args[1]
+
 	return nil
 }
 
diff --git a/server/parser.go b/server/parser.go
index a667b7cc..88a1367c 100644
--- a/server/parser.go
+++ b/server/parser.go
@@ -104,7 +104,11 @@ func (c *client) parse(buf []byte) error {
 			case 'U', 'u':
 				c.state = OP_U
 			case 'M', 'm':
-				c.state = OP_M
+				if c.typ == CLIENT {
+					goto parseErr
+				} else {
+					c.state = OP_M
+				}
 			case 'C', 'c':
 				c.state = OP_C
 			case 'I', 'i':
@@ -613,7 +617,7 @@ func (c *client) parse(buf []byte) error {
 		c.state == CONNECT_ARG) && c.argBuf == nil {
 		c.argBuf = c.scratch[:0]
 		c.argBuf = append(c.argBuf, buf[c.as:i-c.drop]...)
-		// FIXME, check max len
+		// FIXME(dlc), check max control line len
 	}
 	// Check for split msg
 	if (c.state == MSG_PAYLOAD || c.state == MSG_END) && c.msgBuf == nil {
diff --git a/server/parser_test.go b/server/parser_test.go
index 64e0283e..62929b01 100644
--- a/server/parser_test.go
+++ b/server/parser_test.go
@@ -11,6 +11,10 @@ func dummyClient() *client {
 	return &client{}
 }
 
+func dummyRouteClient() *client {
+	return &client{typ: ROUTER}
+}
+
 func TestParsePing(t *testing.T) {
 	c := dummyClient()
 	if c.state != OP_START {
@@ -233,7 +237,7 @@ func TestParsePubBadSize(t *testing.T) {
 }
 
 func TestParseMsg(t *testing.T) {
-	c := dummyClient()
+	c := dummyRouteClient()
 
 	pub := []byte("MSG foo RSID:1:2 5\r\nhello\r")
 	err := c.parse(pub)
@@ -310,6 +314,22 @@ func TestParseMsgArg(t *testing.T) {
 	testMsgArg(c, t)
 }
 
+func TestParseMsgSpace(t *testing.T) {
+	c := dummyRouteClient()
+
+	// Ivan bug he found
+	if err := c.parse([]byte("MSG \r\n")); err == nil {
+		t.Fatalf("Expected parse error for MSG <SPC>")
+	}
+
+	c = dummyClient()
+
+	// Anything with an M from a client should parse error
+	if err := c.parse([]byte("M")); err == nil {
+		t.Fatalf("Expected parse error for M* from a client")
+	}
+}
+
 func TestShouldFail(t *testing.T) {
 	c := dummyClient()
 
diff --git a/server/split_test.go b/server/split_test.go
index b4f7ffbc..ccc868bd 100644
--- a/server/split_test.go
+++ b/server/split_test.go
@@ -344,6 +344,8 @@ func TestSplitDanglingArgBuf(t *testing.T) {
 
 func TestSplitMsgArg(t *testing.T) {
 	_, c, _ := setupClient()
+	// Allow parser to process MSG
+	c.typ = ROUTER
 
 	b := make([]byte, 1024)
 
@@ -371,7 +373,7 @@ func TestSplitMsgArg(t *testing.T) {
 }
 
 func TestSplitBufferMsgOp(t *testing.T) {
-	c := &client{subs: hashmap.New()}
+	c := &client{subs: hashmap.New(), typ: ROUTER}
 	msg := []byte("MSG foo.bar QRSID:15:3 _INBOX.22 11\r\nhello world\r")
 	msg1 := msg[:2]
 	msg2 := msg[2:9]