Merge remote-tracking branch 'origin/master'

twose · twose · commit 674449467b3b · 2019-12-24T16:36:01.000+08:00
diff --git a/swoole_http_client_coro.cc b/swoole_http_client_coro.cc
@@ -1440,10 +1440,10 @@ bool http_client::recv_http_response(double timeout)
         swTraceLog(SW_TRACE_HTTP_CLIENT, "parsed_n=%ld, retval=%ld, total_bytes=%ld, completed=%d", parsed_n, retval, total_bytes, parser.state == s_start_res);
         if (parser.state == s_start_res)
         {
-            // websocket stick package
-            if (parser.upgrade && (size_t) retval > parsed_n + 1 + SW_WEBSOCKET_HEADER_LEN)
+            // handle redundant data (websocket packet)
+            if (parser.upgrade && (size_t) retval > parsed_n + SW_WEBSOCKET_HEADER_LEN)
             {
-                swString_sub(buffer, parsed_n + 1, retval - parsed_n - 1);
+                swString_sub(buffer, parsed_n, retval - parsed_n);
             }
             return true;
         }
diff --git a/swoole_http_request.cc b/swoole_http_request.cc
@@ -902,7 +902,7 @@ static int http_request_message_complete(swoole_http_parser *parser)
     }
     ctx->completed = 1;
 
-    return 0;
+    return 1; /* return from execute */
 }
 
 #ifdef SW_HAVE_COMPRESSION
diff --git a/swoole_http_server.cc b/swoole_http_server.cc
@@ -88,7 +88,7 @@ int php_swoole_http_onReceive(swServer *serv, swEventData *req)
     swoole_http_parser_init(parser, PHP_HTTP_REQUEST);
 
     size_t parsed_n = swoole_http_requset_parse(ctx, Z_STRVAL_P(zdata), Z_STRLEN_P(zdata));
