-
Notifications
You must be signed in to change notification settings - Fork 9.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Curl.php #7405
Update Curl.php #7405
Conversation
Added support for PUT requests
@@ -175,6 +175,9 @@ public function write($method, $url, $http_ver = '1.1', $headers = [], $body = ' | |||
if ($method == \Zend_Http_Client::POST) { | |||
curl_setopt($this->_getResource(), CURLOPT_POST, true); | |||
curl_setopt($this->_getResource(), CURLOPT_POSTFIELDS, $body); | |||
} elseif ($method == \Zend_Http_Client::PUT) { | |||
curl_setopt($this->_getResource(), CURLOPT_CUSTOMREQUEST, 'PUT'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a CURLOPT_PUT
constant and with PUT requests you should be using CURLOPT_INFILE
and CURLOPT_INFILESIZE
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CURLOPT_PUT
option was deprecated a couple versions of cURL ago (source), but the php.net documentation hasn't been updated yet. We verified this; when using the CURLOPT_PUT
option, cURL was using GET instead of PUT.
Also, a lot APIs use the PUT method for other purposes than uploading files (only), for example to update an object rather than creating one. In that case CURLOPT_POSTFIELDS
should be used. If uploading files via PUT requests does not use the CURLOPT_POSTFIELDS
option, the function Curl::write()
method should be modified imho, e.g. with an extra parameter.
I haven't tested this myself, but I think CURLFile
can be used in combination with PUT requests too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I stand corrected as to the deprecation, but in that case the recommendation is to use CURLOPT_UPLOAD
, it will automatically assume the HTTP PUT
method unless otherwise specified, and then you then still have the ability to handle CURLOPT_INFILESIZE
or CURLOPT_INFILESIZE_LARGE
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, but keep in mind that PUT
has other use cases than uploading files only.
We noticed that `CURLOPT_CUSTOMREQUEST` has priority over regular options like `CURLOPT_HTTPGET` and `CURLOPT_POST`. When the `CURLOPT_CUSTOMREQUEST` option has been set to `PUT` for a PUT request, `CURLOPT_HTTPGET` and `CURLOPT_POST` do not overwrite the value of `CURLOPT_CUSTOMREQUEST` and thus `CURLOPT_CUSTOMREQUEST` has to be used again.
B2B-2148: Can no longer install when my database name has a dash in it
Added support for PUT requests