Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix URL Formatting to Prevent Double Slashes in Base URL and API Method Combination #1595

Closed
HTSagara opened this issue Nov 19, 2024 · 1 comment

Comments

@HTSagara
Copy link
Contributor

(Filling out the following details about bugs will help us solve your issue sooner.)
I just notice that my last PR #1594 cause an error after I added the logic in the legacy_base_client.py due to a double slash in the api_url when concatenating the base_url with the api_method.py.

Screenshot 2024-11-19 at 11 44 14 AM

Reproducible in:

>pip freeze | grep slack
python --version
sw_vers && uname -v # or `ver`

Python 3.12.7
ProductName:            macOS
ProductVersion:         14.6.1
BuildVersion:           23G93
Darwin Kernel Version 23.6.0: Mon Jul 29 21:13:04 PDT 2024; root:xnu-10063.141.2~1/RELEASE_ARM64_T6020

>./scripts/run_validation.sh

The Slack SDK version

(Paste the output of pip freeze | grep slack)

Python runtime version

(Paste the output of python --version)
Python 3.12.7

OS info

(Paste the output of sw_vers && uname -v on macOS/Linux or ver on Windows OS)

Steps to reproduce:

(Share the commands to run, source code, and project settings (e.g., setup.py))

  1. source env_3.9.6/bin/activate
  2. ./scripts/run_validation.sh

Expected result:

Expected to pass all the tests
Screenshot 2024-11-19 at 11 48 58 AM

Actual result:

Test test_issue_690_oauth_v2_access from tests/web/test_web_client.py fails

Proposed solution:

Update the _get_url(base_url: str, api_method: str) -> str ensures that a properly formatted absolute URL is constructed from a base_url and an api_method, handling edge cases where redundant slashes might cause incorrect URLs.

def _get_url(base_url: str, api_method: str) -> str:
    """Joins the base Slack URL and an API method to form an absolute URL.

    Args:
        base_url (str): The base URL
        api_method (str): The Slack Web API method. e.g. 'chat.postMessage'

    Returns:
        The absolute API URL.
            e.g. 'https://slack.com/api/chat.postMessage'
    """
    if base_url.endswith("/") and api_method.startswith("/"):
        base_url = base_url.rstrip("/")
        
    return urljoin(base_url, api_method)

Apologies for the inconvenience and my lack of attention, I'm sending a PR to fix this issue ASAP.

Requirements

For general questions/issues about Slack API platform or its server-side, could you submit questions at https://my.slack.com/help/requests/new instead. 🙇

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

@seratch
Copy link
Member

seratch commented Nov 26, 2024

The changes were reverted by #1597

@seratch seratch closed this as completed Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants