Implement _preorder_depth_first_walk recursively

Use a recursive call of _preorder_depth_first_walk
instead of a stack to implement the same algorithm.

Signed-off-by: Teodora Sechkova <[email protected]>

Author: sechkova

tuf/ngclient/updater.py

@@ -6,7 +6,7 @@
 
 import logging
 import os
-from typing import Any, Dict, List, Optional, Union
+from typing import Any, Dict, List, Optional, Set, Tuple, Union
 from urllib import parse
 
 from securesystemslib import util as sslib_util
@@ -110,7 +110,10 @@ def get_one_valid_targetinfo(
             RepositoryError: Metadata failed to verify in some way
             TODO: download-related errors
         """
-        return self._preorder_depth_first_walk(target_path)
+        targetinfo, dummy = self._preorder_depth_first_walk(
+            target_path, set(), ("targets", "root"), self.config.max_delegations
+        )
+        return targetinfo
 
     @staticmethod
     def updated_targets(
@@ -307,77 +310,63 @@ def _load_targets(self, role: str, parent_role: str) -> None:
             self._persist_metadata(role, data)
 
     def _preorder_depth_first_walk(
-        self, target_filepath: str
-    ) -> Union[Dict[str, Any], None]:
+        self,
+        target_filepath: str,
+        visited_role_names: Set[str],
+        current_role_pair: List[Tuple[str, ...]],
+        number_of_delegations: int,
+    ) -> Tuple[Union[Dict[str, Any], None], bool]:
         """
         Interrogates the tree of target delegations in order of appearance
         (which implicitly order trustworthiness), and returns the matching
         target found in the most trusted role.
         """
-
-        role_names = [("targets", "root")]
-        visited_role_names = set()
-        number_of_delegations = self.config.max_delegations
-
+        targetinfo = None
+        terminated = False
         # Preorder depth-first traversal of the graph of target delegations.
-        while number_of_delegations > 0 and len(role_names) > 0:
-
-            # Pop the role name from the top of the stack.
-            role_name, parent_role = role_names.pop(-1)
-
-            # Skip any visited current role to prevent cycles.
-            if (role_name, parent_role) in visited_role_names:
-                logger.debug("Skipping visited current role %s", role_name)
-                continue
-
-            # The metadata for 'role_name' must be downloaded/updated before
-            # its targets, delegations, and child roles can be inspected.
-            self._load_targets(role_name, parent_role)
-
-            role_metadata: Targets = self._trusted_set[role_name].signed
-            target = role_metadata.targets.get(target_filepath)
-
-            if target is not None:
-                logger.debug("Found target in current role %s", role_name)
-                return {"filepath": target_filepath, "fileinfo": target}
-
-            # After preorder check, add current role to set of visited roles.
-            visited_role_names.add((role_name, parent_role))
-
-            # And also decrement number of visited roles.
-            number_of_delegations -= 1
-
-            if role_metadata.delegations is not None:
-                child_roles_to_visit = []
-                # NOTE: This may be a slow operation if there are many
-                # delegated roles.
-                for child_role in role_metadata.delegations.roles:
-                    if child_role.is_in_trusted_paths(target_filepath):
-                        logger.debug("Adding child role %s", child_role.name)
-
-                        child_roles_to_visit.append(
-                            (child_role.name, role_name)
-                        )
-                        if child_role.terminating:
-                            logger.debug("Not backtracking to other roles.")
-                            role_names = []
-                            break
-                # Push 'child_roles_to_visit' in reverse order of appearance
-                # onto 'role_names'.  Roles are popped from the end of
-                # the 'role_names' list.
-                child_roles_to_visit.reverse()
-                role_names.extend(child_roles_to_visit)
-
-        if number_of_delegations == 0 and len(role_names) > 0:
-            logger.debug(
-                "%d roles left to visit, but allowed to "
-                "visit at most %d delegations.",
-                len(role_names),
-                self.config.max_delegations,
-            )
-
-        # If this point is reached then target is not found, return None
-        return None
+        if number_of_delegations <= 0:
+            return targetinfo, terminated
+
+        # Pop the role name from the top of the stack.
+        role_name, parent_role = current_role_pair
+
+        # The metadata for 'role_name' must be downloaded/updated before
+        # its targets, delegations, and child roles can be inspected.
+        self._load_targets(role_name, parent_role)
+        role_metadata: Targets = self._trusted_set[role_name].signed
+        target = role_metadata.targets.get(target_filepath)
+
+        if target is not None:
+            logger.debug("Found target in current role %s", role_name)
+            targetinfo = {"filepath": target_filepath, "fileinfo": target}
+            return targetinfo, terminated
+
+        # After preorder check, add current role to set of visited roles.
+        visited_role_names.add((role_name, parent_role))
+
+        # And also decrement number of visited roles.
+        number_of_delegations -= 1
+        if role_metadata.delegations is not None:
+            for child_role in role_metadata.delegations.roles:
+                # Skip any visited current role to prevent cycles.
+                if (child_role.name, parent_role) in visited_role_names:
+                    continue
+
+                if child_role.is_in_trusted_paths(target_filepath):
+
+                    targetinfo, terminated = self._preorder_depth_first_walk(
+                        target_filepath,
+                        visited_role_names,
+                        (child_role.name, role_name),
+                        number_of_delegations,
+                    )
+
+                    if child_role.terminating or terminated:
+                        terminated = True
+                        logger.debug("Not backtracking to other roles.")
+                        break
+
+        return targetinfo, terminated
 
 
 def _ensure_trailing_slash(url: str):