From 5560fddc86c1fbbc53ab57e885acbd5e879077a8 Mon Sep 17 00:00:00 2001 From: Michael Wallner Date: Fri, 13 Feb 2015 09:50:40 +0100 Subject: [PATCH 1/1] fix Content-Range messages; remove superfluous Content-Lenght:0 header --- php_http_message.c | 4 ++ php_http_message_parser.c | 58 ++++++++++++++++---------- php_http_message_parser.h | 1 + tests/client019.phpt | 1 - tests/client020.phpt | 1 - tests/client025.phpt | 4 +- tests/data/message_r_content_range.txt | 9 ++++ tests/message001.phpt | 22 ++-------- tests/message016.phpt | 24 +++++++++++ tests/messageparser001.phpt | 12 +++++- tests/messageparser002.phpt | 2 +- 11 files changed, 92 insertions(+), 46 deletions(-) create mode 100644 tests/data/message_r_content_range.txt create mode 100644 tests/message016.phpt diff --git a/php_http_message.c b/php_http_message.c index 5a278f6..efdf053 100644 --- a/php_http_message.c +++ b/php_http_message.c @@ -297,6 +297,9 @@ void php_http_message_update_headers(php_http_message_t *msg) if (php_http_message_body_stream(msg->body)->readfilters.head) { /* if a read stream filter is attached to the body the caller must also care for the headers */ + } else if ((h = php_http_message_header(msg, ZEND_STRL("Content-Range"), 0))) { + /* don't mess around with a Content-Range message */ + zval_ptr_dtor(&h); } else if ((size = php_http_message_body_size(msg->body))) { MAKE_STD_ZVAL(h); ZVAL_LONG(h, size); @@ -324,6 +327,7 @@ void php_http_message_update_headers(php_http_message_t *msg) zval_ptr_dtor(&h); if (Z_LVAL_P(h_cpy)) { + /* body->size == 0, so get rid of old Content-Length */ zend_hash_del(&msg->hdrs, "Content-Length", sizeof("Content-Length")); } zval_ptr_dtor(&h_cpy); diff --git a/php_http_message_parser.c b/php_http_message_parser.c index 086ce0c..ce2a515 100644 --- a/php_http_message_parser.c +++ b/php_http_message_parser.c @@ -30,12 +30,13 @@ static const php_http_message_parser_state_spec_t php_http_message_parser_states {PHP_HTTP_MESSAGE_PARSER_STATE_BODY_LENGTH, 1}, {PHP_HTTP_MESSAGE_PARSER_STATE_BODY_CHUNKED, 1}, {PHP_HTTP_MESSAGE_PARSER_STATE_BODY_DONE, 0}, + {PHP_HTTP_MESSAGE_PARSER_STATE_UPDATE_CL, 0}, {PHP_HTTP_MESSAGE_PARSER_STATE_DONE, 0} }; #if DBG_PARSER const char *php_http_message_parser_state_name(php_http_message_parser_state_t state) { - const char *states[] = {"START", "HEADER", "HEADER_DONE", "BODY", "BODY_DUMB", "BODY_LENGTH", "BODY_CHUNK", "BODY_DONE", "DONE"}; + const char *states[] = {"START", "HEADER", "HEADER_DONE", "BODY", "BODY_DUMB", "BODY_LENGTH", "BODY_CHUNK", "BODY_DONE", "UPDATE_CL", "DONE"}; if (state < 0 || state > (sizeof(states)/sizeof(char*))-1) { return "FAILURE"; @@ -168,6 +169,7 @@ php_http_message_parser_state_t php_http_message_parser_parse_stream(php_http_me case PHP_HTTP_MESSAGE_PARSER_STATE_BODY: case PHP_HTTP_MESSAGE_PARSER_STATE_BODY_DONE: + case PHP_HTTP_MESSAGE_PARSER_STATE_UPDATE_CL: /* should not occur */ abort(); break; @@ -252,23 +254,32 @@ php_http_message_parser_state_t php_http_message_parser_parse(php_http_message_p { zval *h, *h_loc = NULL, *h_con = NULL, **h_cl = NULL, **h_cr = NULL, **h_te = NULL; + /* Content-Range has higher precedence than Content-Length, + * and content-length denotes the original length of the entity, + * so let's *NOT* remove CR/CL, because that would fundamentally + * change the meaning of the whole message + */ if ((h = php_http_message_header(*message, ZEND_STRL("Transfer-Encoding"), 1))) { - zend_hash_update(&(*message)->hdrs, "X-Original-Transfer-Encoding", sizeof("X-Original-Transfer-Encoding"), &h, sizeof(zval *), (void *) &h_te); + zend_hash_update(&(*message)->hdrs, "X-Original-Transfer-Encoding", sizeof("X-Original-Transfer-Encoding"), (void *) &h, sizeof(zval *), (void *) &h_te); zend_hash_del(&(*message)->hdrs, "Transfer-Encoding", sizeof("Transfer-Encoding")); + + /* reset */ + MAKE_STD_ZVAL(h); + ZVAL_LONG(h, 0); + zend_hash_update(&(*message)->hdrs, "Content-Length", sizeof("Content-Length"), (void *) &h, sizeof(zval *), NULL); + } else if ((h = php_http_message_header(*message, ZEND_STRL("Content-Length"), 1))) { + zend_hash_update(&(*message)->hdrs, "X-Original-Content-Length", sizeof("X-Original-Content-Length"), (void *) &h, sizeof(zval *), (void *) &h_cl); } - if ((h = php_http_message_header(*message, ZEND_STRL("Content-Length"), 1))) { - zend_hash_update(&(*message)->hdrs, "X-Original-Content-Length", sizeof("X-Original-Content-Length"), &h, sizeof(zval *), (void *) &h_cl); - } + if ((h = php_http_message_header(*message, ZEND_STRL("Content-Range"), 1))) { - zend_hash_update(&(*message)->hdrs, "X-Original-Content-Range", sizeof("X-Original-Content-Range"), &h, sizeof(zval *), (void *) &h_cr); - zend_hash_del(&(*message)->hdrs, "Content-Range", sizeof("Content-Range")); + zend_hash_find(&(*message)->hdrs, ZEND_STRS("Content-Range"), (void *) &h_cr); + if (h != *h_cr) { + zend_hash_update(&(*message)->hdrs, "Content-Range", sizeof("Content-Range"), &h, sizeof(zval *), (void *) &h_cr); + } else { + zval_ptr_dtor(&h); + } } - /* default */ - MAKE_STD_ZVAL(h); - ZVAL_LONG(h, 0); - zend_hash_update(&(*message)->hdrs, "Content-Length", sizeof("Content-Length"), &h, sizeof(zval *), NULL); - /* so, if curl sees a 3xx code, a Location header and a Connection:close header * it decides not to read the response body. */ @@ -377,8 +388,7 @@ php_http_message_parser_state_t php_http_message_parser_parse(php_http_message_p case PHP_HTTP_MESSAGE_PARSER_STATE_BODY: { if (len) { - zval *zcl; - + /* FIXME: what if we re-use the parser? */ if (parser->inflate) { char *dec_str = NULL; size_t dec_len; @@ -396,10 +406,6 @@ php_http_message_parser_state_t php_http_message_parser_parse(php_http_message_p php_stream_write(php_http_message_body_stream((*message)->body), str, len); - /* keep track */ - MAKE_STD_ZVAL(zcl); - ZVAL_LONG(zcl, php_http_message_body_size((*message)->body)); - zend_hash_update(&(*message)->hdrs, "Content-Length", sizeof("Content-Length"), &zcl, sizeof(zval *), NULL); } if (cut) { @@ -472,7 +478,7 @@ php_http_message_parser_state_t php_http_message_parser_parse(php_http_message_p { php_http_message_parser_state_push(parser, 1, PHP_HTTP_MESSAGE_PARSER_STATE_DONE); - if (parser->dechunk) { + if (parser->dechunk && parser->dechunk->ctx) { char *dec_str = NULL; size_t dec_len; @@ -485,14 +491,24 @@ php_http_message_parser_state_t php_http_message_parser_parse(php_http_message_p str = dec_str; len = dec_len; cut = 0; - php_http_message_parser_state_push(parser, 1, PHP_HTTP_MESSAGE_PARSER_STATE_BODY); + php_http_message_parser_state_push(parser, 2, PHP_HTTP_MESSAGE_PARSER_STATE_UPDATE_CL, PHP_HTTP_MESSAGE_PARSER_STATE_BODY); } } break; } - case PHP_HTTP_MESSAGE_PARSER_STATE_DONE: { + case PHP_HTTP_MESSAGE_PARSER_STATE_UPDATE_CL: + { + zval *zcl; + MAKE_STD_ZVAL(zcl); + ZVAL_LONG(zcl, php_http_message_body_size((*message)->body)); + zend_hash_update(&(*message)->hdrs, "Content-Length", sizeof("Content-Length"), &zcl, sizeof(zval *), NULL); + break; + } + + case PHP_HTTP_MESSAGE_PARSER_STATE_DONE: + { char *ptr = buffer->data; while (ptr - buffer->data < buffer->used && PHP_HTTP_IS_CTYPE(space, *ptr)) { diff --git a/php_http_message_parser.h b/php_http_message_parser.h index c2bee15..0bac9da 100644 --- a/php_http_message_parser.h +++ b/php_http_message_parser.h @@ -27,6 +27,7 @@ typedef enum php_http_message_parser_state { PHP_HTTP_MESSAGE_PARSER_STATE_BODY_LENGTH, PHP_HTTP_MESSAGE_PARSER_STATE_BODY_CHUNKED, PHP_HTTP_MESSAGE_PARSER_STATE_BODY_DONE, + PHP_HTTP_MESSAGE_PARSER_STATE_UPDATE_CL, PHP_HTTP_MESSAGE_PARSER_STATE_DONE } php_http_message_parser_state_t; diff --git a/tests/client019.phpt b/tests/client019.phpt index 66d99be..98781e0 100644 --- a/tests/client019.phpt +++ b/tests/client019.phpt @@ -43,5 +43,4 @@ Host: www.example.com:80 User-Agent: PECL_HTTP/%s PHP/%s libcurl/%s Proxy-Connection: Keep-Alive Hello: there! -Content-Length: 0 ===DONE=== diff --git a/tests/client020.phpt b/tests/client020.phpt index 29a6ed2..d8282b4 100644 --- a/tests/client020.phpt +++ b/tests/client020.phpt @@ -38,5 +38,4 @@ GET / HTTP/1.1 User-Agent: PECL_HTTP/%s PHP/%s libcurl/%s Host: localhost:%d Accept: */* -Content-Length: 0 ===DONE=== diff --git a/tests/client025.phpt b/tests/client025.phpt index 7b7a605..18c5095 100644 --- a/tests/client025.phpt +++ b/tests/client025.phpt @@ -30,12 +30,12 @@ X-Original-Transfer-Encoding: chunked Content-Length: %d PUT / HTTP/1.1 +Content-Range: bytes 1-2/3 User-Agent: %s Host: localhost:%d Accept: */* -Content-Length: 2 +Content-Length: 3 Expect: 100-continue X-Original-Content-Length: 3 -X-Original-Content-Range: bytes 1-2/3 23===DONE=== diff --git a/tests/data/message_r_content_range.txt b/tests/data/message_r_content_range.txt new file mode 100644 index 0000000..d42dbff --- /dev/null +++ b/tests/data/message_r_content_range.txt @@ -0,0 +1,9 @@ +PUT / HTTP/1.1 +User-Agent: PECL_HTTP/2.3.0dev PHP/5.6.6-dev libcurl/7.41.0-DEV +Host: localhost:8000 +Accept: */* +Expect: 100-continue +Content-Length: 3 +Content-Range: bytes 1-2/3 + +23 \ No newline at end of file diff --git a/tests/message001.phpt b/tests/message001.phpt index 230fd6b..0214dfa 100644 --- a/tests/message001.phpt +++ b/tests/message001.phpt @@ -184,7 +184,7 @@ array(10) { ["Accept-Ranges"]=> string(5) "bytes" ["Content-Length"]=> - int(0) + string(1) "0" ["Vary"]=> string(15) "Accept-Encoding" ["Connection"]=> @@ -197,7 +197,6 @@ array(10) { GET /default/empty.txt HTTP/1.1 Host: localhost Connection: close -Content-Length: 0 HTTP/1.1 200 OK Date: Thu, 26 Aug 2010 09:55:09 GMT Server: Apache/2.2.16 (Unix) mod_ssl/2.2.16 OpenSSL/1.0.0a mod_fastcgi/2.4.6 @@ -205,7 +204,6 @@ Last-Modified: Wed, 28 Apr 2010 10:54:37 GMT Etag: "2002a-0-48549d615a35c" Accept-Ranges: bytes Vary: Accept-Encoding -Content-Length: 0 Connection: close Content-Type: text/plain X-Original-Content-Length: 20 @@ -214,7 +212,7 @@ string(3) "1.1" bool(true) int(200) string(2) "OK" -array(11) { +array(10) { ["Date"]=> string(29) "Thu, 26 Aug 2010 09:55:09 GMT" ["Server"]=> @@ -227,8 +225,6 @@ array(11) { string(5) "bytes" ["Vary"]=> string(15) "Accept-Encoding" - ["Content-Length"]=> - int(0) ["Connection"]=> string(5) "close" ["Content-Type"]=> @@ -242,7 +238,6 @@ GET /default/empty.txt HTTP/1.1 Host: localhost Accept-Encoding: gzip Connection: close -Content-Length: 0 HTTP/1.1 200 OK Date: Thu, 26 Aug 2010 11:41:02 GMT Server: Apache/2.2.16 (Unix) mod_ssl/2.2.16 OpenSSL/1.0.0a mod_fastcgi/2.4.6 @@ -277,7 +272,6 @@ array(8) { GET /default/empty.php HTTP/1.1 Connection: close Host: localhost -Content-Length: 0 HTTP/1.1 200 OK Date: Thu, 26 Aug 2010 12:51:28 GMT Server: Apache/2.2.16 (Unix) mod_ssl/2.2.16 OpenSSL/1.0.0a mod_fastcgi/2.4.6 @@ -311,7 +305,6 @@ array(7) { GET /cgi-bin/chunked.sh HTTP/1.1 Host: localhost Connection: close -Content-Length: 0 --- HTTP/1.1 200 OK Date: Wed, 25 Aug 2010 12:11:44 GMT @@ -340,7 +333,7 @@ array(10) { ["Accept-Ranges"]=> string(5) "bytes" ["Content-Length"]=> - int(0) + string(1) "0" ["Vary"]=> string(15) "Accept-Encoding" ["Connection"]=> @@ -353,7 +346,6 @@ array(10) { GET /default/empty.txt HTTP/1.1 Host: localhost Connection: close -Content-Length: 0 HTTP/1.1 200 OK Date: Thu, 26 Aug 2010 09:55:09 GMT Server: Apache/2.2.16 (Unix) mod_ssl/2.2.16 OpenSSL/1.0.0a mod_fastcgi/2.4.6 @@ -361,7 +353,6 @@ Last-Modified: Wed, 28 Apr 2010 10:54:37 GMT Etag: "2002a-0-48549d615a35c" Accept-Ranges: bytes Vary: Accept-Encoding -Content-Length: 0 Connection: close Content-Type: text/plain X-Original-Content-Length: 20 @@ -370,7 +361,7 @@ string(3) "1.1" bool(true) int(200) string(2) "OK" -array(11) { +array(10) { ["Date"]=> string(29) "Thu, 26 Aug 2010 09:55:09 GMT" ["Server"]=> @@ -383,8 +374,6 @@ array(11) { string(5) "bytes" ["Vary"]=> string(15) "Accept-Encoding" - ["Content-Length"]=> - int(0) ["Connection"]=> string(5) "close" ["Content-Type"]=> @@ -398,7 +387,6 @@ GET /default/empty.txt HTTP/1.1 Host: localhost Accept-Encoding: gzip Connection: close -Content-Length: 0 HTTP/1.1 200 OK Date: Thu, 26 Aug 2010 11:41:02 GMT Server: Apache/2.2.16 (Unix) mod_ssl/2.2.16 OpenSSL/1.0.0a mod_fastcgi/2.4.6 @@ -433,7 +421,6 @@ array(8) { GET /default/empty.php HTTP/1.1 Connection: close Host: localhost -Content-Length: 0 HTTP/1.1 200 OK Date: Thu, 26 Aug 2010 12:51:28 GMT Server: Apache/2.2.16 (Unix) mod_ssl/2.2.16 OpenSSL/1.0.0a mod_fastcgi/2.4.6 @@ -467,5 +454,4 @@ array(7) { GET /cgi-bin/chunked.sh HTTP/1.1 Host: localhost Connection: close -Content-Length: 0 Done diff --git a/tests/message016.phpt b/tests/message016.phpt new file mode 100644 index 0000000..c29d8d6 --- /dev/null +++ b/tests/message016.phpt @@ -0,0 +1,24 @@ +--TEST-- +message content range +--SKIPIF-- + +--FILE-- + +===DONE=== +--EXPECT-- +Test +PUT / HTTP/1.1 +User-Agent: PECL_HTTP/2.3.0dev PHP/5.6.6-dev libcurl/7.41.0-DEV +Host: localhost:8000 +Accept: */* +Expect: 100-continue +Content-Length: 3 +Content-Range: bytes 1-2/3 +X-Original-Content-Length: 3 + +23===DONE=== diff --git a/tests/messageparser001.phpt b/tests/messageparser001.phpt index f4ec2d9..d2d22a5 100644 --- a/tests/messageparser001.phpt +++ b/tests/messageparser001.phpt @@ -12,6 +12,7 @@ echo "Test\n"; use http\Message\Parser; foreach (glob(__DIR__."/data/message_*.txt") as $file) { + $string = ""; $parser = new Parser; $fd = fopen($file, "r") or die("Could not open $file"); while (!feof($fd)) { @@ -23,6 +24,11 @@ foreach (glob(__DIR__."/data/message_*.txt") as $file) { throw new Exception(($e = error_get_last()) ? $e["message"] : "Could not parse $file"); } } + + if (!$string) { + $s = ["START", "HEADER", "HEADER_DONE", "BODY", "BODY_DUMB", "BODY_LENGTH", "BODY_CHUNK", "BODY_DONE", "UPDATE_CL", "DONE"]; + printf("Unexpected state: %s (%s)\n", $s[$parser->getState()], $file); + } $parser = new Parser; rewind($fd); @@ -38,12 +44,14 @@ foreach (glob(__DIR__."/data/message_*.txt") as $file) { if ($string !== (string) $message) { $a = explode("\n", $string); $b = explode("\n", (string) $message); - while ((null !== ($aa = array_shift($a))) || (null !== ($bb = array_shift($b)))) { + do { + $aa = array_shift($a); + $bb = array_shift($b); if ($aa !== $bb) { isset($aa) and printf("-- %s\n", $aa); isset($bb) and printf("++ %s\n", $bb); } - } + } while ($a || $b); } } ?> diff --git a/tests/messageparser002.phpt b/tests/messageparser002.phpt index 2030e93..71f2b5e 100644 --- a/tests/messageparser002.phpt +++ b/tests/messageparser002.phpt @@ -54,7 +54,7 @@ object(http\Message)#%d (9) { ["Host"]=> string(9) "localhost" ["Content-Length"]=> - int(3) + string(1) "3" ["X-Original-Content-Length"]=> string(1) "3" } -- 2.30.2