WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#9044 closed defect (bug) (fixed)

http_build_query: third arg not added until PHP v. 5.1.2

Reported by: mediafetish Owned by:
Milestone: 2.8 Priority: normal
Severity: blocker Version: 2.7
Component: HTTP API Keywords: has-patch
Focuses: Cc:

Description

In wp-includes/compat.php:

// Added in PHP 5.0

if (!function_exists('http_build_query')) {
	function http_build_query($data, $prefix=null, $sep=null) {
		return _http_build_query($data, $prefix, $sep);
	}
}

But according to: http://us2.php.net/http_build_query

The third argument wasn't added until PHP Version 5.1.2

This caused my install to fail when trying to use the automatic plugin upgrade function. In research, I found several occurrences of others fighting with this error:

Warning: http_build_query() expects at most 2 parameters, 3 given in /stor/ezines/htdocs/wp-includes/http.php on line 248

Line 248 has:

$r['body'] = http_build_query($r['body'], null, '&');

I believe this should call _http_build_query:

$r['body'] = _http_build_query($r['body'], null, '&');

Which fixed my installation.

I'm not sure if this is accurate but I also believe that the logic in compat.php to test for the function http_build_query should also test for php version... something like:

if (!function_exists('http_build_query') || version_compare(PHP_VERSION, '5.1.2') === -1 ) {
	if(function_exists('http_build_query')) {
		override_function('http_build_query', 
							'$data, $prefix=null, $sep=null',
							'return _http_build_query($data, $prefix, $sep);');
	}else{
		function http_build_query($data, $prefix=null, $sep=null) {
			return _http_build_query($data, $prefix, $sep);
		}
	}
}

But that requires APD functions to override the native function so I don't know if that's the best approach or if it's even necessary since http.php was the only file I could find that called http_build_query directly. I'll leave that up to the gurus I guess ;)

Attachments (3)

http_remove_seperator.patch (579 bytes) - added by tomontoast 5 years ago.
Removed seperator argument from http.php
9044.patch (753 bytes) - added by hakre 5 years ago.
Version Check in http.php
http.separator.fix.patch (740 bytes) - added by Simek 5 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 follow-up: jacobsantos5 years ago

Dude, what about upgrading to like at least 5.2.3?

But yeah, it should use the _http_build_query().

comment:2 in reply to: ↑ 1 mediafetish5 years ago

Replying to jacobsantos:

Dude, what about upgrading to like at least 5.2.3?

But yeah, it should use the _http_build_query().

Upgrading isn't an issue for me - but it might be for some. I submitted it as a bug because there is a case where it doesn't work and not everyone has control over their PHP version.

tomontoast5 years ago

Removed seperator argument from http.php

comment:3 Denis-de-Bernardy5 years ago

  • Keywords has-patch tested dev-feedback added; http_build_query bug http plugins removed
  • Milestone changed from 2.7.2 to 2.8

comment:4 follow-up: ryan5 years ago

  • Keywords has-patch tested removed

We must force the separator to be '&'. We'll have to use _http_build_query() then. Maybe add a version check and have it call the builtin http_build_query() if the version is >= 5.1.2.

comment:5 Denis-de-Bernardy5 years ago

  • Keywords needs-patch added; dev-feedback removed

comment:6 Denis-de-Bernardy5 years ago

  • Severity changed from normal to blocker

comment:7 in reply to: ↑ 4 hakre5 years ago

Replying to ryan:

We must force the separator to be '&'. We'll have to use _http_build_query() then. Maybe add a version check and have it call the builtin http_build_query() if the version is >= 5.1.2.

'&' is the default seperator with PHP. So it must not be explicitly named here. Or do I get something wrong here?

hakre5 years ago

Version Check in http.php

comment:8 hakre5 years ago

  • Keywords has-patch added; needs-patch removed

comment:9 ryan5 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [11143]) Use _http_build_query() if PHP version < 5.1.2. Props hakre. fixes #9044

comment:10 Simek5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

According to http://us2.php.net/http_build_query we should use "&amp;" as separator instead of "&".

Simek5 years ago

comment:11 hakre5 years ago

Simek: the &amp; is for XHTML output only, not for communicating with the server. not when a URL is build that is send to a server. The class is for the later.

comment:12 ryan5 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Indeed. &amp; is not appropriate in this context. It causes things to break. That's why we force '&' so we don't pick up the arg separator setting from php.ini.

Note: See TracTickets for help on using tickets.