From 884d2253518265f9469c04ba097e4a2a0407f7e3 Mon Sep 17 00:00:00 2001
From: Ivan Kozlovic <ivan@synadia.com>
Date: Tue, 13 Feb 2018 20:02:02 -0700
Subject: [PATCH] [FIXED] Failed subscription still appears in client's list of
 subscriptions

When the MaxSubscriptions limit is set, an application would correctly
get an error when trying to create a subscription more than the limit.
However, this subscription would still appear in the list of
subscriptions for that client (as seen in the monitor endpoints).

Resolves #467
---
 server/server.go             |  6 +++++-
 server/server_limits_test.go | 38 +++++++++++++++++++++++++++---------
 2 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/server/server.go b/server/server.go
index a8fcff87..00b27478 100644
--- a/server/server.go
+++ b/server/server.go
@@ -3728,7 +3728,11 @@ func (s *StanServer) addSubscription(ss *subStore, sub *subState) error {
 		return fmt.Errorf("can't find clientID: %v", sub.ClientID)
 	}
 	// Store this subscription in subStore
-	return ss.Store(sub)
+	if err := ss.Store(sub); err != nil {
+		s.clients.removeSub(sub.ClientID, sub)
+		return err
+	}
+	return nil
 }
 
 // updateDurable adds back `sub` to the client and updates the store.
diff --git a/server/server_limits_test.go b/server/server_limits_test.go
index 9e9006ad..e9438d6f 100644
--- a/server/server_limits_test.go
+++ b/server/server_limits_test.go
@@ -73,19 +73,39 @@ func TestTooManySubs(t *testing.T) {
 	if _, err := sc.Subscribe("foo", func(_ *stan.Msg) {}); err != nil {
 		t.Fatalf("Unexpected error on subscribe: %v", err)
 	}
+	check := func() {
+		cs := channelsGet(t, s.channels, "foo")
+		ss := cs.ss
+		ss.RLock()
+		if ss.psubs == nil || len(ss.psubs) != 1 {
+			stackFatalf(t, "Expected only one subscription, got %v", len(ss.psubs))
+		}
+		ss.RUnlock()
+		subs := s.clients.getSubs(clientName)
+		if len(subs) != 1 {
+			stackFatalf(t, "Expected 1 subscription for client %q, got %+v", clientName, subs)
+		}
+	}
 	// We should get an error here
 	if _, err := sc.Subscribe("foo", func(_ *stan.Msg) {}); err == nil {
 		t.Fatal("Expected error on subscribe, go none")
 	}
-	cs := channelsGet(t, s.channels, "foo")
-	ss := cs.ss
-	func() {
-		ss.RLock()
-		defer ss.RUnlock()
-		if ss.psubs == nil || len(ss.psubs) != 1 {
-			t.Fatalf("Expected only one subscription, got %v", len(ss.psubs))
-		}
-	}()
+	check()
+	// Try with a durable
+	if _, err := sc.Subscribe("foo", func(_ *stan.Msg) {}, stan.DurableName("dur")); err == nil {
+		t.Fatal("Expected error on subscribe, go none")
+	}
+	check()
+	// And a queue sub
+	if _, err := sc.QueueSubscribe("foo", "queue", func(_ *stan.Msg) {}); err == nil {
+		t.Fatal("Expected error on subscribe, go none")
+	}
+	check()
+	// Finally a durable queue sub
+	if _, err := sc.QueueSubscribe("foo", "queue", func(_ *stan.Msg) {}, stan.DurableName("dur")); err == nil {
+		t.Fatal("Expected error on subscribe, go none")
+	}
+	check()
 }
 
 func TestMaxMsgs(t *testing.T) {