Proposal for HTTP POST and REQUEST API

This should not have 2.4 milestone.

Replying to santosj:

This should not have 2.4 milestone.

Usually we fix/change things in trunk first (which corresponds with the current 2.4 milestone) and then backport it to previous releases if that's deemed necessary.

So this issue may end up fixed in a maintenance release for 2.3.x, but for now it's first slated for 2.4.

I was thinking more that it doesn't have a patch and is non-trival.

I'll probably bring it up on WP-Hackers and write up some code this weekend. I've been thinking about this ticket and the best way to go about it. I don't think it should be all in one function, but using classes might slow down something that should be made as quick as possible.

Prototype of HTTP Request API

Second Iteration of HTTP API

Changes:

• Created wp_remote_register_transport() and wp_remote_unregister_transport() instead of using a custom Action hook. I might just wrap those two functions around the Action API, but needs references to object.
• Finished four of the five http request methods (missing !cURL).
• Added hooks for user agent changing, as well as setting the different time outs with filters.

Missing:

• !cURL implementation (might be plugin material).
• method to setup what each object can be used for, fopen for PHP 4 can only be used for reading and not writing. Might be useful to decide based on $type used.
• Comprehensive Unit Tests (coming next).
• Missing hooks for parsing headers and body after response is finished (figured some things are better left to the user of the functions, better to return raw, instead of mangling when I shouldn't be).

Found this after writing the first prototype. I like mine better, but okay. The guy has some good stuff and I ported some of it over.

Again, this is just a prototype.

Latest with various bug fixes, but still prototype. WORKING!

Unit Test for HTTP Request API for Automattic WordPress Functional Test Suite

Totally sweet! It works! That just blows my mind.

Unit Test covers most of the functions, does not cover wp_remote_get_body() or wp_remote_get_headers(). The Unit Tests informally cover the WP_HTTP_Fsockopen class, the functions cover the test() and the wp_remote_get_object() covers the request() method. Which does prove that the class does work. However, to complete the unit tests, the WP_HTTP_Base class methods need to be covered, as well as the rest of the transport classes.

The next patch, if this is still considered okay, will include complete unit tests and replace Snoopy and transition areas that use fsockopen to use these functions. It will also include splitting the five classes down to one or two (cURL might be useful to be packaged also).

Based off of Automattic WordPress Test Suite, tests ~80% of HTTP API

Polished version of HTTP request API

Changes WordPress core functions and Admin over to new HTTP API, based off of r6350

Production Quality Patch that moves extra HTTP classes to plugin.

Tests 99% (missing wp_remote_post_object() but it works the same as wp_remote_get_object(), so no worries) of HTTP API, Warning, tests live HTTP connections, so is slow

Plugin which uses HTTP API

Unit Tests cover 99.9% over the HTTP API, the API is finished and works. The patch that changes the core of WordPress needs testing, but I'm planning on writing Unit Tests for that. Also need to go over performance tuning, but I'm not sure how much that will do.

Parts that need consideration:

• Using the Plugin API instead of custom array
• Performance Fine-tuning

Is there any chance for 2.6 WordPress can standardise on external access?

Currently WordPress has:

• Snoopy
• fsockopen() in various function
• Curl
• fopen()
• A Wrapper around Snoopy
• A Wrapper around Curl && fopen() (I swear i saw that)

Every piece of code tends to use a different one, Not only does that mean that certain code may work while another doesnt, It also doesnt help with those who want to set a proxy server, If it was wrapped up into a single function/method, It would greatly help those users.

There was never much traction on this ticket from Devs, But it'd be good to get a final word as to if anyone is interested.

I'm not too sure if the patches on here are the best though, But that depends on the direction devs want to take it. I'd be just as happy if fsockopen() was bound together into a single function which everything used(I use this in a few plugins):

function remote_http($url, $method='GET', $data='', $headers = array()){
	//Note: This returns all headers in lowercase.
	global $wp_version;
	//Partly stolen from update checker code :)
	if( is_array($data) )
		$data = http_build_query($data);

	$allowed_methods = array('GET', 'POST', 'HEAD');
	if( ! in_array($method, $allowed_methods) ) $method = 'GET';

	$port = 80;
	$schema = 'http';
	$path = $host = $query = '';
	$parts = parse_url($url);
	extract($parts);

	$http_request  = "$method $path?$query HTTP/1.0\r\n";
	$http_request .= "Host: $host\r\n";
	$http_request .= 'Content-Type: application/x-www-form-urlencoded; charset=' . get_option('blog_charset') . "\r\n";
	if( $data )
		$http_request .= 'Content-Length: ' . strlen($request) . "\r\n";
	$http_request .= 'User-Agent: WordPress/' . $wp_version . '; ' . '; ' . get_bloginfo('url') . "\r\n";
	if( ! empty($headers) )
		foreach($headers as $header => $value)
			$http_request .= "$header: $value\r\n";
	$http_request .= "\r\n";
	if( $data )
		$http_request .= $request;

	$response = '';
	if( false == ( $fs = @fsockopen( $host, $port, $errno, $errstr, 3) ) || ! is_resource($fs) )
		return false;

	fwrite($fs, $http_request);

	while ( !feof($fs) )
		$response .= fgets($fs, 1160); // One TCP-IP packet
	fclose($fs);
	$response = explode("\r\n\r\n", $response, 2);
	
	$ret = (object)array('headers' => array(), 'content'=> $response[1]);

	foreach( explode("\n", $response[0]) as $rheader){
		list($header, $value) = explode(':', $rheader, 2);
		if( !empty($header) && ! empty($value) ) {
			$ret->headers[ strtolower(trim($header)) ] = trim($value);
		} else if( strpos($rheader, 'HTTP') > -1) {
			list(,$status, $status_name) = preg_split('|\s+|', $rheader);
			$ret->headers[ 'status' ] = $status;
			$ret->headers[ 'status-name' ] = $status_name;
		}
	}

	return $ret;
}

At least it could then be improved on in the future if need be, But why not do it right, and get it in there now?

Your code is beautiful DD32.

I've been looking to set the code to use a function instead, but couldn't drive myself to steep to that level. I didn't think it would combine the feature set, but alas, you were able to do it and quite well.

$http_request .= 'Content-Type: application/x-www-form-urlencoded; charset=' . get_option('blog_charset') . "\r\n";

• Needs to be wrapped in a conditional.
• The header support needs to also handle strings.

Other than that I think I can wrap the code in the patch into a pluggable function and create another patch.

I want to get this in and I would like to get rid of Snoopy.

Other than that I think I can wrap the code in the patch into a pluggable function and create another patch.

I might just make a few changes to the code as you sugest and submit as a patch instead? Unless you're allready half way there of course? :)

attachment 4779.remote_http.diff added.

• Includes PHPDoc
• Fixed a number of warnings & bugs related to POST request
• Status and Status Value are no longer returned in the headers list, but are properties themselves.

Example return value:

$a = remote_http('http://api.wordpress.org/core/version-check/1.1/');

var_dump($a);

object(stdClass)[70]
  public 'status' => string '200' (length=3)
  public 'statusvalue' => string 'OK' (length=2)
  public 'headers' => 
    array
      'x-powered-by' => string 'PHP/5.2.5' (length=9)
      'content-type' => string 'text/plain; charset=utf-8' (length=25)
      'content-length' => string '44' (length=2)
      'date' => string 'Sun, 25 May 2008 04:46:16 GMT' (length=29)
      'server' => string 'LiteSpeed' (length=9)
      'connection' => string 'close' (length=5)
  public 'content' => string 'upgrade
http://wordpress.org/download/&#' (length=44)

(note: content field is truncated via var_dump due to a bug in the XDebug extension in that example)

I have a few suggestions:

1. Add another parameter which adds a few other options, one of which for what HTTP version to send, however I must admit version 1.0 is a lot easier to handle.
2. If $headers is not an array, then it will be cast to an array. I would rather like this to be a string also. If that is the case, then other handling can be done. There is a function used in WordPress that will merge both strings or array into another array. Forget the name of it, but it is used throughout WordPress where string|array are used.
3. The data will not always be URL encoded content, might be XML, so might need another function which handles data like that for the user to make it easier. However, I would leave that as an exercise for the developer. If I send XML, I don't want to have to hack around the current implementation.
4. Your current fsockopen does not handle redirection correctly and leaves that as an exercise for the user. This either needs to be noted in the Documentation or handled within the function. The problem is that most other HTTP handlers handle this automatically, so it might be problematic to the user to have to handle it when it will be just for one HTTP handler. I would do it for the user. You can see my current code which handles it (kind of nasty code).
5. There is a typo in the documentation

Other than that it seems to be to be pretty awesome. I would rather the dev team accept the object oriented approach, but I would rather see this ticket closed.

Add another parameter which adds a few other options, one of which for what HTTP version to send, however I must admit version 1.0 is a lot easier to handle.

I'm not up to speed with the HTTP 1.1 spec, Content-Encoding: deflate/chunked/etc just confuse me from a client perspective. It'd be nice to support gzipped data if possible though i guess.

If $headers is not an array, then it will be cast to an array. I would rather like this to be a string also. If that is the case, then other handling can be done.

wp_parse_args() i believe? Yeah, That'd be a better option actually.

The data will not always be URL encoded content ..[snip].. If I send XML, I don't want to have to hack around the current implementation.

Thats handled allready, Pass a Array and its converted to a URL encoded query string. Pass a string and its assumed you know what you want to pass, so it'll post the exact Serialized data or XML. (Unless i've stupidly done something i've not noticed?)

Your current fsockopen does not handle redirection correctly and leaves that as an exercise for the user. ..[snip]

Point taken, It would be best to follow a certain ammount of redirects like some of the WP functions do.

There is a typo in the documentation

oops, thanks for noting that :)

