Optimize ssl context code, fix tests, fix memory leak

matyhtf · matyhtf · commit 050293e7ebaa · 2024-11-05T14:16:40.000+08:00
diff --git a/ext-src/stubs/php_swoole_http_server_coro.stub.php b/ext-src/stubs/php_swoole_http_server_coro.stub.php
@@ -4,7 +4,7 @@ final class Server {
 		public function __construct(string $host, int $port = 0, bool $ssl = false, bool $reuse_port = false) {}
 		public function __destruct() {}
 		public function set(array $settings): bool {}
-		public function handle(string $pattern, callable $callback): void {}
+		public function handle(string $pattern, callable $callback): bool {}
 		public function start(): bool {}
 		public function shutdown(): void {}
 		private function onAccept(\Swoole\Coroutine\Socket $conn): void {}
diff --git a/ext-src/stubs/php_swoole_http_server_coro_arginfo.h b/ext-src/stubs/php_swoole_http_server_coro_arginfo.h
@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 2f5ecf154780c21ccc66ba5e2fd318eb117191b0 */
+ * Stub hash: 6f65975475013861bdaec52a0fb3fe3b1dc75657 */
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_Swoole_Coroutine_Http_Server___construct, 0, 0, 1)
 	ZEND_ARG_TYPE_INFO(0, host, IS_STRING, 0)
@@ -15,7 +15,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_Swoole_Coroutine_Http_Serv
 	ZEND_ARG_TYPE_INFO(0, settings, IS_ARRAY, 0)
 ZEND_END_ARG_INFO()
 
-ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_Swoole_Coroutine_Http_Server_handle, 0, 2, IS_VOID, 0)
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_Swoole_Coroutine_Http_Server_handle, 0, 2, _IS_BOOL, 0)
 	ZEND_ARG_TYPE_INFO(0, pattern, IS_STRING, 0)
 	ZEND_ARG_TYPE_INFO(0, callback, IS_CALLABLE, 0)
 ZEND_END_ARG_INFO()
diff --git a/ext-src/swoole_http_server_coro.cc b/ext-src/swoole_http_server_coro.cc
@@ -105,11 +105,19 @@ class HttpServer {
         delete socket;
     }
 
