http_fopen_wrapper.c - Handle HTTP headers with varying white space

The stream handler assumed all HTTP headers contained exactly one space, but the standard says there may be zero or more. Should fix Bug #47021, and any other edge cases caused by a web server sending unusual spacing, e.g. the MIME type discovered from Content-Type: can no longer contain leading whitespace. We strip trailing whitespace from the headers added into $http_response_header as well.
2024-11-24 02:15:04 +08:00 · 2016-03-30 22:12:03 +00:00 · 2016-03-30 22:12:03 +00:00 · 5146d9f8ac
commit 5146d9f8ac
parent a46bbdda2e
5 changed files with 212 additions and 17 deletions
--- a/6
+++ b/6
@ -20,10 +20,12 @@ PHP                                                                        NEWS

 - Standard:
  . Fixed bug #69442 (closing of fd incorrect when PTS enabled). (jaytaph)
+  . Fixed bug #47021 (SoapClient stumbles over WSDL delivered with
+    "Transfer-Encoding: chunked"). (Rowan Collins)

 - ZIP:
-	. Fixed bug #70103 (ZipArchive::addGlob ignores remove_all_path option). (cmb,
-	  Mitch Hagstrand)
+  . Fixed bug #70103 (ZipArchive::addGlob ignores remove_all_path option). (cmb,
+    Mitch Hagstrand)

 19 Jan 2017 PHP 7.0.15

