Skip to content
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

Allow cURL to save a url's contents to a destination file pointer (available via the CURLOPT_FILE option). #17187

Closed
wants to merge 3 commits into from

Conversation

haydenyoung
Copy link

@haydenyoung haydenyoung commented Jul 20, 2017

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

@haydenyoung
Copy link
Author

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.

@zero-24
Copy link
Contributor

zero-24 commented Aug 13, 2017

Thanks i have just rebooted drone.

@ghost
Copy link

ghost commented Nov 5, 2017

Status is set on "Needs Review".

@mbabker
Copy link
Contributor

mbabker commented Jul 21, 2018

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.

@ghost ghost added the J3 Issue label Apr 5, 2019
@ghost ghost removed the J3 Issue label Apr 19, 2019
Comment on lines +390 to +392
*
* @return int The length of the header.
*/

This comment was marked as abuse.

@laoneo
Copy link
Member

laoneo commented Mar 20, 2022

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.

@laoneo laoneo closed this Mar 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants