fix: ensure partially encoded path parameters are fully URL-encoded (#1762)

Previously, `encodeParam` / `encode_path_param` checked whether
`decode(value) === value` before URL-encoding. If a parameter value
contained both already percent-encoded sequences (such as `%20`) and
unencoded reserved characters (such as `+` or `?`), the equality check
evaluated to `false` and skipped encoding the remaining reserved
characters.

This change normalizes the input by decoding first and always
URL-encoding the decoded result.

Author: popmonkey

csharp/rtl.Tests/SdkUtilsTests.cs

@@ -147,6 +147,7 @@ public void EncodeParamTest()
             // Assert.Equal("2020-01-01T14%3A48%3A00.000Z", Transport.EncodeParam(date));
             Assert.Equal("foo%2Fbar", SdkUtils.EncodeParam("foo%2Fbar"));
             Assert.Equal("foo%2Fbar", SdkUtils.EncodeParam("foo/bar"));
+            Assert.Equal("foo%2Fbar%3Fbaz+qux", SdkUtils.EncodeParam("foo/bar?baz%20qux"));
             var actual = SdkUtils.EncodeParam(true);
             Assert.Equal("true", actual);
             actual = SdkUtils.EncodeParam(2.3);

csharp/rtl/SdkUtils.cs

@@ -114,10 +114,7 @@ public static string EncodeParam(object value)
             }
 
             var decoded = WebUtility.UrlDecode(encoded);
-            if (encoded == decoded)
-            {
-                encoded = WebUtility.UrlEncode(encoded);
-            }
+            encoded = WebUtility.UrlEncode(decoded);
 
             return encoded;
         }

kotlin/src/main/com/looker/rtl/Transport.kt

@@ -225,9 +225,7 @@ fun encodeParam(value: Any?): String {
         }
     try {
         val decoded = URLDecoder.decode(encoded, utf8)
-        if (encoded == decoded) {
-            encoded = URLEncoder.encode(encoded, utf8)
-        }
+        encoded = URLEncoder.encode(decoded, utf8)
     } catch (e: IllegalArgumentException) {
         encoded = URLEncoder.encode(encoded, utf8)
     } catch (e: Exception) {

kotlin/src/test/TestTransport.kt

@@ -80,6 +80,7 @@ class TestTransport {
         assertEquals("2020-01-01T14%3A48%3A00.000Z", encodeParam(today))
         assertEquals("foo%2Fbar", encodeParam("foo%2Fbar"))
         assertEquals("foo%2Fbar", encodeParam("foo/bar"))
+        assertEquals("foo%2Fbar%3Fbaz+qux", encodeParam("foo/bar?baz%20qux"))
         assertEquals("true", encodeParam(true))
         assertEquals("2.3", encodeParam(2.3))
     }

packages/sdk-rtl/src/transport.spec.ts

@@ -91,10 +91,11 @@ describe('Transport', () => {
     expect(encodeParam(today)).toEqual('2020-01-01T14%3A48%3A00.000Z');
     expect(encodeParam('foo%2Fbar')).toEqual('foo%2Fbar');
     expect(encodeParam('foo/bar')).toEqual('foo%2Fbar');
+    expect(encodeParam('foo/bar?baz%20qux')).toEqual('foo%2Fbar%3Fbaz%20qux');
     expect(encodeParam(true)).toEqual('true');
     expect(encodeParam(2.3)).toEqual('2.3');
     expect(encodeParam({ created_date: 'this year to second' })).toEqual(
-      '{"created_date":"this year to second"}'
+      '%7B%22created_date%22%3A%22this%20year%20to%20second%22%7D'
     );
   });
 });

packages/sdk-rtl/src/transport.ts

@@ -603,13 +603,11 @@ export function encodeParam(value: any) {
   // decodeURIComponent throws URIError if there is a % character
   // without it being part of an encoded
   try {
-    const decoded = decodeURIComponent(value);
-    if (value === decoded) {
-      encoded = encodeURIComponent(value);
-    }
+    const decoded = decodeURIComponent(encoded);
+    encoded = encodeURIComponent(decoded);
   } catch (e) {
     if (e instanceof URIError) {
-      encoded = encodeURIComponent(value);
+      encoded = encodeURIComponent(encoded);
     } else {
       throw e;
     }

python/looker_sdk/rtl/api_methods.py

@@ -133,9 +133,7 @@ def _convert_query_params(
 
     @staticmethod
     def encode_path_param(value: str) -> str:
-        if value == urllib.parse.unquote(value):
-            value = urllib.parse.quote(value, safe="")
-        return value
+        return urllib.parse.quote(urllib.parse.unquote(value), safe="")
 
     def get(
         self,

python/tests/rtl/test_api_methods.py

@@ -214,3 +214,16 @@ def test_api_versioned_url_is_built_properly(
     api: api_methods.APIMethods, method_path: str, expected_url: str
 ):
     assert api._path(method_path).endswith(expected_url)
+
+
+@pytest.mark.parametrize(
+    "value, expected",
+    [
+        ("foo/bar", "foo%2Fbar"),
+        ("foo%2Fbar", "foo%2Fbar"),
+        ("foo/bar?baz%20qux", "foo%2Fbar%3Fbaz%20qux"),
+        ("foo/bar%20baz", "foo%2Fbar%20baz"),
+    ],
+)
+def test_encode_path_param(api: api_methods.APIMethods, value: str, expected: str):
+    assert api.encode_path_param(value) == expected