I would rather the dev team accept the object oriented approach

I'd like to see something thats useable by all people accepted. I have a feeling one of the reasons the OO one hasnt been used so far, is that it *may* be seen to by some as being a more complicated option that what currently exists.

Replying to DD32:

I'm not up to speed with the HTTP 1.1 spec, Content-Encoding: deflate/chunked/etc just confuse me from a client perspective. It'd be nice to support gzipped data if possible though i guess.

I'm not saying that the function handle all of that, I'm saying that we allow that option for the developer instead of forcing 1.0. If the developer understands the spec, then they'll probably not want to recreate another function just to send data and parse it back. If the HTTP 1.1 spec requires that chunked data be handled within the function, then perhaps it is better to maintain the 1.0 version.

GZipped can still be handled by the developer. The goal is to just handle sending and receiving, the user has to determine what to do with the response after that. It would become a massive function or library if the function did everything for the developer.

The data will not always be URL encoded content ..[snip].. If I send XML, I don't want to have to hack around the current implementation.

Thats handled allready, Pass a Array and its converted to a URL encoded query string. Pass a string and its assumed you know what you want to pass, so it'll post the exact Serialized data or XML. (Unless i've stupidly done something i've not noticed?)

I rechecked and you are correct, it is completely possible. It should still be noted in the documentation that it is the case for future reference.

