From fc2bff63b320dc2a54458d1a78bc8a41a3f60879 Mon Sep 17 00:00:00 2001
From: Nicola Soranzo <nicola.soranzo@earlham.ac.uk>
Date: Sat, 27 Apr 2019 00:00:01 +0100
Subject: [PATCH] Remove recursion from ``expand_nested_tokens()``

Also add test for recursive nested token.
---
 lib/galaxy/util/xml_macros.py   | 30 +++++++++++-------------------
 test/unit/tools/test_parsing.py | 20 ++++++++++++++++++++
 2 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/lib/galaxy/util/xml_macros.py b/lib/galaxy/util/xml_macros.py
index 540b7a8cfbc..b02796080e1 100644
--- a/lib/galaxy/util/xml_macros.py
+++ b/lib/galaxy/util/xml_macros.py
@@ -83,21 +83,13 @@ def _macros_of_type(root, type, el_func):
     return macro_dict
 
 
-def expand_nested_tokens(tokens, restarts=10):
-    token_copy = tokens.copy()
-    token_changed = False
-    for token_name in token_copy.keys():
-        for current_token_name, current_token_value in token_copy.items():
+def expand_nested_tokens(tokens):
+    for token_name in tokens.keys():
+        for current_token_name, current_token_value in tokens.items():
             if token_name in current_token_value:
-                current_token_value = current_token_value.replace(token_name, token_copy[token_name])
-                tokens[current_token_name] = current_token_value
-                # We changed a token, so we need to restart
-                token_changed = True
-    if token_changed:
-        if restarts > 0:
-            expand_nested_tokens(tokens, restarts=restarts - 1)
-        else:
-            raise Exception("Tokens are nested too deep")
+                if token_name == current_token_name:
+                    raise Exception("Token '%s' cannot contain itself" % token_name)
+                tokens[current_token_name] = current_token_value.replace(token_name, tokens[token_name])
     return tokens
 
 
@@ -122,11 +114,11 @@ def _expand_tokens_for_el(element, tokens):
     _expand_tokens(list(element), tokens)
 
 
-def _expand_tokens_str(str, tokens):
+def _expand_tokens_str(s, tokens):
     for key, value in tokens.items():
-        if str.find(key) > -1:
-            str = str.replace(key, value)
-    return str
+        if key in s:
+            s = s.replace(key, value)
+    return s
 
 
 def _expand_macros(elements, macros, tokens):
@@ -158,7 +150,7 @@ def _expand_macro(element, expand_el, macros, tokens):
 
     # HACK for elementtree, newer implementations (etree/lxml) won't
     # require this parent_map data structure but elementtree does not
-    # track parents or recongnize .find('..').
+    # track parents or recognize .find('..').
     # TODO fix this now that we're not using elementtree
     parent_map = dict((c, p) for p in element.iter() for c in p)
     _xml_replace(expand_el, expanded_elements, parent_map)
diff --git a/test/unit/tools/test_parsing.py b/test/unit/tools/test_parsing.py
index 0f58c7504ef..1ca7308d27a 100644
--- a/test/unit/tools/test_parsing.py
+++ b/test/unit/tools/test_parsing.py
@@ -61,6 +61,22 @@ TOOL_WITH_TOKEN = r"""
 </tool>
 """
 
+TOOL_WITH_RECURSIVE_TOKEN = r"""
+<tool id="tool_with_recursive_token" name="Token" version="1">
+    <macros>
+        <token name="@NESTED_TOKEN@">
+<![CDATA[
+    before
+    @NESTED_TOKEN@
+    after
+        ]]></token>
+    </macros>
+    <command>
+@NESTED_TOKEN@
+    </command>
+</tool>
+"""
+
 TOOL_YAML_1 = """
 name: "Bowtie Mapper"
 class: GalaxyTool
@@ -279,6 +295,10 @@ class XmlLoaderTestCase(BaseLoaderTestCase):
         assert command
         assert '@' not in command
 
+    def test_recursive_token(self):
+        with self.assertRaises(Exception):
+            self._get_tool_source(source_contents=TOOL_WITH_RECURSIVE_TOKEN)
+
 
 class YamlLoaderTestCase(BaseLoaderTestCase):
     source_file_name = "bwa.yml"