-    if (parsed_n < Z_STRLEN_P(zdata))
+    if (ctx->parser.state == s_dead)
     {
 #ifdef SW_HTTP_BAD_REQUEST_PACKET
         ctx->send(ctx, SW_STRL(SW_HTTP_BAD_REQUEST_PACKET));
diff --git a/swoole_http_server_coro.cc b/swoole_http_server_coro.cc
@@ -503,43 +503,61 @@ static PHP_METHOD(swoole_http_server_coro, onAccept)
 
     while (true)
     {
-        hs->receivers.push_front(sock);
-        auto receiver = hs->receivers.begin();
-        ssize_t retval = sock->recv(buffer->str + total_bytes + buffer->offset, buffer->size - total_bytes - buffer->offset);
-        hs->receivers.erase(receiver);
-
-        if (sw_unlikely(retval <= 0))
+        ssize_t retval;
+        if (ctx != nullptr || total_bytes == 0)
         {
-            break;
-        }
-        total_bytes += retval;
+            hs->receivers.push_front(sock);
+            auto receiver = hs->receivers.begin();
+            retval = sock->recv(buffer->str + total_bytes, buffer->size - total_bytes);
+            hs->receivers.erase(receiver);
 
-        if (!ctx)
-        {
-            ctx = hs->create_context(sock, zconn);
-        }
+            if (sw_unlikely(retval <= 0))
+            {
+                break;
+            }
 
-        if (total_bytes > sock->protocol.package_max_length)
+            if (!ctx)
+            {
+                ctx = hs->create_context(sock, zconn);
+            }
+
+            if (total_bytes + retval > sock->protocol.package_max_length)
+            {
+                ctx->response.status = SW_HTTP_REQUEST_ENTITY_TOO_LARGE;
+                break;
+            }
+        }
+        else
         {
-            ctx->response.status = 413;
-            _error:
-            zval_dtor(ctx->request.zobject);
-            zval_dtor(ctx->response.zobject);
-            break;
+            /* redundant data from previous packet */
+            retval = total_bytes;
+            total_bytes = 0;
+
+            if (!ctx)
+            {
+                ctx = hs->create_context(sock, zconn);
+            }
         }
 
-        size_t parsed_n = swoole_http_requset_parse(ctx, buffer->str + total_bytes - retval, retval);
+        size_t parsed_n = swoole_http_requset_parse(ctx, buffer->str + total_bytes, retval);
+        size_t total_parsed_n = total_bytes + parsed_n;
+        total_bytes += retval;
 
         swTraceLog(SW_TRACE_CO_HTTP_SERVER, "parsed_n=%ld, retval=%ld, total_bytes=%ld, completed=%d", parsed_n, retval, total_bytes, ctx->completed);
 
         if (!ctx->completed)
         {
+            if (ctx->parser.state == s_dead)
+            {
+                ctx->response.status = SW_HTTP_BAD_REQUEST;
+                break;
+            }
             if (total_bytes == buffer->size)
             {
                 if (swString_extend(buffer, buffer->size * 2) != SW_OK)
                 {
-                    ctx->response.status = 503;
-                    goto _error;
+                    ctx->response.status = SW_HTTP_SERVICE_UNAVAILABLE;
+                    break;
                 }
             }
             continue;
@@ -549,26 +567,31 @@ static PHP_METHOD(swoole_http_server_coro, onAccept)
         if (ctx->parser.method == PHP_HTTP_NOT_IMPLEMENTED
                 && memcmp(buffer->str, SW_HTTP2_PRI_STRING, sizeof(SW_HTTP2_PRI_STRING) - 1) == 0)
         {
-            buffer->length = retval - (sizeof(SW_HTTP2_PRI_STRING) - 1);
-            buffer->offset = buffer->length == 0 ? 0 : parsed_n;
+            buffer->length = total_bytes - (sizeof(SW_HTTP2_PRI_STRING) - 1);
+            buffer->offset = buffer->length == 0 ? 0 : (sizeof(SW_HTTP2_PRI_STRING) - 1);
             hs->recv_http2_frame(ctx);
             break;
         }
 #endif
 
-        if (retval > (ssize_t) parsed_n)
+        ZVAL_STRINGL(&ctx->request.zdata, buffer->str, total_parsed_n);
+
+        /* handle more packages */
+        if ((size_t) retval > parsed_n)
         {
-            buffer->offset = retval - parsed_n;
-            memmove(buffer->str, buffer->str + total_bytes + parsed_n, buffer->offset);
+            total_bytes = retval - parsed_n;
+            memmove(buffer->str, buffer->str + total_parsed_n, total_bytes);
+            if (ctx->websocket)
+            {
+                /* for recv_packet */
+                buffer->length = total_bytes;
+            }
         }
         else
         {
-            buffer->offset = 0;
+            total_bytes = 0;
         }
 
-        ZVAL_STRINGL(&ctx->request.zdata, buffer->str, total_bytes);
-        total_bytes = 0;
-
         zval *zserver = ctx->request.zserver;
         add_assoc_long(zserver, "server_port", hs->socket->get_bind_port());
         add_assoc_long(zserver, "remote_port", (zend_long) hs->socket->get_port());
@@ -595,18 +618,19 @@ static PHP_METHOD(swoole_http_server_coro, onAccept)
 
         zval_dtor(&args[0]);
         zval_dtor(&args[1]);
+        ctx = nullptr;
 
-        if (hs->running && keep_alive)
-        {
-            swTraceLog(SW_TRACE_CO_HTTP_SERVER, "http_server_coro keepalive");
-            ctx = nullptr;
-            continue;
-        }
-        else
+        if (!hs->running || !keep_alive)
         {
             break;
         }
     }
+
+    if (ctx)
+    {
+        zval_dtor(ctx->request.zobject);
+        zval_dtor(ctx->response.zobject);
+    }
 }
 
 static PHP_METHOD(swoole_http_server_coro, shutdown)
