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

HTTP::pool response returned without the PSR response when accessing a URL with an expired SSL cert. #41360

Closed
sfilzek opened this issue Mar 6, 2022 · 2 comments · Fixed by #41412

Comments

@sfilzek
Copy link
Contributor

sfilzek commented Mar 6, 2022

  • Laravel Version: v9.3.1
  • PHP Version: 8.0.12
  • Database Driver & Version: N/A

Description:

When using the HTTP::pool to access a site with an expired SSL cert, I was expecting a GuzzleHttp\Exception\RequestException to be returned, but instead we get a Illuminate\Http\Client\Response but it is missing the internal PSR response, so accessing any of the response details fails.

Steps To Reproduce:

use Illuminate\Http\Client\Pool;
use Illuminate\Support\Facades\Http;
	
$responses = Http::pool(fn (Pool $pool) => [
    $pool->get('https://expired.badssl.com')
]);

$responses[0]->status();

I would expect the $response[0] to be a type of GuzzleHttp\Exception\RequestException but we get a valid looking Illuminate\Http\Client\Response

The call to ->status() results in an error because the Illuminate\Http\Client\Response internal PSR response property is null:
Call to a member function getStatusCode() on null in ....

At present my solution to detect this failure is to check for a null return on the response->toPsrResponse() and then check for a non zero in the handlerStats ssl_verify_result, but this is fragile as it depends on using the curl supplied ssl_verify_result.

...

if ($responses[0]->toPsrResponse() === null && $responses[0]->handlerStats()['ssl_verify_result']) {
   // SSL error occurred during request
}

Any help or pointers to more detail would be greatly appreciated!

@driesvints
Copy link
Member

This indeed seems like a bug to me as well. Would appreciate PR's for this.

@sulemaanhamza
Copy link
Contributor

Created a PR for this issue: #41412

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

Successfully merging a pull request may close this issue.

3 participants