Your current fsockopen does not handle redirection correctly and leaves that as an exercise for the user. ..[snip]

Point taken, It would be best to follow a certain ammount of redirects like some of the WP functions do.

That could be part of the optional last option parameter with the default at three.

attachment 4779.remote_http.2.diff added.

1. Changed 2nd param from a simple Type selector to a generic options passer, most values can be changed via that.
2. Added code to follow redirects.
3. If a dev wants to, they can set any version of HTTP to use. They may also accept Gzip data/etc by setting the appropriate headers. (Via #1)
4. Added Examples in the PHPDoc and corrected spelling mistakes
5. Timeout for the connection can be specified (Via #1)

Pretty sweet.

One more thing, pluggable functions are wrapped using the colon...endif; blocks.

if( function_exists(...) :
// ...
endif;

and not with curly brackets.

One more thing, pluggable functions are wrapped using the colon...endif; blocks.

Ignoring the current pluggable.php format, Whats the stance on the use of endif; in WordPress?
The coding standards mearly state the preferential use of the Braces, the longhand format of if () : endif; is not mentioned, nor is it mentioned in the PEAR docs. It makes sense in some places in the code (Some of the template functions where a large ammount of code is not fully indented, or has many levels) But the longhand method is also used in some places where it's really doesnt make sense to use it.(No examples off hand)

Any core dev carification on that? if () { ... } for short blocks, if (): endif; for longer blocks?

I'm fine for whatever, But i wont bother updateing the patch unless a dev actually indicates that its likely to be commited(Or a bug/feature gets added to current patch :))

I think the WP standard is to always use braces except in the following two circumstances:

1. When there is "raw" HTML within the block. That is, when there is closing and opening PHP tags ?>, <?php. This occurs in many templates and template functions.
2. Wrapped around pluggable functions.

I'm sure that there are places where WP does not adhere to that standard.

As an aside, I think it's WP standard to use no braces at all for one line loops/conditionals:

if ( $foo )
  do_something();

foreach ( $bobs as $bob )
  bob_it( $bob );

That is probably less adhered to (and more controversial) than the braces/long format issue above.

I mentioned it because it is the standard in the pluggables file. I don't think it matters whether the all encompassing standard is, but what the rest of file standard is and how it is done in that file.

I would rather not think that it should either one or the other. I believe the preference was chosen to not have to indent the functions in the if block.

Replying to jacobsantos:

I mentioned it because it is the standard in the pluggables file. I don't think it matters whether the all encompassing standard is, but what the rest of file standard is and how it is done in that file. I would rather not think that it should either one or the other. I believe the preference was chosen to not have to indent the functions in the if block.

I totally need to get some sleep.

I totally need to get some sleep.

True :P But it makes sense.

I'm sure that there are places where WP does not adhere to that standard.

Yep, I was just wondering why that standard was different in the pluggables, and to a certain extend, the compat.php too(Which uses a combination).

The indentation changes are a good one to pointout actually jacob.

I've personally used this method previously (And is my prefered method, but doesnt fit in with WP):

if( ! function_exists() ) {
function ..() {
...
}} //Double curly close

I'll change the style over if theres any more grip from that patch.

Need to get on IRC.

Falling back to other methods out-of-the-box, as per the original patches, would be nice. I shouldn't have to install a plugin because my fsockopen doesn't work.

See also: #4011 (add global proxy support in options)

Looks like this isnt going to go in 2.6.. Can something be done about this early in 2.7? Something really needs to be standardised upon both for core, and for plugins to rely on.

I should point out that I would like to either see this ticket die or be implemented. I'm unsure what to make of this ticket. For the entire time I went with the object oriented approach I didn't receive a word of feedback. Now with the procedural implementation there is some feedback that basically says that the implementation should be changed, so that it looks like crap.

If the OO implementation was "The Suck," it isn't any worse or better than the current BackPress?. I'm unsure whether or not a similar implementation is going to be written for BackPress? and included in WordPress.

If I may be curious and submit a second proposal with the object oriented approach along with the BackPress? style along with the procedural implementation which implements all four or five methods.

You would think a fully documented API with test cases would score at least some points!

I should point out that I would like to either see this ticket die or be implemented.

Same here, That was the only reason i put forward a simple function, If the OO method wasnt going to make it, I'd at least have liked to see a simplistic function work its way in instead.

For the entire time I went with the object oriented approach I didn't receive a word of feedback.

I noticed that at the time, I just hoped that it meant that no-one had anything against it, And that it was a nice track to follow.

Maybe a list needs to be made for what *would* constitute a good API, and take it from there:

• Single function call to retrieve a simple document
• Automatic fallback to other methods in order of preference
• GET/HEAD/POST fully supported and useable under each fallback
• Able to be used by all core files without issue

the scattered fsockopen's are working pretty well on the majority of hosts at present, However, it lacks Proxy support (an extra 2 lines in each really..), different sections of code may use a different method (ie. updater uses fsockopen, ping uses remote_url_open & fopen() AFAIK) which leads to one section working, and another not.

Maybe some feedback from actual devs with reasons for/against the provided method, and/or what would need to be present in any approaches put forward

I'm fine with either approach as long as it does its best to work with whatever transports are available. I want this to work out of the box for as many people as possible. The OOP patches seem to do that so that's a point in their favor as far as I'm concerned.

If we can pick an approach, we might be able to put this in 2.6 and use it in one or two lower-impact spots in the core code to try it out. We can migrate everything over in 2.7.

Sounds reassuring.

I believe it will be about two hours work to extend the test cases to cover the changes to the API. Also, use some of the code from DD32's patch in with the old code and refresh the old code. The cURL implementation also needs to be completed and test cases written for it also.