--- a/ext/standard/http_fopen_wrapper.c
+++ b/ext/standard/http_fopen_wrapper.c
@ -755,8 +755,10 @@ finish:

 	while (!body && !php_stream_eof(stream)) {
 		size_t http_header_line_length;
+		
 		if (php_stream_get_line(stream, http_header_line, HTTP_HEADER_BLOCK_SIZE, &http_header_line_length) && *http_header_line != '\n' && *http_header_line != '\r') {
 			char *e = http_header_line + http_header_line_length - 1;
+			char *http_header_value;
 			if (*e != '\n') {
 				do { /* partial header */
 					if (php_stream_get_line(stream, http_header_line, HTTP_HEADER_BLOCK_SIZE, &http_header_line_length) == NULL) {
@ -770,26 +772,54 @@ finish:
 			while (*e == '\n' || *e == '\r') {
 				e--;
 			}
-			http_header_line_length = e - http_header_line + 1;
-			http_header_line[http_header_line_length] = '\0';

-			if (!strncasecmp(http_header_line, "Location: ", 10)) {
+			/* The primary definition of an HTTP header in RFC 7230 states:
+			 * > Each header field consists of a case-insensitive field name followed
+			 * > by a colon (":"), optional leading whitespace, the field value, and
+			 * > optional trailing whitespace. */
+
+			/* Strip trailing whitespace */
+			while (*e == ' ' || *e == '\t') {
+				e--;
+			}
+
+			/* Terminate header line */
+			e++;
+			*e = '\0';
+			http_header_line_length = e - http_header_line;
+
+			http_header_value = memchr(http_header_line, ':', http_header_line_length);
+			if (http_header_value) {
+				http_header_value++; /* Skip ':' */
+
+				/* Strip leading whitespace */
+				while (http_header_value < e
+						&& (*http_header_value == ' ' || *http_header_value == '\t')) {
+					http_header_value++;
+				}
+			}
+
+			if (!strncasecmp(http_header_line, "Location:", sizeof("Location:")-1)) {
 				if (context && (tmpzval = php_stream_context_get_option(context, "http", "follow_location")) != NULL) {
 					follow_location = zval_is_true(tmpzval);
-				} else if (!((response_code >= 300 && response_code < 304) || 307 == response_code || 308 == response_code)) {
+				} else if (!((response_code >= 300 && response_code < 304)
+						|| 307 == response_code || 308 == response_code)) {
 					/* we shouldn't redirect automatically
 					if follow_location isn't set and response_code not in (300, 301, 302, 303 and 307)
 					see http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.3.1
 					RFC 7238 defines 308: http://tools.ietf.org/html/rfc7238 */
 					follow_location = 0;
 				}
-				strlcpy(location, http_header_line + 10, sizeof(location));
-			} else if (!strncasecmp(http_header_line, "Content-Type: ", 14)) {
-				php_stream_notify_info(context, PHP_STREAM_NOTIFY_MIME_TYPE_IS, http_header_line + 14, 0);
-			} else if (!strncasecmp(http_header_line, "Content-Length: ", 16)) {
-				file_size = atoi(http_header_line + 16);
+				strlcpy(location, http_header_value, sizeof(location));
+			} else if (!strncasecmp(http_header_line, "Content-Type:", sizeof("Content-Type:")-1)) {
+				php_stream_notify_info(context, PHP_STREAM_NOTIFY_MIME_TYPE_IS, http_header_value, 0);
+			} else if (!strncasecmp(http_header_line, "Content-Length:", sizeof("Content-Length")-1)) {
+				file_size = atoi(http_header_value);
 				php_stream_notify_file_size(context, file_size, http_header_line, 0);
-			} else if (!strncasecmp(http_header_line, "Transfer-Encoding: chunked", sizeof("Transfer-Encoding: chunked"))) {
+			} else if (
+				!strncasecmp(http_header_line, "Transfer-Encoding:", sizeof("Transfer-Encoding")-1)
+				&& !strncasecmp(http_header_value, "Chunked", sizeof("Chunked")-1)
+			) {

 				/* create filter to decode response body */
 				if (!(options & STREAM_ONLY_GET_HEADERS)) {
@ -808,13 +838,9 @@ finish:
 				}
 			}

-			if (http_header_line[0] == '\0') {
-				body = 1;
-			} else {
+			{
 				zval http_header;
-
 				ZVAL_STRINGL(&http_header, http_header_line, http_header_line_length);
-
 				zend_hash_next_index_insert(Z_ARRVAL(response_header), &http_header);
 			}
 		} else {
--- a/ext/standard/tests/http/bug47021.phpt
+++ b/ext/standard/tests/http/bug47021.phpt
@ -0,0 +1,93 @@
+--TEST--
+Bug #47021 (SoapClient stumbles over WSDL delivered with "Transfer-Encoding: chunked")
+--INI--
+allow_url_fopen=1
+--SKIPIF--
+<?php require 'server.inc'; http_server_skipif('tcp://127.0.0.1:12342'); ?>
+--FILE--
+<?php
+require 'server.inc';
+
+function stream_notification_callback($notification_code, $severity, $message, $message_code, $bytes_transferred, $bytes_max) {
+
+    switch($notification_code) {
+        case STREAM_NOTIFY_MIME_TYPE_IS:
+            echo "Type='$message'\n";
+	    break;
+        case STREAM_NOTIFY_FILE_SIZE_IS:
+            echo "Size=$bytes_max\n";
+            break;
+    }
+}
+
+function do_test($num_spaces, $leave_trailing_space=false) {
+  // SOAPClient exhibits the bug because it forces HTTP/1.1,
+  // whereas file_get_contents() uses HTTP/1.0 by default.
+  $options = [
+    'http' => [
+      'protocol_version' => '1.1',
+      'header' => 'Connection: Close'
+    ],
+  ];
+
+  $ctx = stream_context_create($options);
+  stream_context_set_params($ctx, array("notification" => "stream_notification_callback"));
+
+  $spaces = str_repeat(' ', $num_spaces);
+  $trailing = ($leave_trailing_space ? ' ' : '');
+  $responses = [
+    "data://text/plain,HTTP/1.1 200 OK\r\n"
+      . "Content-Type:{$spaces}text/plain{$trailing}\r\n"
+      . "Transfer-Encoding:{$spaces}Chunked{$trailing}\r\n\r\n"
+      . "5\nHello\n0\n",
+    "data://text/plain,HTTP/1.1 200 OK\r\n"
+      . "Content-Type\r\n" // Deliberately invalid header
+      . "Content-Length:{$spaces}5{$trailing}\r\n\r\n"
+      . "World"
+  ];
+  $pid = http_server('tcp://127.0.0.1:12342', $responses);
+
+  echo file_get_contents('http://127.0.0.1:12342/', false, $ctx);
+  echo "\n";
+  echo file_get_contents('http://127.0.0.1:12342/', false, $ctx);
+  echo "\n";
+
+  http_server_kill($pid);
+}
+
+// Chunked decoding should be recognised by the HTTP stream wrapper regardless of whitespace
+// Transfer-Encoding:Chunked
+do_test(0);
+echo "\n";
+// Transfer-Encoding: Chunked
+do_test(1);
+echo "\n";
+// Transfer-Encoding:  Chunked
+do_test(2);
+echo "\n";
+// Trailing space at end of header
+do_test(1, true);
+echo "\n";
+
+?>
+--EXPECT--
+Type='text/plain'
+Hello
+Size=5
+World
+
+Type='text/plain'
+Hello
+Size=5
+World
+
+Type='text/plain'
+Hello
+Size=5
+World
+
+Type='text/plain'
+Hello
+Size=5
+World
+
--- a/ext/standard/tests/http/http_response_header_04.phpt
+++ b/ext/standard/tests/http/http_response_header_04.phpt
@ -0,0 +1,37 @@
+--TEST--
+$http_reponse_header (header with trailing whitespace)
+--SKIPIF--
+<?php require 'server.inc'; http_server_skipif('tcp://127.0.0.1:22349'); ?>
+--INI--
+allow_url_fopen=1
+allow_url_include=1
+--FILE--
+<?php
+require 'server.inc';
+
+$responses = array(
+	"data://text/plain,HTTP/1.0 200 Ok\r\nSome:   Header   \r\n\r\nBody",
+);
+
+$pid = http_server("tcp://127.0.0.1:22349", $responses, $output);
+
+function test() {
+    $f = file_get_contents('http://127.0.0.1:22349/');
+    var_dump($f);
+    var_dump($http_response_header);
+}
+test();
+
+http_server_kill($pid);
+?>
+==DONE==
+--EXPECT--
+string(4) "Body"
+array(2) {
+  [0]=>
+  string(15) "HTTP/1.0 200 Ok"
+  [1]=>
+  string(14) "Some:   Header"
+}
+==DONE==
+
--- a/ext/standard/tests/http/http_response_header_05.phpt
+++ b/ext/standard/tests/http/http_response_header_05.phpt
@ -0,0 +1,37 @@
+--TEST--
+$http_reponse_header (whitespace-only "header")
+--SKIPIF--
+<?php require 'server.inc'; http_server_skipif('tcp://127.0.0.1:22350'); ?>
+--INI--
+allow_url_fopen=1
+allow_url_include=1
+--FILE--
+<?php
+require 'server.inc';
+
+$responses = array(
+	"data://text/plain,HTTP/1.0 200 Ok\r\n    \r\n\r\nBody",
+);
+
+$pid = http_server("tcp://127.0.0.1:22350", $responses, $output);
+
+function test() {
+    $f = file_get_contents('http://127.0.0.1:22350/');
+    var_dump($f);
+    var_dump($http_response_header);
+}
+test();
+
+http_server_kill($pid);
+?>
+==DONE==
+--EXPECT--
+string(4) "Body"
+array(2) {
+  [0]=>
+  string(15) "HTTP/1.0 200 Ok"
+  [1]=>
+  string(0) ""
+}
+==DONE==
+