From 5146d9f8ac170d8ba7109370d732d56dc0777578 Mon Sep 17 00:00:00 2001 From: Rowan Collins Date: Wed, 30 Mar 2016 22:12:03 +0000 Subject: [PATCH] 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. --- NEWS | 6 +- ext/standard/http_fopen_wrapper.c | 56 ++++++++--- ext/standard/tests/http/bug47021.phpt | 93 +++++++++++++++++++ .../tests/http/http_response_header_04.phpt | 37 ++++++++ .../tests/http/http_response_header_05.phpt | 37 ++++++++ 5 files changed, 212 insertions(+), 17 deletions(-) create mode 100644 ext/standard/tests/http/bug47021.phpt create mode 100644 ext/standard/tests/http/http_response_header_04.phpt create mode 100644 ext/standard/tests/http/http_response_header_05.phpt diff --git a/NEWS b/NEWS index 8a297b98f22..aff159edf51 100644 --- a/NEWS +++ b/NEWS @@ -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 diff --git a/ext/standard/http_fopen_wrapper.c b/ext/standard/http_fopen_wrapper.c index 0c996d8eae5..1ed7dc2b808 100644 --- 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 { diff --git a/ext/standard/tests/http/bug47021.phpt b/ext/standard/tests/http/bug47021.phpt new file mode 100644 index 00000000000..6cdfda6a6b5 --- /dev/null +++ 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-- + +--FILE-- + [ + '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 + diff --git a/ext/standard/tests/http/http_response_header_04.phpt b/ext/standard/tests/http/http_response_header_04.phpt new file mode 100644 index 00000000000..e8f22e00b14 --- /dev/null +++ b/ext/standard/tests/http/http_response_header_04.phpt @@ -0,0 +1,37 @@ +--TEST-- +$http_reponse_header (header with trailing whitespace) +--SKIPIF-- + +--INI-- +allow_url_fopen=1 +allow_url_include=1 +--FILE-- + +==DONE== +--EXPECT-- +string(4) "Body" +array(2) { + [0]=> + string(15) "HTTP/1.0 200 Ok" + [1]=> + string(14) "Some: Header" +} +==DONE== + diff --git a/ext/standard/tests/http/http_response_header_05.phpt b/ext/standard/tests/http/http_response_header_05.phpt new file mode 100644 index 00000000000..27ba77adcc4 --- /dev/null +++ b/ext/standard/tests/http/http_response_header_05.phpt @@ -0,0 +1,37 @@ +--TEST-- +$http_reponse_header (whitespace-only "header") +--SKIPIF-- + +--INI-- +allow_url_fopen=1 +allow_url_include=1 +--FILE-- + +==DONE== +--EXPECT-- +string(4) "Body" +array(2) { + [0]=> + string(15) "HTTP/1.0 200 Ok" + [1]=> + string(0) "" +} +==DONE== +