r45549 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r45548‎ | r45549 | r45550 >
Date:06:56, 8 January 2009
Author:demon
Status:ok (Comments)
Tags:
Comment:
Fix odd edge case for POST Http::request()'s. I incidentally found this while working with Special:Import. For some reason that I can't 100% nail down--works fine on Win32, fails on CentOS--cURL seems to want CURLOPT_POSTFIELDS set to at least an empty string, if not values. Using a workaround I found, we'll set it to an empty string if we're making a POST. Since we explicitly allow setting of arbitrary curlopts later, users can still override the empty string if they in fact want to set this. This fixes my Special:Import problem (and possibly similar issues) and has no regressions afaict. Credit to Phill Sparks @ http://www.milk-hub.net/blog/2008/08/26/curl_error_26#comments-77
Modified paths:
  • /trunk/phase3/includes/HttpFunctions.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/HttpFunctions.php
——@@ -56,8 +56,10 @@
5757
5858 curl_setopt( $c, CURLOPT_TIMEOUT, $timeout );
5959 curl_setopt( $c, CURLOPT_USERAGENT, self :: userAgent() );
60 - if ( $method == 'POST' )
 60+ if ( $method == 'POST' ) {
6161 curl_setopt( $c, CURLOPT_POST, true );
 62+ curl_setopt( $c, CURLOPT_POSTFIELDS, '' );
 63+ }
6264 else
6365 curl_setopt( $c, CURLOPT_CUSTOMREQUEST, $method );
6466

Follow-up revisions

RevisionCommit summaryAuthorDate
r45553Backport Http fix from r45549 to 1.14 branch.demon07:29, 8 January 2009
r59446Removed JS2 work (has been moved to the js2-work branch). Has been lightly te...tstarling12:00, 26 November 2009

Comments

#Comment by Tim Starling (talk | contribs)   08:00, 8 January 2009

Fix or revert per IRC conversation

#Comment by 😂 (talk | contribs)   18:54, 8 January 2009

Per further IRC conversations (and some more checking), this seems to be ok. As Tim pointed out, POSTFIELDS *must* be set to null when CURLOPT_READFUNCTION or CURLOPT_INFILE are set. Fortunately, it appears from my testing last night that curl_setopt() sets empty strings as nulls. This means A) It fixes my original problem and B) It won't cause a regression when READFUNCTION or INFILE are set (which is now possible, with $curlOptions).

See the curl manual for more info.

#Comment by Brion VIBBER (talk | contribs)   21:18, 13 January 2009

Marking as OK; re-open if there's still an issue. :)

Status & tagging log