Ticket #3886 (closed defect: fixed)

Opened 2 years ago

Last modified 1 year ago

wrong server protocol for HTTP/1.0 requests resulting in uninterpreted chunked encoding

Reported by: abtime Assigned to: anonymous
Priority: high Milestone: 2.2
Component: General Version: 2.1.1
Severity: major Keywords: has-patch
Cc:

Description

After upgrading to php5.2.1 we found "strange hex numbers" at the beginning and in the middle of the html code, occasionally blank pages, and malfunctioning javascripts. This behaviour was only visible when using a proxy, e.g. squid or Novell Bordermanager.

On closer inspection of the request and response headers, we found out that (some) proxies use HTTP/1.0, but WordPress always returned a HTTP/1.1 header.

We were able to fix this problem by changing the response header in wp-inclues/functions.php in the function status_header(). 

wp-includes/functions.php
-		@header("HTTP/1.1 $header $text", true, $header);
+		@header($_SERVER["SERVER_PROTOCOL"]." ".$header." ".$text, true, $header);

I am not sure if this breaks any other functionality, and whether a patch is needed for earlier php versions.

Attachments

3886.diff (0.5 kB) - added by Nazgul on 02/27/07 21:48:59.
status_header.diff (0.8 kB) - added by ryan on 05/11/07 00:11:13.
status_header.2.diff (1.0 kB) - added by ryan on 05/12/07 21:36:49.

Change History

02/27/07 21:43:07 changed by foolswisdom

  • milestone set to 2.2.

02/27/07 21:48:59 changed by Nazgul

  • attachment 3886.diff added.

02/27/07 21:49:30 changed by Nazgul

  • keywords set to has-patch.

03/05/07 21:07:26 changed by foolswisdom

Frank Helmschrott [wp-testers] wp & proxy & php 5.2.1? described a matching problem scenario in detail, and confirmed that the attached patch resolved the issue.

(follow-up: ↓ 6 ) 03/07/07 03:18:19 changed by ryan

I think some status codes are different with the different protocols. Anything we need to watch out for there?

04/12/07 18:16:40 changed by foolswisdom

  • milestone changed from 2.2 to 2.3.

(in reply to: ↑ 4 ) 05/05/07 00:03:24 changed by rob1n

Replying to ryan:

I think some status codes are different with the different protocols. Anything we need to watch out for there?

http://www.research.att.com/~bala/papers/h0vh1.html might give some info.

Also, 1xx codes were added in 1.1 (http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html) that must "*NOT*" be given to 1.0, apparently.

(follow-up: ↓ 8 ) 05/05/07 08:37:20 changed by azza-bazoo

Hi all,
I think setting the milestone to 2.3 is underestimating the severity of this problem. If gzip compression is on (as per default) and chunked encoding is enabled in Apache, HTTP 1.0 clients that speak gzip won't see anything, because the hex numbers corrupt the gzip stream.

Re ryan: As best as I can tell using grep and WP 2.1.3, calls to the status_header function only ever use these codes, none of which change between HTTP 1.0 and 1.1: 200, 302, 304, 404.

The proposed fix ( $_SERVER["SERVER_PROTOCOL"] ) was introduced in PHP 4.1, and the WP docs list this as a minimum requirement, so I'd expect no problems with PHP versions.

Actually, to support PHP between 4.1 and 4.3, maybe the next line should be changed as well: 

wp-includes/functions.php
-	if ( version_compare(phpversion(), '4.3.0', '>=') )
-		@header("HTTP/1.1 $header $text", true, $header);
-	else
-		@header("HTTP/1.1 $header $text");
+	if ( version_compare(phpversion(), '4.3.0', '>=') )
+		@header($_SERVER["SERVER_PROTOCOL"]." ".$header." ".$text, true, $header);
+	else
+		@header($_SERVER["SERVER_PROTOCOL"]." ".$header." ".$text);

(in reply to: ↑ 7 ) 05/09/07 06:22:43 changed by torbens

  • priority changed from normal to high.
  • milestone changed from 2.3 to 2.2.

Replying to azza-bazoo:

Hi all,
I think setting the milestone to 2.3 is underestimating the severity of this problem.

I see it exactly the same way. Additionally, I've seen quite some installations (including my own) that had the strange 4 digit random number problem described over here: http://wordpress.org/support/topic/114503 These random numbers also mess up RSS feeds and XHTML validation of course.

For me, the patch solved the issue. I suggest to include the patch with v2.2 since it doesn't seem to cause any trouble.

(follow-up: ↓ 10 ) 05/09/07 17:27:27 changed by ryan

Is SERVER_PROTOCOL trusthworty. If not we open ourselves to header injection. Maybe match against the following just to be safe. 

^HTTP/(0\.9|1\.0|1\.1)$

(in reply to: ↑ 9 ) 05/10/07 15:13:05 changed by azza-bazoo

Replying to ryan:

Is SERVER_PROTOCOL trusthworty. If not we open ourselves to header injection.

I believe it is, but haven't checked the PHP source code. Maybe use this code instead, since I don't think there's any software in the wild still using HTTP/0.9: 

	if ( version_compare(phpversion(), '4.3.0', '>=') )
		if ( $_SERVER["SERVER_PROTOCOL"] == "HTTP/1.1" )
			@header("HTTP/1.1 $header $text", true, $header);
		else
			@header("HTTP/1.0 $header $text", true, $header);
	else
		if ( $_SERVER["SERVER_PROTOCOL"] == "HTTP/1.1" )
			@header("HTTP/1.1 $header $text");
		else
			@header("HTTP/1.0 $header $text");

Also, I'd suggest that a fix be put into WP 2.1.4 rather than waiting for 2.2, if that's possible?

05/11/07 00:11:13 changed by ryan

  • attachment status_header.diff added.

(follow-up: ↓ 12 ) 05/11/07 00:11:31 changed by ryan

Attached patch also adds a status_header filter.

(in reply to: ↑ 11 ) 05/11/07 05:27:19 changed by azza-bazoo

I like this patch, but two nitpicks:

  • there's no need to trim() the SERVER_PROTOCOL value
  • If the protocol is unrecognised, it's safer to fall back to HTTP/1.0 than HTTP/1.1 -- wrongly assuming 1.1 was the original cause of this bug, after all.

05/12/07 21:33:57 changed by ryan

I'll remove the trim. Now that we're checking for 1.0, I think falling back to 1.1 as default is okay. That's the most common.

05/12/07 21:36:49 changed by ryan

  • attachment status_header.2.diff added.

05/12/07 21:55:14 changed by ryan

  • status changed from new to closed.
  • resolution set to fixed.

(In [5460]) Return proper protocol. fixes #3886 for 2.3

05/12/07 21:55:38 changed by ryan

After more thought, decided to default to 1.0.

05/12/07 21:58:38 changed by ryan

(In [5461]) Return proper protocol. fixes #3886 for 2.2