From d89538e0375168b23cca72b79cdd9aea9b3deb67 Mon Sep 17 00:00:00 2001
From: Matthias Bernt <m.bernt@ufz.de>
Date: Fri, 14 Jan 2022 11:10:37 +0100
Subject: [PATCH] check for cycles in nested macros

---
 lib/galaxy/util/xml_macros.py           | 18 +++++++++++++----
 test/unit/tool_util/test_tool_loader.py | 27 +++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/lib/galaxy/util/xml_macros.py b/lib/galaxy/util/xml_macros.py
index d126d4950871..beed546463da 100644
--- a/lib/galaxy/util/xml_macros.py
+++ b/lib/galaxy/util/xml_macros.py
@@ -155,7 +155,7 @@ def _expand_tokens_str(s, tokens):
     return s
 
 
-def _expand_macros(elements, macros, tokens):
+def _expand_macros(elements, macros, tokens, visited=None):
     if not macros and not tokens:
         return
 
@@ -164,12 +164,21 @@ def _expand_macros(elements, macros, tokens):
             expand_el = element.find('.//expand')
             if expand_el is None:
                 break
-            _expand_macro(expand_el, macros, tokens)
+            if visited is None:
+                v = set()
+            else:
+                v = visited
+            _expand_macro(expand_el, macros, tokens, v)
 
 
-def _expand_macro(expand_el, macros, tokens):
+def _expand_macro(expand_el, macros, tokens, visited):
     macro_name = expand_el.get('macro')
     assert macro_name is not None, "Attempted to expand macro with no 'macro' attribute defined."
+
+    # check for cycles in the nested macro expansion
+    assert macro_name not in visited, f"Cycle in nested macros {visited}"
+    visited.add(macro_name)
+
     assert macro_name in macros, f"No macro named {macro_name} found, known macros are {', '.join(macros.keys())}."
     macro_def = macros[macro_name]
     expanded_elements = deepcopy(macro_def.element)
@@ -180,8 +189,9 @@ def _expand_macro(expand_el, macros, tokens):
         _expand_tokens(expanded_elements, macro_tokens)
 
     # Recursively expand contained macros.
-    _expand_macros(expanded_elements, macros, tokens)
+    _expand_macros(expanded_elements, macros, tokens, visited)
     _xml_replace(expand_el, expanded_elements)
+    visited.remove(macro_name)
 
 
 def _expand_yield_statements(macro_def, expand_el):
diff --git a/test/unit/tool_util/test_tool_loader.py b/test/unit/tool_util/test_tool_loader.py
index a4bbd7fffc66..efa71b772fc7 100644
--- a/test/unit/tool_util/test_tool_loader.py
+++ b/test/unit/tool_util/test_tool_loader.py
@@ -644,3 +644,30 @@ def test_loader_specify_nested_macro_by_token():
     <A/>
     <B/>
 </tool>'''
+
+
+def test_loader_circular_macros():
+    """
+    check the cycles in nested macros are detected
+    """
+    with TestToolDirectory() as tool_dir:
+        tool_dir.write('''
+<tool>
+    <macros>
+        <xml name="a">
+            <A>
+                <expand macro="b"/>
+            </A>
+        </xml>
+        <xml name="b">
+            <B>
+                <expand macro="a"/>
+            </B>
+        </xml>
+    </macros>
+    <expand macro="a"/>
+</tool>''')
+        try:
+            tool_dir.load()
+        except AssertionError as a:
+            assert str(a) == "Cycle in nested macros {'a', 'b'}"