[HttpClient] Check status code before decoding in TraceableResponse::toArray()

chalasr · chalasr · commit 640fa708b59e · 2020-11-03T21:20:38.000+01:00
diff --git a/src/Symfony/Component/HttpClient/Response/TraceableResponse.php b/src/Symfony/Component/HttpClient/Response/TraceableResponse.php
@@ -52,24 +52,24 @@ public function getHeaders(bool $throw = true): array
 
     public function getContent(bool $throw = true): string
     {
-        $this->content = $this->response->getContent(false);
-
-        if ($throw) {
-            $this->checkStatusCode($this->response->getStatusCode());
+        try {
+            return $this->content = $this->response->getContent(false);
+        } finally {
+            if ($throw) {
+                $this->checkStatusCode($this->response->getStatusCode());
+            }
         }
-
-        return $this->content;
     }
 
     public function toArray(bool $throw = true): array
     {
-        $this->content = $this->response->toArray(false);
-
-        if ($throw) {
-            $this->checkStatusCode($this->response->getStatusCode());
+        try {
+            return $this->content = $this->response->toArray(false);
+        } finally {
+            if ($throw) {
+                $this->checkStatusCode($this->response->getStatusCode());
+            }
         }
-
-        return $this->content;
     }
 
     public function cancel(): void
diff --git a/src/Symfony/Component/HttpClient/Tests/TraceableHttpClientTest.php b/src/Symfony/Component/HttpClient/Tests/TraceableHttpClientTest.php
@@ -16,6 +16,7 @@
 use Symfony\Component\HttpClient\NativeHttpClient;
 use Symfony\Component\HttpClient\Response\MockResponse;
 use Symfony\Component\HttpClient\TraceableHttpClient;
+use Symfony\Contracts\HttpClient\Exception\ClientExceptionInterface;
 use Symfony\Contracts\HttpClient\HttpClientInterface;
 use Symfony\Contracts\HttpClient\Test\TestHttpServer;
 
@@ -100,4 +101,16 @@ public function testStream()
         $this->assertGreaterThan(1, \count($chunks));
         $this->assertSame('Symfony is awesome!', implode('', $chunks));
     }
+
+    public function testToArrayChecksStatusCodeBeforeDecoding()
+    {
+        $this->expectException(ClientExceptionInterface::class);
+
+        $sut = new TraceableHttpClient(new MockHttpClient($responseFactory = function (): MockResponse {
+            return new MockResponse('Errored.', ['http_code' => 400]);
+        }));
+
+        $response = $sut->request('GET', 'https://example.com/foo/bar');
+        $response->toArray();
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -52,24 +52,24 @@ public function getHeaders(bool $throw = true): array`
`52`	`52`
`53`	`53`	`public function getContent(bool $throw = true): string`
`54`	`54`	`{`
`55`		`- $this->content = $this->response->getContent(false);`
`56`		`-`
`57`		`- if ($throw) {`
`58`		`- $this->checkStatusCode($this->response->getStatusCode());`
	`55`	`+ try {`
	`56`	`+ return $this->content = $this->response->getContent(false);`
	`57`	`+ } finally {`
	`58`	`+ if ($throw) {`
	`59`	`+ $this->checkStatusCode($this->response->getStatusCode());`
	`60`	`+ }`
`59`	`61`	`}`
`60`		`-`
`61`		`- return $this->content;`
`62`	`62`	`}`
`63`	`63`
`64`	`64`	`public function toArray(bool $throw = true): array`
`65`	`65`	`{`
`66`		`- $this->content = $this->response->toArray(false);`
`67`		`-`
`68`		`- if ($throw) {`
`69`		`- $this->checkStatusCode($this->response->getStatusCode());`
	`66`	`+ try {`
	`67`	`+ return $this->content = $this->response->toArray(false);`
	`68`	`+ } finally {`
	`69`	`+ if ($throw) {`
	`70`	`+ $this->checkStatusCode($this->response->getStatusCode());`
	`71`	`+ }`
`70`	`72`	`}`
`71`		`-`
`72`		`- return $this->content;`
`73`	`73`	`}`
`74`	`74`
`75`	`75`	`public function cancel(): void`