check for cycles in nested macros

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 = list()
+            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: already expanded {visited} can't expand '{macro_name}' again"
+    visited.append(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)
+    del visited[-1]
 
 
 def _expand_yield_statements(macro_def, expand_el):

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: already expanded ['a', 'b'] can't expand 'a' again"