Skip to content

Commit

Permalink
minor #38508 [HttpClient] Content doesn't get decoded when fetched fr…
Browse files Browse the repository at this point in the history
…om an exception (HypeMC)

This PR was merged into the 4.4 branch.

Discussion
----------

[HttpClient] Content doesn't get decoded when fetched from an exception

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

My previous PR #38493 created a new bug, the content doesn't get decoded when fetched from an exception because the inflate resource gets unset:

```php
<?php

use Symfony\Component\HttpClient\CurlHttpClient;
use Symfony\Contracts\HttpClient\Exception\ClientExceptionInterface;

include __DIR__.'/symfony/vendor/autoload.php';

$client = new CurlHttpClient();

try {
    $client->request('GET', 'http://example.com/404');
} catch (ClientExceptionInterface $exception) {
    echo $exception->getResponse()->getContent(false);
}
```

I've removed the part where the resource gets unset since it still might be used at some point.

The test is implementation independent so I believe it should go in contracts, please correct me if I wrong. Also, I was unable to find a way to do the test without adding a new endpoint this time, any suggestions would be appreciated.

Commits
-------

8fa4f85 Don't unset the inflate resource on close as it might still be needed
  • Loading branch information
nicolas-grekas committed Oct 11, 2020
2 parents 1cef665 + 8fa4f85 commit a8479e8
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 2 deletions.
1 change: 0 additions & 1 deletion src/Symfony/Component/HttpClient/Response/CurlResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,6 @@ public function __destruct()
*/
private function close(): void
{
$this->inflate = null;
unset($this->multi->openHandles[$this->id], $this->multi->handlesActivity[$this->id]);
curl_setopt($this->handle, \CURLOPT_PRIVATE, '_0');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ private function open(): void
private function close(): void
{
unset($this->multi->openHandles[$this->id], $this->multi->handlesActivity[$this->id]);
$this->handle = $this->buffer = $this->inflate = $this->onProgress = null;
$this->handle = $this->buffer = $this->onProgress = null;
}

/**
Expand Down
6 changes: 6 additions & 0 deletions src/Symfony/Contracts/HttpClient/Test/Fixtures/web/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@
header('Content-Type: application/json', true, 404);
break;

case '/404-gzipped':
header('Content-Type: text/plain', true, 404);
ob_start('ob_gzhandler');
echo 'some text';
exit;

case '/301':
if ('Basic Zm9vOmJhcg==' === $vars['HTTP_AUTHORIZATION']) {
header('Location: http://127.0.0.1:8057/302', true, 301);
Expand Down
12 changes: 12 additions & 0 deletions src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,18 @@ public function testGetContentAfterDestruct()
}
}

public function testGetEncodedContentAfterDestruct()
{
$client = $this->getHttpClient(__FUNCTION__);

try {
$client->request('GET', 'http://localhost:8057/404-gzipped');
$this->fail(ClientExceptionInterface::class.' expected');
} catch (ClientExceptionInterface $e) {
$this->assertSame('some text', $e->getResponse()->getContent(false));
}
}

public function testProxy()
{
$client = $this->getHttpClient(__FUNCTION__);
Expand Down

0 comments on commit a8479e8

Please sign in to comment.