diff --git a/tests/swoole_http_server_coro/bad_request.phpt b/tests/swoole_http_server_coro/bad_request.phpt
@@ -0,0 +1,40 @@
+--TEST--
+swoole_http_server_coro: bad request
+--SKIPIF--
+<?php require __DIR__ . '/../include/skipif.inc'; ?>
+--FILE--
+<?php
+require __DIR__ . '/../include/bootstrap.php';
+
+use Swoole\Coroutine;
+use Swoole\Coroutine\Http\Server;
+use Swoole\Coroutine\Socket;
+use Swoole\Http\Request;
+use Swoole\Http\Response;
+
+const REQUEST = "GET / HTTP/1.1\r\nContent-Length: invalid\r\n\r\n";
+
+Coroutine\run(function () {
+    $server = new Server('127.0.0.1', 0);
+    Coroutine::create(function () use ($server) {
+        $server->handle('/', function (Request $request, Response $response) use ($server) {
+            echo "never here\n";
+        });
+        $server->start();
+    });
+    Coroutine::sleep(0.001);
+    Coroutine::create(function () use ($server) {
+        $socket = new Socket(AF_INET, SOCK_STREAM, IPPROTO_IP);
+        if (Assert::true($socket->connect('127.0.0.1', $server->port, -1))) {
+            $ret = $socket->sendAll(REQUEST);
+            Assert::same($ret, strlen(REQUEST));
+            Assert::contains($socket->recv(), 'HTTP/1.1 400 Bad Request');
+            $server->shutdown();
+            echo "DONE\n";
+        }
+    });
+});
+
+?>
+--EXPECT--
+DONE
diff --git a/tests/swoole_http_server_coro/pipeline.phpt b/tests/swoole_http_server_coro/pipeline.phpt
@@ -0,0 +1,45 @@
+--TEST--
+swoole_http_server_coro: pipeline
+--SKIPIF--
+<?php require __DIR__ . '/../include/skipif.inc'; ?>
+--FILE--
+<?php
+require __DIR__ . '/../include/bootstrap.php';
+
+use Swoole\Coroutine;
+use Swoole\Coroutine\Http\Server;
+use Swoole\Coroutine\Socket;
+use Swoole\Http\Request;
+use Swoole\Http\Response;
+
+const REQUEST = "GET / HTTP/1.1\r\n\r\n";
+
+Coroutine\run(function () {
+    $server = new Server('127.0.0.1', 0);
+    Coroutine::create(function () use ($server) {
+        $server->handle('/', function (Request $request, Response $response) use ($server) {
+            static $count = 0;
+            if (++$count === MAX_CONCURRENCY * MAX_REQUESTS) {
+                echo "DONE\n";
+                $server->shutdown();
+            }
+        });
+        $server->start();
+    });
+    for ($c = MAX_CONCURRENCY; $c--;) {
+        Coroutine::sleep(0.001);
+        Coroutine::create(function () use ($server) {
+            $socket = new Socket(AF_INET, SOCK_STREAM, IPPROTO_IP);
+            if (Assert::true($socket->connect('127.0.0.1', $server->port, -1))) {
+                $ret = $socket->sendAll(str_repeat(REQUEST, MAX_REQUESTS));
+                Assert::same($ret, strlen(REQUEST) * MAX_REQUESTS);
+            } else {
+                throw new RuntimeException('Connect failed: ' . $socket->errMsg);
+            }
+        });
+    }
+});
+
+?>
+--EXPECT--
+DONE
diff --git a/tests/swoole_http_server_coro/websocket_mixed.phpt b/tests/swoole_http_server_coro/websocket_mixed.phpt
@@ -0,0 +1,51 @@
+--TEST--
+swoole_http_server_coro: bad request
+--SKIPIF--
+<?php require __DIR__ . '/../include/skipif.inc'; ?>
+--FILE--
+<?php
+require __DIR__ . '/../include/bootstrap.php';
+
+use Swoole\Coroutine;
+use Swoole\Coroutine\Http\Server;
+use Swoole\Coroutine\Socket;
+use Swoole\Http\Request;
+use Swoole\Http\Response;
+
+const REQUEST =
+    "GET /websocket HTTP/1.1\r\n" .
+    "Host: 127.0.0.1\r\n" .
+    "Connection: Upgrade\r\n" .
+    "Upgrade: websocket\r\n" .
+    "Sec-WebSocket-Version: 13\r\n" .
+    "Sec-WebSocket-Key: ZE5FYi8lZlZBbnlrTmxYKQ==\r\n\r\n" .
+    "\x81\x0cHello Swoole";
+
+Coroutine\run(function () {
+    $server = new Server('127.0.0.1', 0);
+    Coroutine::create(function () use ($server) {
+        $server->handle('/', function (Request $request, Response $websocket) use ($server) {
+            $websocket->upgrade();
+            $frame = $websocket->recv();
+            if ($frame) {
+                $websocket->push($frame);
+            }
+        });
+        $server->start();
+    });
+    Coroutine::sleep(0.001);
+    Coroutine::create(function () use ($server) {
+        $socket = new Socket(AF_INET, SOCK_STREAM, IPPROTO_IP);
+        if (Assert::true($socket->connect('127.0.0.1', $server->port, -1))) {
+            $ret = $socket->sendAll(REQUEST);
+            Assert::same($ret, strlen(REQUEST));
+            Assert::contains($socket->recv(), 'Hello Swoole');
+            $server->shutdown();
+            echo "DONE\n";
+        }
+    });
+});
+
+?>
+--EXPECT--
+DONE
diff --git a/thirdparty/swoole_http_parser.c b/thirdparty/swoole_http_parser.c

