[#387] Do not escape the "timeout" parameter. (#394)

* [#387] Do not escape the "timeout" parameter.

When the "timeout" parameter is escaped and turned into
a string, it does not pass validation checks.
This PR ensures that timeout is passed through as-is,
just like "request_timeout".
There's no explicit test for this parameter, but I included
it on some existing tests just to ensure that the code
is exercised.  Without the fix in place, one of the tests as modified will not pass.

Signed-off-by: Charles Greer <[email protected]>

* Unit test for timeout parameter

Revert original test modifications.

Signed-off-by: Charles Greer <[email protected]>

* Extend test and linter

Signed-off-by: Charles Greer <[email protected]>

---------

Signed-off-by: Charles Greer <[email protected]>

CHANGELOG.md

@@ -73,6 +73,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Updates `master` to `cluster_manager` to be inclusive ([#242](https://github.com/opensearch-project/opensearch-py/pull/242))
 - Support a custom signing service name for AWS SigV4 ([#268](https://github.com/opensearch-project/opensearch-py/pull/268))
 - Updated CI tests to make them work locally ([#275](https://github.com/opensearch-project/opensearch-py/pull/275))
+- Fix bug with validation of 'timeout' parameter ([#387](Do not escape the "timeout" parameter.))
 ### Deprecated
 
 ### Removed
@@ -87,3 +88,4 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 [2.1.0]: https://github.com/opensearch-project/opensearch-py/compare/v2.0.1...v2.1.0
 [2.1.1]: https://github.com/opensearch-project/opensearch-py/compare/v2.1.0...v2.1.1
 [2.2.0]: https://github.com/opensearch-project/opensearch-py/compare/v2.1.1...v2.2.0
+

opensearchpy/client/utils.py

@@ -165,16 +165,17 @@ def query_params(*opensearch_query_params):
             elif api_key is not None:
                 headers["authorization"] = "ApiKey %s" % (_base64_auth_header(api_key),)
 
+            # don't escape ignore, request_timeout, or timeout
+            for p in ("ignore", "request_timeout", "timeout"):
+                if p in kwargs:
+                    params[p] = kwargs.pop(p)
+
             for p in opensearch_query_params + GLOBAL_PARAMS:
                 if p in kwargs:
                     v = kwargs.pop(p)
                     if v is not None:
                         params[p] = _escape(v)
 
-            # don't treat ignore, request_timeout, and opaque_id as other params to avoid escaping
-            for p in ("ignore", "request_timeout"):
-                if p in kwargs:
-                    params[p] = kwargs.pop(p)
             return func(*args, params=params, headers=headers, **kwargs)
 
         return _wrapped

test_opensearchpy/test_client/test_utils.py

@@ -85,6 +85,31 @@ class TestQueryParams(TestCase):
         self.func_to_wrap(headers={"X": "y"})
         self.assertEqual(self.calls[-1], ((), {"params": {}, "headers": {"x": "y"}}))
 
+    def test_non_escaping_params(self):
+        # the query_params decorator doesn't validate "timeout" it simply avoids escaping as it did
+        self.func_to_wrap(simple_param="x", timeout="4s")
+        self.assertEqual(
+            self.calls[-1],
+            ((), {"params": {"simple_param": b"x", "timeout": "4s"}, "headers": {}}),
+        )
+
+        self.func_to_wrap(simple_param="x", timeout=4, ignore=5, request_timeout=6)
+        self.assertEqual(
+            self.calls[-1],
+            (
+                (),
+                {
+                    "params": {
+                        "simple_param": b"x",
+                        "timeout": 4,
+                        "ignore": 5,
+                        "request_timeout": 6,
+                    },
+                    "headers": {},
+                },
+            ),
+        )
+
     def test_per_call_authentication(self):
         self.func_to_wrap(api_key=("name", "key"))
         self.assertEqual(