Skip to content

Commit

Permalink
Adding Trailing Slash, Fix Double Slash Issue in _get_url, and Edge…
Browse files Browse the repository at this point in the history
… 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]>
  • Loading branch information
HTSagara and WilliamBergamin authored Nov 28, 2024
1 parent 5464f89 commit f7db3b0
Show file tree
Hide file tree
Showing 9 changed files with 92 additions and 0 deletions.
2 changes: 2 additions & 0 deletions slack_sdk/web/async_base_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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/'`."""
Expand Down
2 changes: 2 additions & 0 deletions slack_sdk/web/base_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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/'`."""
Expand Down
2 changes: 2 additions & 0 deletions slack_sdk/web/internal_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down
2 changes: 2 additions & 0 deletions slack_sdk/web/legacy_base_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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/'`."""
Expand Down
18 changes: 18 additions & 0 deletions tests/slack_sdk/web/test_internal_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
_parse_web_class_objects,
_to_v2_file_upload_item,
_next_cursor_is_present,
_get_url,
)


Expand Down Expand Up @@ -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",
)
18 changes: 18 additions & 0 deletions tests/slack_sdk/web/test_legacy_web_client_url_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
8 changes: 8 additions & 0 deletions tests/slack_sdk/web/test_web_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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/")
18 changes: 18 additions & 0 deletions tests/slack_sdk/web/test_web_client_url_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
22 changes: 22 additions & 0 deletions tests/slack_sdk_async/web/test_web_client_url_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)

0 comments on commit f7db3b0

Please sign in to comment.