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

URLHelper::join() does not respect full-url in $endpoint #99

Closed
Gummibeer opened this issue Nov 7, 2022 · 1 comment
Closed

URLHelper::join() does not respect full-url in $endpoint #99

Gummibeer opened this issue Nov 7, 2022 · 1 comment
Assignees

Comments

@Gummibeer
Copy link
Contributor

Gummibeer commented Nov 7, 2022

The \Sammyjo20\Saloon\Helpers\URLHelper::join() method does not respect a full URL in the $endpoint argument but still prefixes it with the $baseUrl.
Some APIs have a few endpoints on another base URL/domain. I would like to override the default base URL with a full URL in the request endpoint instead of creating a whole new connector.

The already used filter_var($endpoint, FILTER_VALIDATE_URL) could be used to early return in case the $endpoint is already a full and valid URL. Possibly also combined with parse_url() to check scheme and host are defined.

Right now I'm running in the following curl error which fully bubbles through all layers. Even with the AlwaysThrowsOnErrors trait it fully bubbles through and doesn't get wrapped up in a \GuzzleHttp\Exception\ConnectException or similar.

cURL error 6: Could not resolve host: api.steampowered.comhttps (see https://curl.haxx.se/libcurl/c/libcurl-errors.html) for https://api.steampowered.comhttps//steamcommunity.com/actions/QueryLocations?key=xyz&format=json

class SteamConnector extends SaloonConnector
{
    public function defineBaseUrl(): string
    {
        return 'https://api.steampowered.com';
    }
}
class QueryLocationsRequest extends SaloonRequest
{
    protected ?string $method = 'GET';

    public function defineEndpoint(): string
    {
        $query = '';

        if ($this->countryCode) {
            $query .= "/{$this->countryCode}";

            if ($this->stateCode) {
                $query .= "/{$this->stateCode}";
            }
        }

        return "https://steamcommunity.com/actions/QueryLocations{$query}";
    }
}
@ashleyhood
Copy link
Contributor

I agree that if the \Sammyjo20\Saloon\Helpers\URLHelper::join() method could check if the $endpoint is a correctly formed URL and if so, exit early and return the $endpoint as the URL.

This would help me, as at the moment when connecting to the Xero Accounting API I have to create two connectors. One connector to authorize and another connector to get the token because Xero has different base URLs for each:

  • Authorize: https://login.xero.com
  • Token: https://identity.xero.com

This means I would just be able to set the OAuthConfig::make()->setTokenEndpoint() to the Xero API URL token endpoint without creating a separate connector.

I don't think this would be a breaking change as it will not cause any existing code to be changed. I would be happy to create a PR if this is something that would be helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants