WordPress.org

Make WordPress Core

Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#3886 closed defect (bug) (fixed)

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

Reported by: abtime Owned by:
Milestone: 2.2 Priority: high
Severity: major Version: 2.1.1
Component: General Keywords: has-patch
Focuses: 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 (3)

3886.diff (556 bytes) - added by Nazgul 12 years ago.
status_header.diff (801 bytes) - added by ryan 12 years ago.
status_header.2.diff (998 bytes) - added by ryan 12 years ago.

Download all attachments as: .zip

Change History (19)

#1 @foolswisdom
12 years ago

  • Milestone set to 2.2

@Nazgul
12 years ago

#2 @Nazgul
12 years ago

  • Keywords has-patch added

#3 @foolswisdom
12 years ago

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.

#4 follow-up: @ryan
12 years ago

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

#5 @foolswisdom
12 years ago

  • Milestone changed from 2.2 to 2.3

#6 in reply to: ↑ 4 @rob1n
12 years ago

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.

#7 follow-up: @azza-bazoo
12 years ago

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);

#8 in reply to: ↑ 7 @torbens
12 years ago

  • Milestone changed from 2.3 to 2.2
  • Priority changed from normal to high

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.

#9 follow-up: @ryan
12 years ago

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)$

#10 in reply to: ↑ 9 @azza-bazoo
12 years ago

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?

@ryan
12 years ago

#11 follow-up: @ryan
12 years ago

Attached patch also adds a status_header filter.

#12 in reply to: ↑ 11 @azza-bazoo
12 years ago

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.

#13 @ryan
12 years ago

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.

#14 @ryan
12 years ago

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

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

#15 @ryan
12 years ago

After more thought, decided to default to 1.0.

#16 @ryan
12 years ago

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

Note: See TracTickets for help on using tickets.