avoid endless recursion (#43)

avoid endless recursion when cyclic exchange-to-exchange bindings are encountered (fixes #42)

CHANGELOG.md

@@ -1,6 +1,10 @@
 
 # Changelog for rabtap
 
+## v1.23 (2020-04-09)
+
+* fix: avoid endless recursion in info command (#42)
+
 ## v1.22 (2020-01-28)
 
 * The `pub` command now allows ialso to replay messages from a direcory previously

cmd/rabtap/broker_info_tree_builder.go

@@ -216,31 +216,48 @@ func (s defaultBrokerInfoTreeBuilder) createQueueNodeFromBinding(
 func (s defaultBrokerInfoTreeBuilder) createExchangeNode(
 	exchange rabtap.RabbitExchange, brokerInfo rabtap.BrokerInfo) *exchangeNode {
 
-	exchangeNode := exchangeNode{baseNode{[]interface{}{}}, exchange}
-
-	// process all bindings for current exchange
-	for _, binding := range rabtap.FindBindingsForExchange(exchange, brokerInfo.Bindings) {
-		if binding.DestinationType == "exchange" {
-			// exchange to exchange binding
-			i := rabtap.FindExchangeByName(
-				brokerInfo.Exchanges,
-				binding.Vhost,
-				binding.Destination)
-			if i != -1 {
-				exchangeNode.Add(
-					s.createExchangeNode(
-						brokerInfo.Exchanges[i],
-						brokerInfo))
-			} // TODO else log error
-		} else {
-			// queue to exchange binding
-			queues := s.createQueueNodeFromBinding(binding, exchange, brokerInfo)
-			for _, queue := range queues {
-				exchangeNode.Add(queue)
+	// to detect cyclic exchange-to-exchange bindings. Yes, this is possible.
+	visited := map[string]bool{}
+
+	var create func(rabtap.RabbitExchange, rabtap.BrokerInfo) *exchangeNode
+	create = func(exchange rabtap.RabbitExchange, brokerInfo rabtap.BrokerInfo) *exchangeNode {
+
+		exchangeNode := exchangeNode{baseNode{[]interface{}{}}, exchange}
+
+		// process all bindings for current exchange
+		for _, binding := range rabtap.FindBindingsForExchange(exchange, brokerInfo.Bindings) {
+			if binding.DestinationType == "exchange" {
+				// exchange to exchange binding
+				i := rabtap.FindExchangeByName(
+					brokerInfo.Exchanges,
+					binding.Vhost,
+					binding.Destination)
+				if i == -1 {
+					// ignore if not found
+					continue
+				}
+				boundExchange := brokerInfo.Exchanges[i]
+				if _, found := visited[boundExchange.Name]; found {
+					// cyclic exchange-to-exchange binding detected
+					continue
+				}
+				visited[boundExchange.Name] = true
+				exchangeNode.Add(create(exchange, brokerInfo))
+			} else {
+				// do not add (redundant) queues if in recursive exchange call
+				if len(visited) > 0 {
+					continue
+				}
+				// queue to exchange binding
+				queues := s.createQueueNodeFromBinding(binding, exchange, brokerInfo)
+				for _, queue := range queues {
+					exchangeNode.Add(queue)
+				}
 			}
 		}
+		return &exchangeNode
 	}
-	return &exchangeNode
+	return create(exchange, brokerInfo)
 }
 
 func (s defaultBrokerInfoTreeBuilder) createRootNode(rootNodeURL string,

cmd/rabtap/cmd_info_test.go

@@ -72,7 +72,8 @@ func Example_cmdInfoByExchangeInTextFormat() {
 	//     │   └── header-q2 (queue, key='headers-q2', idle since 2017-05-25 19:14:47, [D])
 	//     └── test-topic (exchange, type 'topic', [D])
 	//         ├── topic-q1 (queue, key='topic-q1', idle since 2017-05-25 19:14:17, [D|AD|EX])
-	//         └── topic-q2 (queue, key='topic-q2', idle since 2017-05-25 19:14:21, [D])
+	//         ├── topic-q2 (queue, key='topic-q2', idle since 2017-05-25 19:14:21, [D])
+	//         └── test-topic (exchange, type 'topic', [D])
 
 }
 
@@ -182,12 +183,16 @@ const expectedResultDotByExchange = `graph broker {
 
 "exchange_test-topic" -- "boundqueue_topic-q1" [fontsize=10; headport=n; label="topic-q1"];
 "exchange_test-topic" -- "boundqueue_topic-q2" [fontsize=10; headport=n; label="topic-q2"];
+"exchange_test-topic" -- "exchange_test-topic" [fontsize=10; headport=n; label=""];
 
 "boundqueue_topic-q1" [shape="record"; label="{ topic-q1 | { D  | AD  | EX  } }"];
 
 
 "boundqueue_topic-q2" [shape="record"; label="{ topic-q2 | { D  | | } }"];
 
+
+"exchange_test-topic" [shape="record"; label="{ test-topic |topic | { D  | | } }"];
+
 }`
 
 func TestCmdInfoByExchangeInDotFormat(t *testing.T) {

pkg/rabbitmq_rest_client_test.go

@@ -28,7 +28,7 @@ func TestGetAllResources(t *testing.T) {
 	assert.Equal(t, 1, len(all.Connections))
 	assert.Equal(t, 12, len(all.Exchanges))
 	assert.Equal(t, 8, len(all.Queues))
-	assert.Equal(t, 16, len(all.Bindings))
+	assert.Equal(t, 17, len(all.Bindings))
 	assert.Equal(t, 2, len(all.Consumers))
 }
 

pkg/testcommon/rabbitmq_rest_api_mock.go

@@ -270,6 +270,17 @@ const (
 
         },
         "properties_key": "topic-q2"
+    },
+    {
+        "source": "test-topic",
+        "vhost": "/",
+        "destination": "test-topic",
+        "destination_type": "exchange",
+        "routing_key": "",
+        "arguments": {
+
+        },
+        "properties_key": "~"
     }
 ]`