Fail lookups on invalid resource names

ament_index_python/ament_index_python/__init__.py

@@ -21,6 +21,8 @@
 from .resources import get_resource_types
 from .resources import get_resources
 from .resources import has_resource
+from .resources import InvalidResourceNameError
+from .resources import InvalidResourceTypeNameError
 from .search_paths import get_search_paths
 
 __all__ = [
@@ -32,6 +34,8 @@
     'get_search_paths',
     'get_resource_types',
     'has_resource',
+    'InvalidResourceTypeNameError',
+    'InvalidResourceNameError',
     'PackageNotFoundError',
     'RESOURCE_INDEX_SUBFOLDER',
 ]

ament_index_python/ament_index_python/packages.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 
 import os
+import re
 
 from .resources import get_resource
 from .resources import get_resources
@@ -44,7 +45,11 @@ def get_package_prefix(package_name):
     :param str package_name: name of the package to locate
     :returns: installation prefix of the package
     :raises: :exc:`PackageNotFoundError` if the package is not found
+    :raises: :exc:`ValueError` if the package name is invalid
     """
+    if re.fullmatch('[a-z][a-z0-9_]+', package_name, re.ASCII) is None:
+        raise ValueError(
+            "'{}' is not a valid package name".format(package_name))
     try:
         content, package_prefix = get_resource('packages', package_name)
     except LookupError:
@@ -65,5 +70,6 @@ def get_package_share_directory(package_name):
     :param str package_name: name of the package to locate
     :returns: share directory of the package
     :raises: :exc:`PackageNotFoundError` if the package is not found
+    :raises: :exc:`ValueError` if the package name is invalid
     """
     return os.path.join(get_package_prefix(package_name), 'share', package_name)

ament_index_python/ament_index_python/resources.py

@@ -19,6 +19,18 @@
 from .search_paths import get_search_paths
 
 
+class InvalidResourceTypeNameError(ValueError):
+    pass
+
+
+class InvalidResourceNameError(ValueError):
+    pass
+
+
+def name_is_invalid(resource_name):
+    return ('/' in resource_name) or ('\\' in resource_name)
+
+
 def get_resource(resource_type, resource_name):
     """
     Get the content of a specific resource and its prefix path.
@@ -31,9 +43,17 @@ def get_resource(resource_type, resource_name):
     :raises: :exc:`EnvironmentError`
     :raises: :exc:`OSError`
     :raises: :exc:`LookupError`
+    :raises: :exc:`InvalidResourceTypeNameError`
+    :raises: :exc:`InvalidResourceNameError`
     """
     assert resource_type, 'The resource type must not be empty'
     assert resource_name, 'The resource name must not be empty'
+    if name_is_invalid(resource_type):
+        raise InvalidResourceTypeNameError(
+            "Resource type '%s' is invalid" % resource_type)
+    if name_is_invalid(resource_name):
+        raise InvalidResourceNameError(
+            "Resource name '%s' is invalid" % resource_name)
     for path in get_search_paths():
         resource_path = os.path.join(path, RESOURCE_INDEX_SUBFOLDER, resource_type, resource_name)
         if os.path.isfile(resource_path):
@@ -57,8 +77,12 @@ def get_resources(resource_type):
     :type resource_type: str
     :returns: dict of resource names to the prefix path they are in
     :raises: :exc:`EnvironmentError`
+    :raises: :exc:`InvalidResourceTypeNameError`
     """
     assert resource_type, 'The resource type must not be empty'
+    if name_is_invalid(resource_type):
+        raise InvalidResourceTypeNameError(
+            "Resource type '%s' is invalid" % resource_type)
     resources = {}
     for path in get_search_paths():
         resource_path = os.path.join(path, RESOURCE_INDEX_SUBFOLDER, resource_type)
@@ -103,9 +127,17 @@ def has_resource(resource_type, resource_name):
     :type resource_name: str
     :returns: The prefix path if the resource exists, False otherwise
     :raises: :exc:`EnvironmentError`
+    :raises: :exc:`InvalidResourceTypeNameError`
+    :raises: :exc:`InvalidResourceNameError`
     """
     assert resource_type, 'The resource type must not be empty'
     assert resource_name, 'The resource name must not be empty'