Original file line number	Diff line number	Diff line change
`@@ -1440,10 +1440,10 @@ bool http_client::recv_http_response(double timeout)`
`1440`	`1440`	`swTraceLog(SW_TRACE_HTTP_CLIENT, "parsed_n=%ld, retval=%ld, total_bytes=%ld, completed=%d", parsed_n, retval, total_bytes, parser.state == s_start_res);`
`1441`	`1441`	`if (parser.state == s_start_res)`
`1442`	`1442`	`{`
`1443`		`- // websocket stick package`
`1444`		`- if (parser.upgrade && (size_t) retval > parsed_n + 1 + SW_WEBSOCKET_HEADER_LEN)`
	`1443`	`+ // handle redundant data (websocket packet)`
	`1444`	`+ if (parser.upgrade && (size_t) retval > parsed_n + SW_WEBSOCKET_HEADER_LEN)`
`1445`	`1445`	`{`
`1446`		`- swString_sub(buffer, parsed_n + 1, retval - parsed_n - 1);`
	`1446`	`+ swString_sub(buffer, parsed_n, retval - parsed_n);`
`1447`	`1447`	`}`
`1448`	`1448`	`return true;`
`1449`	`1449`	`}`
Original file line number	Diff line number	Diff line change
`@@ -902,7 +902,7 @@ static int http_request_message_complete(swoole_http_parser *parser)`
`902`	`902`	`}`
`903`	`903`	`ctx->completed = 1;`
`904`	`904`
`905`		`- return 0;`
	`905`	`+ return 1; /* return from execute */`
`906`	`906`	`}`
`907`	`907`
`908`	`908`	`#ifdef SW_HAVE_COMPRESSION`
Original file line number	Diff line number	Diff line change
`@@ -88,7 +88,7 @@ int php_swoole_http_onReceive(swServer serv, swEventData req)`
`88`	`88`	`swoole_http_parser_init(parser, PHP_HTTP_REQUEST);`
`89`	`89`
`90`	`90`	`size_t parsed_n = swoole_http_requset_parse(ctx, Z_STRVAL_P(zdata), Z_STRLEN_P(zdata));`
`91`		`- if (parsed_n < Z_STRLEN_P(zdata))`
	`91`	`+ if (ctx->parser.state == s_dead)`
`92`	`92`	`{`
`93`	`93`	`#ifdef SW_HTTP_BAD_REQUEST_PACKET`
`94`	`94`	`ctx->send(ctx, SW_STRL(SW_HTTP_BAD_REQUEST_PACKET));`