-    void set_handler(std::string pattern, zend::Callable *cb) {
+    bool set_handler(std::string pattern, zval *zfn) {
+        auto cb = sw_callable_create(zfn);
+        if (!cb) {
+            return false;
+        }
+        if (handlers.find(pattern) != handlers.end()) {
+            sw_callable_free(handlers[pattern]);
+        }
         handlers[pattern] = cb;
         if (pattern == "/") {
             default_handler = cb;
         }
+        return true;
     }
 
     zend::Callable *get_handler(HttpContext *ctx) {
@@ -388,13 +396,8 @@ static PHP_METHOD(swoole_http_server_coro, handle) {
     Z_PARAM_ZVAL(zfn)
     ZEND_PARSE_PARAMETERS_END();
 
-    auto cb = sw_callable_create(zfn);
-    if (!cb) {
-        RETURN_FALSE;
-    }
-
     std::string key(pattern, pattern_len);
-    hs->set_handler(key, cb);
+    RETURN_BOOL(hs->set_handler(key, zfn));
 }
 
 static PHP_METHOD(swoole_http_server_coro, set) {
@@ -421,10 +424,9 @@ static PHP_METHOD(swoole_http_server_coro, start) {
     /* get callback fci cache */
     char *func_name = nullptr;
     zend_fcall_info_cache fci_cache;
-    zval zcallback;
-    ZVAL_STRING(&zcallback, "onAccept");
+    zend::Variable zcallback("onAccept");
     if (!sw_zend_is_callable_at_frame(
-            &zcallback, ZEND_THIS, execute_data, 0, &func_name, nullptr, &fci_cache, nullptr)) {
+            zcallback.ptr(), ZEND_THIS, execute_data, 0, &func_name, nullptr, &fci_cache, nullptr)) {
         php_swoole_fatal_error(E_CORE_ERROR, "function '%s' is not callable", func_name);
         return;
     }
@@ -510,7 +512,7 @@ static PHP_METHOD(swoole_http_server_coro, start) {
         if (conn) {
             zval zsocket;
             php_swoole_init_socket_object(&zsocket, conn);
-            long cid = PHPCoroutine::create(&fci_cache, 1, &zsocket, &zcallback);
+            long cid = PHPCoroutine::create(&fci_cache, 1, &zsocket, zcallback.ptr());
             zval_dtor(&zsocket);
             if (cid < 0) {
                 goto _wait_1s;
@@ -535,8 +537,6 @@ static PHP_METHOD(swoole_http_server_coro, start) {
         }
     }
 
-    zval_dtor(&zcallback);
-
     RETURN_TRUE;
 }
 
diff --git a/ext-src/swoole_runtime.cc b/ext-src/swoole_runtime.cc
@@ -786,10 +786,13 @@ static int socket_enable_crypto(php_stream *stream, Socket *sock, php_stream_xpo
         return -1;
     }
 
-    zval *val = php_stream_context_get_option(context, "ssl", "capture_peer_cert");
-    if (val && zend_is_true(val) && !php_openssl_capture_peer_certs(stream, sock)) {
-        return -1;
+    if (context && sock->ssl_is_available()) {
+        zval *val = php_stream_context_get_option(context, "ssl", "capture_peer_cert");
+        if (val && zend_is_true(val) && !php_openssl_capture_peer_certs(stream, sock)) {
+            return -1;
+        }
     }
+
     return 1;
 }
 #endif
diff --git a/include/swoole_coroutine_socket.h b/include/swoole_coroutine_socket.h
@@ -129,8 +129,7 @@ class Socket {
      * Operation sequence:
      * 1. enable_ssl_encrypt()
      * 2. Set SSL parameters, such as certificate file, key file
-     * 3. ssl_check_context()
-     * 4. ssl_accept()/ssl_connect()/ssl_handshake()
+     * 3. ssl_handshake(), to be executed after connect or accept
      */
     bool enable_ssl_encrypt() {
         if (ssl_context.get()) {
@@ -442,7 +441,6 @@ class Socket {
     std::string ssl_host_name;
     bool ssl_context_create();
     bool ssl_create(SSLContext *ssl_context);
-    bool ssl_listen();
 #endif
 
     bool connected = false;
diff --git a/src/core/base.cc b/src/core/base.cc
@@ -255,6 +255,12 @@ void swoole_clean(void) {
         delete SwooleTG.buffer_stack;
         SwooleTG.buffer_stack = nullptr;
     }
+    SW_LOOP_N(SW_MAX_HOOK_TYPE) {
+        if (SwooleG.hooks[i]) {
+            auto hooks = static_cast<std::list<swoole::Callback> *>(SwooleG.hooks[i]);
+            delete hooks;
+        }
+    }
     swoole_signal_clear();
     SwooleG = {};
 }
diff --git a/src/coroutine/socket.cc b/src/coroutine/socket.cc
@@ -1137,10 +1137,7 @@ bool Socket::listen(int backlog) {
         return false;
     }
 #ifdef SW_USE_OPENSSL
-    if (ssl_is_enable() && !ssl_listen()) {
-        set_err(SW_ERROR_SSL_CREATE_CONTEXT_FAILED);
-        return false;
-    }
+    ssl_is_server = true;
 #endif
     return true;
 }
@@ -1149,6 +1146,11 @@ Socket *Socket::accept(double timeout) {
     if (sw_unlikely(!is_available(SW_EVENT_READ))) {
         return nullptr;
     }
+#ifdef SW_USE_OPENSSL
+    if (ssl_is_enable() && sw_unlikely(ssl_context->context == nullptr) && !ssl_context_create()) {
+        return nullptr;
+    }
+#endif
     network::Socket *conn = socket->accept();
     if (conn == nullptr && errno == EAGAIN) {
         TimerController timer(&read_timer, timeout == 0 ? read_timeout : timeout, this, timer_callback);
@@ -1213,14 +1215,6 @@ bool Socket::ssl_create(SSLContext *ssl_context) {
     return true;
 }
 
-bool Socket::ssl_listen() {
-    ssl_is_server = true;
-    if (ssl_context->context == nullptr && !ssl_context_create()) {
-        return false;
-    }
-    return true;
-}
-
 bool Socket::ssl_handshake() {
     if (ssl_handshaked) {
         return false;
diff --git a/tests/swoole_server_coro/ssl.phpt b/tests/swoole_server_coro/ssl.phpt
@@ -13,8 +13,7 @@ $pm = new ProcessManager;
 
 $pm->parentFunc = function ($pid) use ($pm) {
     $client = new Swoole\Client(SWOOLE_SOCK_TCP | SWOOLE_SSL, SWOOLE_SOCK_SYNC); //同步阻塞
-    if (!$client->connect('127.0.0.1', $pm->getFreePort()))
-    {
+    if (!$client->connect('127.0.0.1', $pm->getFreePort())) {
         exit("connect failed\n");
     }
     $client->send("hello world");

Original file line number	Diff line number	Diff line change
`@@ -786,10 +786,13 @@ static int socket_enable_crypto(php_stream stream, Socket sock, php_stream_xpo`
`786`	`786`	`return -1;`
`787`	`787`	`}`
`788`	`788`
`789`		`- zval *val = php_stream_context_get_option(context, "ssl", "capture_peer_cert");`
`790`		`- if (val && zend_is_true(val) && !php_openssl_capture_peer_certs(stream, sock)) {`
`791`		`- return -1;`
	`789`	`+ if (context && sock->ssl_is_available()) {`
	`790`	`+ zval *val = php_stream_context_get_option(context, "ssl", "capture_peer_cert");`
	`791`	`+ if (val && zend_is_true(val) && !php_openssl_capture_peer_certs(stream, sock)) {`
	`792`	`+ return -1;`
	`793`	`+ }`
`792`	`794`	`}`
	`795`	`+`
`793`	`796`	`return 1;`
`794`	`797`	`}`
`795`	`798`	`#endif`
Original file line number	Diff line number	Diff line change
`@@ -255,6 +255,12 @@ void swoole_clean(void) {`
`255`	`255`	`delete SwooleTG.buffer_stack;`
`256`	`256`	`SwooleTG.buffer_stack = nullptr;`
`257`	`257`	`}`
	`258`	`+ SW_LOOP_N(SW_MAX_HOOK_TYPE) {`
	`259`	`+ if (SwooleG.hooks[i]) {`
	`260`	`+ auto hooks = static_cast<std::list<swoole::Callback> *>(SwooleG.hooks[i]);`
	`261`	`+ delete hooks;`
	`262`	`+ }`
	`263`	`+ }`
`258`	`264`	`swoole_signal_clear();`
`259`	`265`	`SwooleG = {};`
`260`	`266`	`}`
Original file line number	Diff line number	Diff line change
`@@ -13,8 +13,7 @@ $pm = new ProcessManager;`
`13`	`13`
`14`	`14`	`$pm->parentFunc = function ($pid) use ($pm) {`
`15`	`15`	`$client = new Swoole\Client(SWOOLE_SOCK_TCP \| SWOOLE_SSL, SWOOLE_SOCK_SYNC); //同步阻塞`
`16`		`- if (!$client->connect('127.0.0.1', $pm->getFreePort()))`
`17`		`- {`
	`16`	`+ if (!$client->connect('127.0.0.1', $pm->getFreePort())) {`
`18`	`17`	`exit("connect failed\n");`
`19`	`18`	`}`
`20`	`19`	`$client->send("hello world");`