From f7db3b0a8a42da62d95e702b8d8398802b4d002c Mon Sep 17 00:00:00 2001 From: Henrique Sagara Date: Thu, 28 Nov 2024 15:35:06 -0500 Subject: [PATCH] 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 * Update tests/slack_sdk_async/web/test_web_client_url_format.py Co-authored-by: William Bergamin --------- Co-authored-by: William Bergamin --- slack_sdk/web/async_base_client.py | 2 ++ slack_sdk/web/base_client.py | 2 ++ slack_sdk/web/internal_utils.py | 2 ++ slack_sdk/web/legacy_base_client.py | 2 ++ tests/slack_sdk/web/test_internal_utils.py | 18 +++++++++++++++ .../web/test_legacy_web_client_url_format.py | 18 +++++++++++++++ tests/slack_sdk/web/test_web_client.py | 8 +++++++ .../web/test_web_client_url_format.py | 18 +++++++++++++++ .../web/test_web_client_url_format.py | 22 +++++++++++++++++++ 9 files changed, 92 insertions(+) diff --git a/slack_sdk/web/async_base_client.py b/slack_sdk/web/async_base_client.py index 2418b08c..65f852b6 100644 --- a/slack_sdk/web/async_base_client.py +++ b/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/'`.""" diff --git a/slack_sdk/web/base_client.py b/slack_sdk/web/base_client.py index e112d35c..4ce67b0c 100644 --- a/slack_sdk/web/base_client.py +++ b/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/'`.""" diff --git a/slack_sdk/web/internal_utils.py b/slack_sdk/web/internal_utils.py index 56604041..66dc5979 100644 --- a/slack_sdk/web/internal_utils.py +++ b/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) diff --git a/slack_sdk/web/legacy_base_client.py b/slack_sdk/web/legacy_base_client.py index c58e062a..44d49970 100644 --- a/slack_sdk/web/legacy_base_client.py +++ b/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/'`.""" diff --git a/tests/slack_sdk/web/test_internal_utils.py b/tests/slack_sdk/web/test_internal_utils.py index 13da2c1a..1740841f 100644 --- a/tests/slack_sdk/web/test_internal_utils.py +++ b/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", + ) diff --git a/tests/slack_sdk/web/test_legacy_web_client_url_format.py b/tests/slack_sdk/web/test_legacy_web_client_url_format.py index 0ba2f5ae..65c8c35b 100644 --- a/tests/slack_sdk/web/test_legacy_web_client_url_format.py +++ b/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) diff --git a/tests/slack_sdk/web/test_web_client.py b/tests/slack_sdk/web/test_web_client.py index 8423dee7..cea47a38 100644 --- a/tests/slack_sdk/web/test_web_client.py +++ b/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/") diff --git a/tests/slack_sdk/web/test_web_client_url_format.py b/tests/slack_sdk/web/test_web_client_url_format.py index 9ef4f932..56dcab44 100644 --- a/tests/slack_sdk/web/test_web_client_url_format.py +++ b/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) diff --git a/tests/slack_sdk_async/web/test_web_client_url_format.py b/tests/slack_sdk_async/web/test_web_client_url_format.py index 3f25dac0..d80786a1 100644 --- a/tests/slack_sdk_async/web/test_web_client_url_format.py +++ b/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)