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

TransportExceptionInterface handled in a wrong place #1905

Closed
zerkms opened this issue Jul 1, 2022 · 1 comment · Fixed by #1911
Closed

TransportExceptionInterface handled in a wrong place #1905

zerkms opened this issue Jul 1, 2022 · 1 comment · Fixed by #1911

Comments

@zerkms
Copy link

zerkms commented Jul 1, 2022

Q A
Bug? yes
New Feature? no
Support question? no
Version 2.0-beta2

Actual Behavior

Currently transport exception is set up to be caught, wrapped and rethrown in the AbstractResourceOwner like this:

try {
return $this->httpClient->request(
$method,
$url,
$options
);
} catch (TransportExceptionInterface $e) {
throw new HttpTransportException('Error while sending HTTP request', $this->getName(), $e->getCode(), $e);
}

and it's called at

$response = $this->doGetTokenRequest($this->options['access_token_url'], $parameters);
$response = $this->getResponseContent($response);

BUT!!!!

at least in symfony 5.4 the http client request method does not make an actual request but instead creates an TraceableResponse that is performed asynchronously. Then on line 97 $response = $this->getResponseContent($response); we try to deserialise the response

try {
return $rawResponse->toArray(false);
} catch (JsonException $e) {
parse_str($rawResponse->getContent(false), $response);
return $response;
}

and only catch JsonException. But what is thrown is TransportResponseTrait.

Here is the actual stack trace

Symfony\Component\HttpClient\Exception\TransportException: Could not resolve host: <redacted> for "https://<redacted>/access_token".
#24 /vendor/symfony/http-client/Response/CurlResponse.php(334): Symfony\Component\HttpClient\Response\CurlResponse::perform
#23 /vendor/symfony/http-client/Response/TransportResponseTrait.php(174): Symfony\Component\HttpClient\Response\CurlResponse::stream
#22 /vendor/symfony/http-client/Response/CommonResponseTrait.php(153): Symfony\Component\HttpClient\Response\CurlResponse::initialize
#21 /vendor/symfony/http-client/Response/CommonResponseTrait.php(44): Symfony\Component\HttpClient\Response\CurlResponse::doGetContent
#20 /vendor/symfony/http-client/Response/CurlResponse.php(243): Symfony\Component\HttpClient\Response\CurlResponse::getContent
#19 /vendor/symfony/http-client/Response/CommonResponseTrait.php(83): Symfony\Component\HttpClient\Response\CurlResponse::toArray
#18 /vendor/hwi/oauth-bundle/src/OAuth/ResourceOwner/AbstractResourceOwner.php(293): HWI\Bundle\OAuthBundle\OAuth\ResourceOwner\AbstractResourceOwner::getResponseContent
#17 /vendor/hwi/oauth-bundle/src/OAuth/ResourceOwner/GenericOAuth2ResourceOwner.php(97): HWI\Bundle\OAuthBundle\OAuth\ResourceOwner\GenericOAuth2ResourceOwner::getAccessToken
#16 /vendor/hwi/oauth-bundle/src/Security/Http/Authenticator/OAuthAuthenticator.php(137): HWI\Bundle\OAuthBundle\Security\Http\Authenticator\OAuthAuthenticator::authenticate
#15 /vendor/symfony/security-http/Authentication/AuthenticatorManager.php(180): Symfony\Component\Security\Http\Authentication\AuthenticatorManager::executeAuthenticator
#14 /vendor/symfony/security-http/Authentication/AuthenticatorManager.php(161): Symfony\Component\Security\Http\Authentication\AuthenticatorManager::executeAuthenticators
#13 /vendor/symfony/security-http/Authentication/AuthenticatorManager.php(141): Symfony\Component\Security\Http\Authentication\AuthenticatorManager::authenticateRequest
#12 /vendor/symfony/security-http/Firewall/AuthenticatorManagerListener.php(40): Symfony\Component\Security\Http\Firewall\AuthenticatorManagerListener::authenticate
#11 /vendor/symfony/security-http/Firewall/AbstractListener.php(26): Symfony\Component\Security\Http\Firewall\AbstractListener::__invoke
#10 /vendor/symfony/security-http/Firewall.php(119): Symfony\Component\Security\Http\Firewall::callListeners
#9 /vendor/symfony/security-http/Firewall.php(92): Symfony\Component\Security\Http\Firewall::onKernelRequest
#8 /vendor/symfony/event-dispatcher/EventDispatcher.php(270): Symfony\Component\EventDispatcher\EventDispatcher::Symfony\Component\EventDispatcher\{closure}
#7 /vendor/symfony/event-dispatcher/EventDispatcher.php(230): Symfony\Component\EventDispatcher\EventDispatcher::callListeners
#6 /vendor/symfony/event-dispatcher/EventDispatcher.php(59): Symfony\Component\EventDispatcher\EventDispatcher::dispatch
#5 /vendor/symfony/http-kernel/HttpKernel.php(128): Symfony\Component\HttpKernel\HttpKernel::handleRaw
#4 /vendor/symfony/http-kernel/HttpKernel.php(74): Symfony\Component\HttpKernel\HttpKernel::handle
#3 /vendor/symfony/http-kernel/Kernel.php(202): Symfony\Component\HttpKernel\Kernel::handle
#2 /vendor/symfony/runtime/Runner/Symfony/HttpKernelRunner.php(35): Symfony\Component\Runtime\Runner\Symfony\HttpKernelRunner::run
#1 /vendor/autoload_runtime.php(35): require_once
#0 /public/index.php(5): null

What is the actual behavior?

Symfony throws 500 because authenticator has no idea how to handle TransportException

Expected Behavior

What is the behavior you expect?

The exception should be caught and rewrapped into HWI\Bundle\OAuthBundle\OAuth\Exception\HttpTransportException

Steps to Reproduce

What are the steps to reproduce this bug? Please add code examples,
screenshots or links to GitHub repositories that reproduce the problem.

Possible Solutions

If you have already ideas how to solve the issue, add them here.
(remove this section if not needed)

AbstractResourceOwner::getResponseContent() should also catch for the TransportExceptionInterface and rewrap.

@stloyd
Copy link
Collaborator

stloyd commented Jul 21, 2022

Good catch, should be fixed by #1911.

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