Adding Trailing Slash, Fix Double Slash Issue in _get_url, and Edge Case Tests (#1598)

* adding trailing slash

* prevent double slash before appending the API method

* tests if adds or preserves the trailing slash

* testing _get_url

* improving tests and _get_url()

* removing duplicated test case

* test /api

* Update tests/slack_sdk_async/web/test_web_client_url_format.py

Co-authored-by: William Bergamin <[email protected]>

* Update tests/slack_sdk_async/web/test_web_client_url_format.py

Co-authored-by: William Bergamin <[email protected]>

---------

Co-authored-by: William Bergamin <[email protected]>

Author: HTSagara

slack_sdk/web/async_base_client.py

@@ -46,6 +46,8 @@ def __init__(
     ):
         self.token = None if token is None else token.strip()
         """A string specifying an `xoxp-*` or `xoxb-*` token."""
+        if not base_url.endswith("/"):
+            base_url += "/"
         self.base_url = base_url
         """A string representing the Slack API base URL.
         Default is `'https://slack.com/api/'`."""

slack_sdk/web/base_client.py

@@ -59,6 +59,8 @@ def __init__(
     ):
         self.token = None if token is None else token.strip()
         """A string specifying an `xoxp-*` or `xoxb-*` token."""
+        if not base_url.endswith("/"):
+            base_url += "/"
         self.base_url = base_url
         """A string representing the Slack API base URL.
         Default is `'https://slack.com/api/'`."""

slack_sdk/web/internal_utils.py

@@ -67,6 +67,8 @@ def _get_url(base_url: str, api_method: str) -> str:
         The absolute API URL.
             e.g. 'https://slack.com/api/chat.postMessage'
     """
+    # Ensure no leading slash in api_method to prevent double slashes
+    api_method = api_method.lstrip("/")
     return urljoin(base_url, api_method)
 
 

slack_sdk/web/legacy_base_client.py

@@ -62,6 +62,8 @@ def __init__(
     ):
         self.token = None if token is None else token.strip()
         """A string specifying an `xoxp-*` or `xoxb-*` token."""
+        if not base_url.endswith("/"):
+            base_url += "/"
         self.base_url = base_url
         """A string representing the Slack API base URL.
         Default is `'https://slack.com/api/'`."""

tests/slack_sdk/web/test_internal_utils.py

@@ -12,6 +12,7 @@
     _parse_web_class_objects,
     _to_v2_file_upload_item,
     _next_cursor_is_present,
+    _get_url,
 )
 
 
@@ -108,3 +109,20 @@ def test_next_cursor_is_present(self):
         assert _next_cursor_is_present({"response_metadata": {"next_cursor": ""}}) is False
         assert _next_cursor_is_present({"response_metadata": {"next_cursor": None}}) is False
         assert _next_cursor_is_present({"something_else": {"next_cursor": "next-page"}}) is False
+
+    def test_get_url_prevent_double_slash(self):
+        # Test case: Prevent double slash when both base_url and api_method include slashes
+        api_url = _get_url("https://slack.com/api/", "/chat.postMessage")
+        self.assertEqual(
+            api_url,
+            "https://slack.com/api/chat.postMessage",
+            "Should correctly handle and remove double slashes between base_url and api_method",
+        )
+
+        # Test case: Handle api_method without leading slash
+        api_url = _get_url("https://slack.com/api/", "chat.postMessage")
+        self.assertEqual(
+            api_url,
+            "https://slack.com/api/chat.postMessage",
+            "Should correctly handle api_method without a leading slash",
+        )

tests/slack_sdk/web/test_legacy_web_client_url_format.py

@@ -10,6 +10,8 @@ def setUp(self):
         setup_mock_web_api_server(self, MockHandler)
         self.client = LegacyWebClient(token="xoxb-api_test", base_url="http://localhost:8888")
         self.client_base_url_slash = LegacyWebClient(token="xoxb-api_test", base_url="http://localhost:8888/")
+        self.client_api = LegacyWebClient(token="xoxb-api_test", base_url="http://localhost:8888/api")
+        self.client_api_slash = LegacyWebClient(token="xoxb-api_test", base_url="http://localhost:8888/api/")
 
     def tearDown(self):
         cleanup_mock_web_api_server(self)
@@ -33,3 +35,19 @@ def test_base_url_with_slash_api_method_with_slash(self):
     def test_base_url_without_slash_api_method_with_slash_and_trailing_slash(self):
         self.client.api_call("/chat.postMessage/")
         assert_received_request_count(self, "/chat.postMessage/", 1)
+
+    def test_base_url_with_api(self):
+        self.client_api.api_call("chat.postMessage")
+        assert_received_request_count(self, "/api/chat.postMessage", 1)
+
+    def test_base_url_with_api_method_without_slash_method_with_slash(self):
+        self.client_api.api_call("/chat.postMessage")
+        assert_received_request_count(self, "/api/chat.postMessage", 1)
+
+    def test_base_url_with_api_slash(self):
+        self.client_api_slash.api_call("chat.postMessage")
+        assert_received_request_count(self, "/api/chat.postMessage", 1)
+
+    def test_base_url_with_api_slash_and_method_with_slash(self):
+        self.client_api_slash.api_call("/chat.postMessage")
+        assert_received_request_count(self, "/api/chat.postMessage", 1)

tests/slack_sdk/web/test_web_client.py

@@ -227,3 +227,11 @@ def test_user_auth_blocks(self):
             user_auth_blocks=[DividerBlock(), DividerBlock()],
         )
         self.assertIsNone(new_message.get("error"))
+
+    def test_base_url_appends_trailing_slash_issue_15141(self):
+        client = self.client
+        self.assertEqual(client.base_url, "http://localhost:8888/")
+
+    def test_base_url_preserves_trailing_slash_issue_15141(self):
+        client = WebClient(base_url="http://localhost:8888/")
+        self.assertEqual(client.base_url, "http://localhost:8888/")

tests/slack_sdk/web/test_web_client_url_format.py

@@ -10,6 +10,8 @@ def setUp(self):
         setup_mock_web_api_server(self, MockHandler)
         self.client = WebClient(token="xoxb-api_test", base_url="http://localhost:8888")
         self.client_base_url_slash = WebClient(token="xoxb-api_test", base_url="http://localhost:8888/")
+        self.client_api = WebClient(token="xoxb-api_test", base_url="http://localhost:8888/api")
+        self.client_api_slash = WebClient(token="xoxb-api_test", base_url="http://localhost:8888/api/")
 
     def tearDown(self):
         cleanup_mock_web_api_server(self)
@@ -33,3 +35,19 @@ def test_base_url_with_slash_api_method_with_slash(self):
     def test_base_url_without_slash_api_method_with_slash_and_trailing_slash(self):
         self.client.api_call("/chat.postMessage/")
         assert_received_request_count(self, "/chat.postMessage/", 1)
+
+    def test_base_url_with_api(self):
+        self.client_api.api_call("chat.postMessage")
+        assert_received_request_count(self, "/api/chat.postMessage", 1)
+
+    def test_base_url_with_api_method_with_slash(self):
+        self.client_api.api_call("/chat.postMessage")
+        assert_received_request_count(self, "/api/chat.postMessage", 1)
+
+    def test_base_url_with_api_slash(self):
+        self.client_api_slash.api_call("chat.postMessage")
+        assert_received_request_count(self, "/api/chat.postMessage", 1)
+
+    def test_base_url_with_api_slash_and_method_with_slash(self):
+        self.client_api_slash.api_call("/chat.postMessage")
+        assert_received_request_count(self, "/api/chat.postMessage", 1)

tests/slack_sdk_async/web/test_web_client_url_format.py

@@ -15,6 +15,8 @@ def setUp(self):
         setup_mock_web_api_server_async(self, MockHandler)
         self.client = AsyncWebClient(token="xoxb-api_test", base_url="http://localhost:8888")
         self.client_base_url_slash = AsyncWebClient(token="xoxb-api_test", base_url="http://localhost:8888/")
+        self.client_api = AsyncWebClient(token="xoxb-api_test", base_url="http://localhost:8888/api")
+        self.client_api_slash = AsyncWebClient(token="xoxb-api_test", base_url="http://localhost:8888/api/")
 
     def tearDown(self):
         cleanup_mock_web_api_server_async(self)
@@ -43,3 +45,23 @@ async def test_base_url_with_slash_api_method_with_slash(self):
     async def test_base_url_without_slash_api_method_with_slash_and_trailing_slash(self):
         await self.client.api_call("/chat.postMessage/")
         await assert_received_request_count_async(self, "/chat.postMessage/", 1)
+
+    @async_test
+    async def test_base_url_with_api(self):
+        await self.client_api.api_call("chat.postMessage")
+        await assert_received_request_count_async(self, "/api/chat.postMessage", 1)
+
+    @async_test
+    async def test_base_url_with_api_method_without_slash_method_with_slash(self):
+        await self.client_api.api_call("/chat.postMessage")
+        await assert_received_request_count_async(self, "/api/chat.postMessage", 1)
+
+    @async_test
+    async def test_base_url_with_api_slash(self):
+        await self.client_api_slash.api_call("chat.postMessage")
+        await assert_received_request_count_async(self, "/api/chat.postMessage", 1)
+
+    @async_test
+    async def test_base_url_with_api_slash_and_method_with_slash(self):
+        await self.client_api_slash.api_call("/chat.postMessage")
+        await assert_received_request_count_async(self, "/api/chat.postMessage", 1)