+    if name_is_invalid(resource_type):
+        raise InvalidResourceTypeNameError(
+            "Resource type '%s' is invalid" % resource_type)
+    if name_is_invalid(resource_name):
+        raise InvalidResourceNameError(
+            "Resource name '%s' is invalid" % resource_name)
     for path in get_search_paths():
         resource_path = os.path.join(path, RESOURCE_INDEX_SUBFOLDER, resource_type, resource_name)
         if os.path.isfile(resource_path):

ament_index_python/test/test_ament_index_python.py

@@ -23,6 +23,8 @@
 from ament_index_python import get_resources
 from ament_index_python import get_search_paths
 from ament_index_python import has_resource
+from ament_index_python import InvalidResourceNameError
+from ament_index_python import InvalidResourceTypeNameError
 from ament_index_python import PackageNotFoundError
 from ament_index_python.cli import main
 from ament_index_python.cli import resource_name_completer
@@ -66,6 +68,25 @@ def test_unknown_resources():
     assert len(resources) == 0, 'Expected no resources'
 
 
+def test_invalid_resources():
+    set_ament_prefix_path(['prefix1'])
+
+    invalid_resource_type_names = [
+        '/invalid/name', 'invalid/name', '\\invalid\\name', 'invalid\\name']
+
+    for name in invalid_resource_type_names:
+        with pytest.raises(InvalidResourceTypeNameError):
+            resources = get_resources(name)
+            assert len(resources) == 0, 'Expected no resources'
+
+        with pytest.raises(InvalidResourceTypeNameError):
+            exists = has_resource(name, 'example_resource')
+            assert not exists, 'Resource should not exist'
+
+        with pytest.raises(InvalidResourceTypeNameError):
+            get_resource(name, 'example_resource')
+
+
 def test_resources():
     set_ament_prefix_path(['prefix1'])
     resources = get_resources('resource_type1')
@@ -96,6 +117,33 @@ def test_unknown_resource():
         get_resource('resource_type4', 'bar')
 
 
+def test_invalid_resource_names():
+    set_ament_prefix_path(['prefix1'])
+
+    invalid_resource_names = [
+        '/invalid/name', 'invalid/name', '\\invalid\\name', 'invalid\\name']
+
+    for name in invalid_resource_names:
+        with pytest.raises(InvalidResourceNameError):
+            exists = has_resource('resource_type4', name)
+            assert not exists, 'Resource should not exist'
+
+        with pytest.raises(InvalidResourceNameError):
+            get_resource('resource_type4', name)
+
+
+def test_absolute_path_resource():
+    extant_absolute_path = os.path.abspath(__file__)
+
+    set_ament_prefix_path(['prefix1'])
+    with pytest.raises(InvalidResourceNameError):
+        exists = has_resource('resource_type4', str(extant_absolute_path))
+        assert not exists, 'Resource should not exist'
+
+    with pytest.raises(InvalidResourceNameError):
+        get_resource('resource_type4', str(extant_absolute_path))
+
+
 def test_resource():
     set_ament_prefix_path(['prefix1'])
     exists = has_resource('resource_type4', 'foo')
@@ -144,6 +192,17 @@ def get_package_prefix_basename(package_name):
         get_package_prefix('does_not_exist')
     assert issubclass(PackageNotFoundError, KeyError)
 
+    invalid_package_names = [
+        '_package', 'packageA', 'package a', 'package/a', '0package', 'package.a']
+    for name in invalid_package_names:
+        with pytest.raises(ValueError):
+            get_package_prefix(name)
+
+    with pytest.raises(ValueError):
+        # An absolue path is not a valid package name
+        extant_absolute_path = os.path.abspath(__file__)
+        get_package_prefix(extant_absolute_path)
+
 
 def test_get_package_share_directory():
     set_ament_prefix_path(['prefix1', 'prefix2'])
@@ -165,6 +224,9 @@ def get_package_share_directory_test(package_name, expect_prefix):
     with pytest.raises(PackageNotFoundError):
         get_package_share_directory('does_not_exist')
 
+    with pytest.raises(ValueError):
+        get_package_share_directory('/invalid/package/name')
+
 
 def test_get_resource_types():
     set_ament_prefix_path([])