error when curl read func returns unexpected long#22757
Open
Sjord wants to merge 1 commit into
Open
Conversation
Raise a value error when the callback registered with CURLOPT_READFUNCTION returns an unexpected long. The function registered with CURLOPT_READFUNCTION should return a string. PHP then writes that string to a buffer and returns the length, so that curl can read that many bytes from the buffer. The function can also return CURL_READFUNC_ABORT and CURL_READFUNC_PAUSE, so it also supports returning longs. However, when it returns a long other than these two constants, it is interpreted as a length. PHP does not update the buffer, but does instruct curl it can read that many bytes from the buffer. It reads whatever uninitialized data that is in the buffer and sends it over the line to the server. This seems bad, so validate the return value of the read function and raise an error. Returning 0 is a bit of an edge case. It is not documented but does results in correct behavior (i.e. end-of-file). So we accept that, but don't advertise it as valid in the error message. I am not worried about BC-break, because sending an uninitialized buffer does not result in a valid request, so this would already not work. I considered silently casting the int to a string. This would be very "old PHP" behavior. I think throwing an error is more strict and better and more in line with "new PHP" behavior, if that makes sense. Related to php#10270
Contributor
Author
|
@nicolas-grekas Could you review this? |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Raise a value error when the callback registered with CURLOPT_READFUNCTION returns an unexpected long.
The function registered with CURLOPT_READFUNCTION should return a string. PHP then writes that string to a buffer and returns the length, so that curl can read that many bytes from the buffer.
The function can also return CURL_READFUNC_ABORT and CURL_READFUNC_PAUSE, so it also supports returning longs. However, when it returns a long other than these two constants, it is interpreted as a length. PHP does not update the buffer, but does instruct curl it can read that many bytes from the buffer. It reads whatever uninitialized data that is in the buffer and sends it over the line to the server.
This seems bad, so validate the return value of the read function and raise an error.
Returning 0 is a bit of an edge case. It is not documented but does results in correct behavior (i.e. end-of-file). So we accept that, but don't advertise it as valid in the error message.
I am not worried about BC-break, because sending an uninitialized buffer does not result in a valid request, so this would already not work.
I considered silently casting the int to a string. This would be very "old PHP" behavior. I think throwing an error is more strict and better and more in line with "new PHP" behavior, if that makes sense.
Related to #10270