[#17187] - Allow cURL to save a url's contents to a destination file pointer (available via the CURLOPT_FILE option).
- Closed
- 20 Mar 2022
- Medium
- Build: staging
- # 17187
- Diff
- haydenyoung:curl-transport
- Success hound No violations found. Woof! Details
- Success continuous-integration/drone/pr the build was successful Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success continuous-integration/appveyor/pr AppVeyor build succeeded Details
User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Summary of Changes
cURL's CURLOPT_FILE option does not return headers within the curl_exec returned output and so the Curl Transport object is unable to detect what headers have been returned for the request which ultimately results in an exception. Additionally, when curl_exec is used in conjunction with CURLOPT_FILE, a boolean true is returned as the body of the response; this causes the Curl Transport's request method to throw an exception as it is expecting a string.
To handle CURLOPT_FILE, the Curl Transport's request method now detects the use of the FILE option and uses the HEADERFUNCTION option to return the request's headers, appending them to the response content. Additionally, request now checks for either boolean true or a string to determine if the curl_exec was successful or not.
Testing Instructions
$src = "http://domain.tld/file.name";
$dest = "/tmp/file.name";
$fp = fopen($dest, 'w+');
$http = JHttpFactory::getHttp(null, 'curl');
$http->setOption("transport.curl", [CURLOPT_FILE=>$fp]);
$http->get($src, null, 50);
Expected result
File should be saved to file system.
Actual result
File is saved to file system but is empty.
Output of file is dumped to stdout/web page.
Documentation Changes Required
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries |
| Labels |
Added:
?
|
||
| Category | Libraries | ⇒ | Unit Tests Repository Administration com_admin SQL Postgresql MS SQL com_banners com_categories com_config com_contact com_content com_fields |
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-08-11 16:56:43 |
| Closed_By | ⇒ | haydenyoung | |
| Labels |
Added:
?
|
||
| Status | Closed | ⇒ | Pending |
| Category | Unit Tests Repository Administration com_admin SQL Postgresql MS SQL com_banners com_categories com_config com_contact com_content com_fields | ⇒ | Libraries |
Thanks i have just rebooted drone.
| Labels |
Removed:
?
|
||
| Status | Pending | ⇒ | Needs Review |
Status is set on "Needs Review".
It'd be nice if this could get tested, or is this going to be another of those "this is a purely code change with no user facing benefit so it's going to be ignored" PRs.
| Status | Needs Review | ⇒ | Discussion |
Sorry that it has taken so long to respond to this pull request. This is a nice enhancement we would like to consider for Joomla 4. Can you rebase the branch on the Joomla 4.2-dev branch so we can do some proper testing. In the meantime I'm closing the pr, when ready, please reopen so it will get our attention again. Thanks so far for your contribution and time to make Joomla better.
| Status | Discussion | ⇒ | Closed |
| Closed_Date | 2017-08-11 16:56:43 | ⇒ | 2022-03-20 08:39:06 |
| Closed_By | haydenyoung | ⇒ | laoneo |
| Labels |
Added:
?
Removed: ? |
||
Drone seems to be stuck on:
continuous-integration/drone/pr — this build is pending
It has been 2 days and all other tests have passed.