[ 
https://issues.apache.org/jira/browse/THRIFT-4641?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16623918#comment-16623918
 ] 

ASF GitHub Bot commented on THRIFT-4641:
----------------------------------------

jeking3 closed pull request #1598: THRIFT-4641: Check HTTP Status Code in 
TCurlClient
URL: https://github.com/apache/thrift/pull/1598
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/lib/php/lib/Transport/TCurlClient.php 
b/lib/php/lib/Transport/TCurlClient.php
index f51fa88ea1..2ca4f65bb0 100644
--- a/lib/php/lib/Transport/TCurlClient.php
+++ b/lib/php/lib/Transport/TCurlClient.php
@@ -220,12 +220,18 @@ public function flush()
         curl_setopt(self::$curlHandle, CURLOPT_URL, $fullUrl);
         $this->response_ = curl_exec(self::$curlHandle);
 
-        // Connect failed?
-        if (!$this->response_) {
+        $code = curl_getinfo(self::$curlHandle, CURLINFO_HTTP_CODE);
+
+        // Handle non 200 status code / connect failure
+        if (!$this->response_ || $code !== 200) {
             curl_close(self::$curlHandle);
             self::$curlHandle = null;
+            $this->response_ = null;
             $error = 'TCurlClient: Could not connect to ' . $fullUrl;
-            throw new TTransportException($error, 
TTransportException::NOT_OPEN);
+            if ($code) {
+                $error .= ', HTTP status code: ' . $code;
+            }
+            throw new TTransportException($error, 
TTransportException::UNKNOWN);
         }
     }
 


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> TCurlClient doesn't check for HTTP status code
> ----------------------------------------------
>
>                 Key: THRIFT-4641
>                 URL: https://issues.apache.org/jira/browse/THRIFT-4641
>             Project: Thrift
>          Issue Type: Bug
>          Components: PHP - Library
>    Affects Versions: 0.11.0
>            Reporter: Josip Sokcevic
>            Assignee: James E. King III
>            Priority: Major
>             Fix For: 0.12.0
>
>         Attachments: 
> 0001-THRIFT-4641-Check-HTTP-Status-Code-in-TCurlClient.patch
>
>
> TCurlClient doesn't check for HTTP status code. Other languages, such as Go 
> and Java do check for it. This can lead to potentially wrong exception being 
> thrown: TProtocolException instead of TTransportException. To be more 
> precise, TCurlTransport doesn't error out and Protocol starts to parse 
> response, which may lead to TProtocolException. For example, AWS ALB may 
> return 502 Bad Gateway with HTML as part of payload when there's an issue 
> with an upstream. In such case, a client may want to retry but it only makes 
> sense if there's a transport exception.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to