Be more precise in recognising three-part curl response #102
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I'm using this modification in production, and for me it solved the "Got an invalid JSON response from the server" problem. Could this be merged so I can get rid of a private patch to make MesageBird work on Redhat 7? |
modprobe
suggested changes
Oct 8, 2019
| @@ -219,7 +219,7 @@ public function performHttpRequest($method, $resourceName, $query = null, $body | |||
|
|
|||
| // Split the header and body | |||
| $parts = explode("\r\n\r\n", $response, 3); | |||
| list($responseHeader, $responseBody) = ($parts[0] === 'HTTP/1.1 100 Continue') ? array ($parts[1], $parts[2]) : array ($parts[0], $parts[1]); | |||
| list($responseHeader, $responseBody) = (strpos($parts[0], "\n") == false && strpos($parts[0], 'HTTP/1.') == 0) ? array ($parts[1], $parts[2]) : array ($parts[0], $parts[1]); | |||
There was a problem hiding this comment.
Could you extract the condition into a separate variable like $isThreePartResponse for more readability?
There was a problem hiding this comment.
Also, please use strict type comparison.
There was a problem hiding this comment.
Yup, agreed! I have updated the code.
…ct bits out of the result array. Use strict checking for type safety.
modprobe
approved these changes
Oct 8, 2019
|
@driehuis @vanderwijk I just pushed v1.16.1 which includes this fix 🙂 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On RHEL7, libcurl's
curl_execreturns the data in two-part form when not using a proxy, and in three-part form when using a proxy (for example, when run with HTTPS_PROXY=myproxy:8080 in the PHP environment). The existing code presumes curl will only use the three part form with chunked transfers. This pull request proposes to better look at the first part to see if it is an isolated HTTP response line, indicating a three part response, or a multi-line response, indicating a two part response. Arguably, the strpos() is redundant, but given the vagaries of libcurl I'd rather be safe than sorry.This fixes the side issue mentioned in #48 where the MessageBird PHP REST API reports "Got an invalid JSON response from the server." The issue could possibly be worked around by upgrading curl and/or its php wrapper, but that opens up another whole can of worms, and I'm not sure curl is wrong here in the first place.