_encode_uri_path_for_uss acknowledge encoding on a URI given as input

CHANGELOG.md

@@ -10,6 +10,7 @@ All notable changes to the Zowe Client Python SDK will be documented in this fil
 
 ### Bug Fixes
 
+- Fixed `_encode_uri_path_for_uss` and `_encode_uri_path_for_zos` double-encoding paths that were already percent-encoded. Encoded paths are now normalized and validated, but not encoded again. [#415](https://github.com/zowe/zowe-client-python-sdk/pull/415)
 - **Breaking**: Removed support for loading a JSON schema from a remote `http(s)://` URL via the `$schema` config property. Local schema files are still supported. [#412](https://github.com/zowe/zowe-client-python-sdk/pull/412)
 - Fixed secure `user`/`password` properties not being loaded for team-config profiles nested more than one level deep which caused 401 errors. [#411](https://github.com/zowe/zowe-client-python-sdk/pull/411)
 - Redacted request headers and restricted log directory/file to owner-only access. [#404](https://github.com/zowe/zowe-client-python-sdk/pull/404)

src/core/zowe/core_for_zowe_sdk/sdk_api.py

@@ -12,15 +12,14 @@
 
 import copy
 import posixpath
-import urllib
+import urllib.parse
 
 from . import session_constants
 from .logger import Log
 from .request_handler import RequestHandler
 from .session import ISession, Session
 from typing import Any, Optional, Type
 
-# Characters that fail against both z/OSMF and API-ML unless they are encoded.
 _USS_CHARS_TO_ENCODE = {" ": "%20", "%": "%25", "+": "%2B", "?": "%3F"}
 
 # Characters that API-ML rejects with an HTTP 400 unless they are encoded.
@@ -145,6 +144,27 @@ def _is_using_apiml(self) -> bool:
             return True
         return self.session.base_path is not None
 
+    def _is_uri_encoded(self, uri_path: str) -> bool:
+        """
+        Determine whether a path is already percent-encoded.
+
+        A path is treated as encoded when decoding it changes it. A literal percent sign that
+        is not followed by two hex digits, such as the one in the USS file name "100% done",
+        does not decode to anything and so does not make a path encoded.
+
+        Parameters
+        ----------
+        uri_path : str
+            The path to inspect
+
+        Returns
+        -------
+        bool
+            True if the path contains a percent-encoded sequence, False otherwise
+        """
+        # unquote, not unquote_plus: "+" is a literal that USS paths must still encode as %2B
+        return urllib.parse.unquote(uri_path) != uri_path
+
     def _encode_uri_path_for_zos(self, zos_uri_path: str) -> str:
         """
         Encode a z/OS resource (dataset, job, or volser) path for the path component of a URI.
@@ -157,10 +177,13 @@ def _encode_uri_path_for_zos(self, zos_uri_path: str) -> str:
         successfully by z/OSMF. API-ML rejects a literal "#" with an HTTP 400 error unless it is
         encoded, so it is also adjusted here when routed through API-ML.
 
+        A path that is already percent-encoded is normalized but not encoded a second time, so
+        that a caller-encoded "%23" is not turned into "%2523".
+
         Parameters
         ----------
         zos_uri_path : str
-            The URI path to encode
+            The URI path to encode, either unencoded or already percent-encoded
 
         Returns
         -------
@@ -170,6 +193,8 @@ def _encode_uri_path_for_zos(self, zos_uri_path: str) -> str:
         """
         # Normalizing against root collapses ".." segments without escaping the service path
         normalized = posixpath.normpath("/" + zos_uri_path).lstrip("/")
+        if self._is_uri_encoded(normalized):
+            return normalized
         encoded = normalized.replace("?", "%3F")
         if self._is_using_apiml():
             encoded = encoded.replace("#", "%23")
@@ -183,10 +208,15 @@ def _encode_uri_path_for_uss(self, uss_uri_path: str) -> str:
         unless they are encoded. Forward slashes are preserved rather than encoded
         as %2F, since encoded slashes are expected to be rejected in future.
 
+        A path that is already percent-encoded is normalized and validated but not encoded a
+        second time, so that a caller-encoded "%20" is not turned into "%2520". A file name
+        containing a literal percent sign followed by two hex digits is indistinguishable from
+        an encoded path, so such a name must be passed already encoded.
+
         Parameters
         ----------
         uss_uri_path : str
-            The USS path to encode
+            The USS path to encode, either unencoded or already percent-encoded
 
         Returns
         -------
@@ -200,26 +230,29 @@ def _encode_uri_path_for_uss(self, uss_uri_path: str) -> str:
         """
         # Normalizing against root collapses // and resolves /../ without escaping the service path
         normalized = posixpath.normpath("/" + uss_uri_path).lstrip("/")
-        encode_for_apiml = self._is_using_apiml()
 
+        if "\\" in normalized:
+            # Both encoded and unencoded backslashes fail in REST requests
+            self.logger.error(f"The USS path '{uss_uri_path}' contains a backslash character.")
+            raise ValueError(
+                f"The supplied USS path '{uss_uri_path}' contains a backslash \\ character. "
+                "When a backslash is present, z/OSMF and API-ML servers fail with an HTTP 400 "
+                "or 500 error code, or the backslash is ignored. This request was not sent."
+            )
+        if '"' in normalized:
+            # Both encoded and unencoded double-quotes fail in REST requests
+            self.logger.error(f"The USS path '{uss_uri_path}' contains a double-quote character.")
+            raise ValueError(
+                f"The supplied USS path '{uss_uri_path}' contains a double-quote \" character. "
+                "When a double-quote is present, z/OSMF and API-ML servers fail with an HTTP 400 "
+                "or 500 error code. This request was not sent."
+            )
+        if self._is_uri_encoded(normalized):
+            return normalized
+
+        encode_for_apiml = self._is_using_apiml()
         encoded_path = []
         for next_char in normalized:
-            if next_char == "\\":
-                # Both encoded and unencoded backslashes fail in REST requests
-                self.logger.error(f"The USS path '{uss_uri_path}' contains a backslash character.")
-                raise ValueError(
-                    f"The supplied USS path '{uss_uri_path}' contains a backslash \\ character. "
-                    "When a backslash is present, z/OSMF and API-ML servers fail with an HTTP 400 "
-                    "or 500 error code, or the backslash is ignored. This request was not sent."
-                )
-            if next_char == '"':
-                # Both encoded and unencoded double-quotes fail in REST requests
-                self.logger.error(f"The USS path '{uss_uri_path}' contains a double-quote character.")
-                raise ValueError(
-                    f'The supplied USS path \'{uss_uri_path}\' contains a double-quote " character. '
-                    "When a double-quote is present, z/OSMF and API-ML servers fail with an HTTP 400 "
-                    "or 500 error code. This request was not sent."
-                )
             if next_char in _USS_CHARS_TO_ENCODE:
                 encoded_path.append(_USS_CHARS_TO_ENCODE[next_char])
             elif encode_for_apiml and next_char in _APIML_CHARS_TO_ENCODE:

tests/unit/core/test_sdk_api.py

@@ -192,3 +192,56 @@ def test_encode_uri_path_for_uss_rejects_unusable_characters(self):
         with self.assertRaises(ValueError) as double_quote:
             sdk_api._encode_uri_path_for_uss('/u/a"b')
         self.assertIn("double-quote", str(double_quote.exception))
+
+    def test_is_uri_encoded(self):
+        """Only a percent sign followed by two hex digits counts as an encoded sequence."""
+        sdk_api = SdkApi(self.basic_props, self.default_url)
+
+        self.assertTrue(sdk_api._is_uri_encoded("u/my%20file.txt"))
+        self.assertTrue(sdk_api._is_uri_encoded("MY.DS%23NAME"))
+        self.assertTrue(sdk_api._is_uri_encoded("u/a%2fb"))
+        self.assertFalse(sdk_api._is_uri_encoded("u/my file.txt"))
+        self.assertFalse(sdk_api._is_uri_encoded("u/100% done"))
+        self.assertFalse(sdk_api._is_uri_encoded("u/a%zzb"))
+        # A literal "+" must not be mistaken for an encoded space
+        self.assertFalse(sdk_api._is_uri_encoded("u/a+b"))
+
+    def test_encode_uri_path_for_zos_skips_encoded_path(self):
+        """An already-encoded z/OS path must not be encoded a second time."""
+        sdk_api = SdkApi({**self.basic_props, "basePath": "/api/v1"}, self.default_url)
+
+        self.assertEqual(sdk_api._encode_uri_path_for_zos("MY.DS%23NAME$HERE"), "MY.DS%23NAME$HERE")
+        self.assertEqual(sdk_api._encode_uri_path_for_zos("X%3Ffsname=Y"), "X%3Ffsname=Y")
+
+    def test_encode_uri_path_for_zos_resolves_dot_segments_in_encoded_path(self):
+        """An already-encoded z/OS path is still normalized against the service root."""
+        sdk_api = SdkApi(self.basic_props, self.default_url)
+
+        self.assertEqual(
+            sdk_api._encode_uri_path_for_zos("../../restjobs/jobs/OTHER%23JOB/JOB00001"),
+            "restjobs/jobs/OTHER%23JOB/JOB00001",
+        )
+
+    def test_encode_uri_path_for_uss_skips_encoded_path(self):
+        """An already-encoded USS path must not be encoded a second time."""
+        sdk_api = SdkApi(self.basic_props, self.default_url)
+        apiml_api = SdkApi({**self.basic_props, "basePath": "/api/v1"}, self.default_url)
+
+        self.assertEqual(sdk_api._encode_uri_path_for_uss("/u/my%20file.txt"), "u/my%20file.txt")
+        self.assertEqual(sdk_api._encode_uri_path_for_uss("/u/a%25b"), "u/a%25b")
+        self.assertEqual(apiml_api._encode_uri_path_for_uss("/u/a%23b;c"), "u/a%23b;c")
+
+    def test_encode_uri_path_for_uss_normalizes_and_validates_encoded_path(self):
+        """An already-encoded USS path is still normalized and checked for unusable characters."""
+        sdk_api = SdkApi(self.basic_props, self.default_url)
+
+        self.assertEqual(sdk_api._encode_uri_path_for_uss("/u/user/..//other%20dir/f"), "u/other%20dir/f")
+        self.assertEqual(sdk_api._encode_uri_path_for_uss("/u/a/../../../../etc/pass%20wd"), "etc/pass%20wd")
+
+        with self.assertRaises(ValueError) as backslash:
+            sdk_api._encode_uri_path_for_uss("/u/a%20b\\c")
+        self.assertIn("backslash", str(backslash.exception))
+
+        with self.assertRaises(ValueError) as double_quote:
+            sdk_api._encode_uri_path_for_uss('/u/a%20b"c')
+        self.assertIn("double-quote", str(double_quote.exception))