Page MenuHomePhabricator

class HTTP::getAsFile
Open, LowPublicFeature

Description

Author: wmf.amgine3691

Description:
A common http request in extensions is to retrieve a url as a file.


Version: 1.17.x
Severity: enhancement

Details

Reference
bz27391

Event Timeline

bzimport raised the priority of this task from to Low.Nov 21 2014, 11:16 PM
bzimport set Reference to bz27391.
bzimport added a subscriber: Unknown Object (MLST).

wmf.amgine3691 wrote:

Here is a function currently used in an extension maintenance script to retrieve a remote file; it uses cURL which is not universally installed, stores files in a temporary directory under the extension using a temporary name, the former of which is fragile and writability must be tested within the script, and returns the temporary filename or false:

function getHttpFile( $url ){
$val = false;
$tempName = tempnam( 'temp/', 'MyExtension' );
if( is_writable( $tempName ) ){

		$tfh = fopen( $tempName, 'w' );
		if( is_resource( $tfh ) ){
			$ch = curl_init( $url );
			curl_setopt( $ch, CURLOPT_FILE, $tfh );
			curl_setopt( $ch, CURLOPT_HEADER, false );
			if( curl_exec( $ch ) ){
				$val = $tempName;
			}
			curl_close( $ch );
			fclose( $tfh );
		}

}
return $val;
}

There are a lot of reasons this is a less-than-desirable solution.

The extension should use wfTempDir() instead of "temp/" (unless it really wants the file to stick around, then it should use $wgTmpDir).

As far as writing output...this can already be done by specifying a callback function. I improved this in r82117 to allow specifying it in the $options array (rather than having to call setCallback() yourself)

Generally, I don't think letting callers handle file output is bad, there's nothing inherently wrong with it. However, we could add the ability to the Http class fairly trivially, if we really think it would help in some places (or reduce code duplication, I haven't grep'd around to see)

Assigning to myself since this is something I asked Amgine to submit. But feel free to do this yourself, Chad ;)

Mark:
This report has been in ASSIGNED status for more than one year and you are set as its assignee. In case that you are not actively working on a fix, please reset the bug status to NEW/UNCONFIRMED.
In case you do not plan to work on a fix in the near future: Please also edit the "Assigned To" field by clicking "Reset Assignee to default", in order to not prevent potential contributors from working on a fix. Thanks for your help!
[assigned>=1y]

Aklapper changed the subtype of this task from "Task" to "Feature Request".Feb 4 2022, 11:00 AM