Make methods adding targets consistent

Adopt the same path verification strategy in all methods
of Targets class that add new target files by utilizing
the _check_relpath() method.

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

Author: sechkova

tuf/repository_tool.py

@@ -1896,22 +1896,26 @@ def add_paths(self, paths, child_rolename):
   def add_target(self, filepath, custom=None):
     """
     <Purpose>
-      Add a filepath (must be located in the repository's targets directory) to
-      the Targets object.
+      Add a filepath (must be relative to the repository's targets directory)
+      to the Targets object.
 
-      This method does not actually create 'filepath' on the file system.
-      'filepath' must already exist on the file system.  If 'filepath'
-      has already been added, it will be replaced with any new file
-      or 'custom' information.
+      This method does not access the file system. 'filepath' must already
+      exist on the file system.
+
+      If 'filepath' does not exist the file will still be added to 'roleinfo'.
+      Only later calls to write() and writeall() will fail.
+
+      If 'filepath' has already been added, it will be replaced with any new
+      file or 'custom' information.
 
       >>>
       >>>
       >>>
 
     <Arguments>
       filepath:
-        The path of the target file.  It must exist in the repository's targets
-        directory.
+        The path of the target file.  It must be relative to the repository's
+        targets directory.
 
       custom:
         An optional object providing additional information about the file.
@@ -1920,8 +1924,8 @@ def add_target(self, filepath, custom=None):
       securesystemslib.exceptions.FormatError, if 'filepath' is improperly
       formatted.
 
-      securesystemslib.exceptions.Error, if 'filepath' is not located in the
-      repository's targets directory.
+      tuf.exceptions.InvalidNameError, if 'filepath' is not relative (starts
+      with a directory separator).
 
     <Side Effects>
       Adds 'filepath' to this role's list of targets.  This role's
@@ -1935,47 +1939,38 @@ def add_target(self, filepath, custom=None):
     # Ensure the arguments have the appropriate number of objects and object
     # types, and that all dict keys are properly named.  Raise
     # 'securesystemslib.exceptions.FormatError' if there is a mismatch.
-    securesystemslib.formats.PATH_SCHEMA.check_match(filepath)
+    tuf.formats.RELPATH_SCHEMA.check_match(filepath)
 
     if custom is None:
       custom = {}
-
     else:
       tuf.formats.CUSTOM_SCHEMA.check_match(custom)
 
-    filepath = os.path.join(self._targets_directory, filepath)
-
     # Add 'filepath' (i.e., relative to the targets directory) to the role's
     # list of targets.  'filepath' will not be verified as an allowed path
     # according to some delegating role.  Not verifying 'filepath' here allows
     # freedom to add targets and parent restrictions in any order, minimize the
     # number of times these checks are performed, and allow any role to
-    # delegate trust of packages to this Targes role.
-    if os.path.isfile(filepath):
-
-      # Update the role's 'tuf.roledb.py' entry and avoid duplicates.  Make
-      # sure to exclude the path separator when calculating the length of the
-      # targets directory.
-      targets_directory_length = len(self._targets_directory) + 1
-      roleinfo = tuf.roledb.get_roleinfo(self._rolename, self._repository_name)
-      relative_path = filepath[targets_directory_length:].replace('\\', '/')
+    # delegate trust of packages to this Targets role.
 
-      if relative_path not in roleinfo['paths']:
-        logger.debug('Adding new target: ' + repr(relative_path))
-        roleinfo['paths'].update({relative_path: custom})
-
-      else:
-        logger.debug('Replacing target: ' + repr(relative_path))
-        roleinfo['paths'].update({relative_path: custom})
+    # Check if the target path is relative and normalize it. File's existence
+    # on the file system is not verified. If the file does not exist relative
+    # to the targets directory, later calls to write() will fail.
+    relative_path = self._check_relpath(filepath)
 
+    # Update the role's 'tuf.roledb.py' entry and avoid duplicates.
+    roleinfo = tuf.roledb.get_roleinfo(self._rolename, self._repository_name)
 
-      tuf.roledb.update_roleinfo(self._rolename, roleinfo,
-          repository_name=self._repository_name)
+    if relative_path not in roleinfo['paths']:
+      logger.debug('Adding new target: ' + repr(relative_path))
+      roleinfo['paths'].update({relative_path: custom})
 
     else:
-      raise securesystemslib.exceptions.Error(repr(filepath) + ' is not'
-          ' a valid file in the repository\'s targets'
-          ' directory: ' + repr(self._targets_directory))
+      logger.debug('Replacing target: ' + repr(relative_path))
+      roleinfo['paths'].update({relative_path: custom})
+
+    tuf.roledb.update_roleinfo(self._rolename, roleinfo,
+      repository_name=self._repository_name)
 
 
 
@@ -1999,9 +1994,8 @@ def add_targets(self, list_of_targets):
       securesystemslib.exceptions.FormatError, if the arguments are improperly
       formatted.
 
-      securesystemslib.exceptions.Error, if any of the paths listed in
-      'list_of_targets' is not located in the repository's targets directory or
-      is invalid.
+      tuf.exceptions.InvalidNameError, if any target in 'list_of_targets'
+      is not relative (starts with a directory separator).
 
     <Side Effects>
       This Targets' roleinfo is updated with the paths in 'list_of_targets'.
@@ -2017,7 +2011,6 @@ def add_targets(self, list_of_targets):
     tuf.formats.RELPATHS_SCHEMA.check_match(list_of_targets)
 
     # Update the tuf.roledb entry.
-    targets_directory_length = len(self._targets_directory)
     relative_list_of_targets = []
 
     # Ensure the paths in 'list_of_targets' are valid and are located in the
@@ -2027,15 +2020,7 @@ def add_targets(self, list_of_targets):
     # freedom to add targets and parent restrictions in any order, and minimize
     # the number of times these checks are performed.
     for target in list_of_targets:
-      filepath = os.path.join(self._targets_directory, target)
-
-      if os.path.isfile(filepath):
-        relative_list_of_targets.append(
-            filepath[targets_directory_length + 1:].replace('\\', '/'))
-
-      else:
-        raise securesystemslib.exceptions.Error(repr(filepath) + ' is not'
-          ' a valid file.')
+      relative_list_of_targets.append(self._check_relpath(target))
 
     # Update this Targets 'tuf.roledb.py' entry.
     roleinfo = tuf.roledb.get_roleinfo(self._rolename, self._repository_name)
@@ -2223,12 +2208,11 @@ def delegate(self, rolename, public_keys, paths, threshold=1,
       securesystemslib.exceptions.FormatError, if any of the arguments are
       improperly formatted.
 
-      securesystemslib.exceptions.Error, if the delegated role already exists
-      or if any target in 'list_of_targets' is an invalid path (i.e., not
-      located in the repository's targets directory).
+      securesystemslib.exceptions.Error, if the delegated role already exists.
 
-      tuf.exceptions.InvalidNameError, if any path in 'paths' is not a
-      relative path (starts with a directory separator).
+      tuf.exceptions.InvalidNameError, if any path in 'paths' or any
+      target in 'list_of_targets' is not relative (starts with a directory
+      separator).
 
     <Side Effects>
       A new Target object is created for 'rolename' that is accessible to the
@@ -2274,16 +2258,15 @@ def delegate(self, rolename, public_keys, paths, threshold=1,
     # Ensure the paths of 'list_of_targets' are located in the repository's
     # targets directory.
     relative_targetpaths = {}
-    targets_directory_length = len(self._targets_directory)
 
     if list_of_targets:
       for target in list_of_targets:
-        target = os.path.join(self._targets_directory, target)
-        if not os.path.isfile(target):
-          logger.warning(repr(target) + ' does not exist in the'
-              ' repository\'s targets directory: ' + repr(self._targets_directory))
-
-        relative_targetpaths.update({target[targets_directory_length:]: {}})
+        # Check if the target path is relative and normalize it. File's
+        # existence on the file system is not verified. If the file does not
+        # exist relative to the targets directory, later calls to write()
+        # will fail.
+        rel_targetpath = self._check_relpath(target)
+        relative_targetpaths.update({rel_targetpath: {}})
 
     # A list of relative and verified paths or glob patterns to be added to the
     # child role's entry in the parent's delegations field.
@@ -2468,9 +2451,12 @@ def delegate_hashed_bins(self, list_of_targets, keys_of_hashed_bins,
       formatted.
 
       securesystemslib.exceptions.Error, if 'number_of_bins' is not a power of
-      2, or one of the targets in 'list_of_targets' is not located in the
+      2, or one of the targets in 'list_of_targets' is not relative to the
       repository's targets directory.
 
+      tuf.exceptions.InvalidNameError, if any target in 'list_of_targets'
+      is not relative (starts with a directory separator).
+
     <Side Effects>
       Delegates multiple target roles from the current parent role.
 
@@ -2518,17 +2504,12 @@ def delegate_hashed_bins(self, list_of_targets, keys_of_hashed_bins,
     for bin_index in six.moves.xrange(total_hash_prefixes):
       target_paths_in_bin[bin_index] = []
 
-    # Assign every path to its bin.  Log a warning if the target path does not
-    # exist in the repository's targets directory.
+    # Assign every path to its bin.
     for target_path in list_of_targets:
-      if not os.path.isfile(os.path.join(self._targets_directory, target_path)):
-        logger.warning('A path in "list of'
-            ' targets" is not located in the repository\'s targets'
-            ' directory: ' + repr(target_path))
-
-      else:
-        logger.debug(repr(target_path) + ' is located in the repository\'s'
-            ' targets directory.')
+      # Check if the target path is relative and normalize it. File's existence
+      # on the file system is not verified. If the file does not exist relative
+      # to the targets directory, later calls to write() will fail.
+      target_path = self._check_relpath(target_path)
 
       # Determine the hash prefix of 'target_path' by computing the digest of
       # its path relative to the targets directory.  Example:
@@ -2736,11 +2717,10 @@ def _locate_and_update_target_in_bin(self, target_filepath, method_name):
       raise securesystemslib.exceptions.Error(self.rolename + ' has not'
         ' delegated to hashed bins.')
 
-    # Log warning if 'target_filepath' is not located in the repository's
-    # targets directory.
-    if not os.path.isfile(os.path.join(self._targets_directory, target_filepath)):
-      logger.warning(repr(target_filepath) + ' is not located in the'
-          ' repository\'s targets directory: ' + repr(self._targets_directory))
+    # Check if the target path is relative and normalize it. File's existence
+    # on the file system is not verified. If the file does not exist relative
+    # to the targets directory, later calls to write() will fail.
+    target_filepath = self._check_relpath(target_filepath)
 
     # Determine the hash prefix of 'target_path' by computing the digest of
     # its path relative to the targets directory.  Example: