Skip to content

Commit

Permalink
Optimize the parameter validation logic in Client::enableSSL()
Browse files Browse the repository at this point in the history
  • Loading branch information
matyhtf committed Dec 4, 2024
1 parent 81d0f45 commit 5359616
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 21 deletions.
37 changes: 24 additions & 13 deletions ext-src/swoole_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ static zend_object *client_create_object(zend_class_entry *ce) {
return &client->std;
}


SW_EXTERN_C_BEGIN
static PHP_METHOD(swoole_client, __construct);
static PHP_METHOD(swoole_client, __destruct);
Expand Down Expand Up @@ -166,8 +165,7 @@ void php_swoole_client_minit(int module_number) {
SW_SET_CLASS_NOT_SERIALIZABLE(swoole_client);
SW_SET_CLASS_CLONEABLE(swoole_client, sw_zend_class_clone_deny);
SW_SET_CLASS_UNSET_PROPERTY_HANDLER(swoole_client, sw_zend_class_unset_property_deny);
SW_SET_CLASS_CUSTOM_OBJECT(
swoole_client, client_create_object, client_free_object, ClientObject, std);
SW_SET_CLASS_CUSTOM_OBJECT(swoole_client, client_create_object, client_free_object, ClientObject, std);

SW_INIT_CLASS_ENTRY_EX(swoole_client_exception, "Swoole\\Client\\Exception", nullptr, nullptr, swoole_exception);

Expand Down Expand Up @@ -720,11 +718,11 @@ static PHP_METHOD(swoole_client, connect) {
RETURN_TRUE;
}
php_swoole_core_error(E_WARNING,
"connect to server[%s:%d] failed. Error: %s[%d]",
host,
(int) port,
swoole_strerror(swoole_get_last_error()),
swoole_get_last_error());
"connect to server[%s:%d] failed. Error: %s[%d]",
host,
(int) port,
swoole_strerror(swoole_get_last_error()),
swoole_get_last_error());
php_swoole_client_free(ZEND_THIS, cli);
RETURN_FALSE;
}
Expand Down Expand Up @@ -1234,6 +1232,19 @@ bool php_swoole_client_enable_ssl_encryption(Client *cli, zval *zobject) {
}

static PHP_METHOD(swoole_client, enableSSL) {
zval *zcallback = nullptr;

ZEND_PARSE_PARAMETERS_START(0, 1)
Z_PARAM_OPTIONAL
Z_PARAM_ZVAL(zcallback)
ZEND_PARSE_PARAMETERS_END_EX(RETURN_FALSE);

if (zcallback) {
zend_throw_exception(
swoole_exception_ce, "sync client does not support `onSslReady` callback", SW_ERROR_INVALID_PARAMS);
RETURN_FALSE;
}

Client *cli = php_swoole_client_get_cli_safe(ZEND_THIS);
if (!cli) {
RETURN_FALSE;
Expand Down Expand Up @@ -1296,11 +1307,11 @@ PHP_FUNCTION(swoole_client_select) {
double timeout = SW_CLIENT_CONNECT_TIMEOUT;

ZEND_PARSE_PARAMETERS_START(3, 4)
Z_PARAM_ARRAY_EX2(r_array, 1, 1, 0)
Z_PARAM_ARRAY_EX2(w_array, 1, 1, 0)
Z_PARAM_ARRAY_EX2(e_array, 1, 1, 0)
Z_PARAM_OPTIONAL
Z_PARAM_DOUBLE(timeout)
Z_PARAM_ARRAY_EX2(r_array, 1, 1, 0)
Z_PARAM_ARRAY_EX2(w_array, 1, 1, 0)
Z_PARAM_ARRAY_EX2(e_array, 1, 1, 0)
Z_PARAM_OPTIONAL
Z_PARAM_DOUBLE(timeout)
ZEND_PARSE_PARAMETERS_END();

int maxevents = SW_MAX(SW_MAX(php_swoole_array_length_safe(r_array), php_swoole_array_length_safe(w_array)),
Expand Down
23 changes: 15 additions & 8 deletions ext-src/swoole_client_async.cc
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ static sw_inline void client_execute_callback(zval *zobject, enum php_swoole_cli
}
}


// clang-format off
static const zend_function_entry swoole_client_async_methods[] = {
PHP_ME(swoole_client_async, __construct, arginfo_class_Swoole_Async_Client___construct, ZEND_ACC_PUBLIC)
Expand All @@ -144,7 +143,8 @@ static const zend_function_entry swoole_client_async_methods[] = {
// clang-format on

void php_swoole_client_async_minit(int module_number) {
SW_INIT_CLASS_ENTRY_EX(swoole_client_async, "Swoole\\Async\\Client", nullptr, swoole_client_async_methods, swoole_client);
SW_INIT_CLASS_ENTRY_EX(
swoole_client_async, "Swoole\\Async\\Client", nullptr, swoole_client_async_methods, swoole_client);
SW_SET_CLASS_NOT_SERIALIZABLE(swoole_client_async);
SW_SET_CLASS_CLONEABLE(swoole_client_async, sw_zend_class_clone_deny);
SW_SET_CLASS_UNSET_PROPERTY_HANDLER(swoole_client_async, sw_zend_class_unset_property_deny);
Expand Down Expand Up @@ -504,6 +504,18 @@ static PHP_METHOD(swoole_client_async, wakeup) {

#ifdef SW_USE_OPENSSL
static PHP_METHOD(swoole_client_async, enableSSL) {
zval *zcallback = nullptr;

ZEND_PARSE_PARAMETERS_START(0, 1)
Z_PARAM_OPTIONAL
Z_PARAM_ZVAL(zcallback)
ZEND_PARSE_PARAMETERS_END_EX(RETURN_FALSE);

if (zcallback == nullptr) {
zend_throw_exception(swoole_exception_ce, "require `onSslReady` callback", SW_ERROR_INVALID_PARAMS);
RETURN_FALSE;
}

Client *cli = php_swoole_client_get_cli_safe(ZEND_THIS);
if (!cli) {
RETURN_FALSE;
Expand All @@ -512,11 +524,6 @@ static PHP_METHOD(swoole_client_async, enableSSL) {
RETURN_FALSE;
}

zval *zcallback;
if (zend_parse_parameters(ZEND_NUM_ARGS(), "z", &zcallback) == FAILURE) {
RETURN_FALSE;
}

auto client_obj = php_swoole_client_fetch_object(ZEND_THIS);
if (swoole_event_set(cli->socket, SW_EVENT_WRITE) < 0) {
RETURN_FALSE;
Expand All @@ -528,7 +535,7 @@ static PHP_METHOD(swoole_client_async, enableSSL) {

auto cb = sw_callable_create(zcallback);
if (!cb) {
return;
RETURN_FALSE;
}
zend_update_property(swoole_client_async_ce, Z_OBJ_P(ZEND_THIS), ZEND_STRL("onSSLReady"), zcallback);
client_obj->async->onSSLReady = cb;
Expand Down
16 changes: 16 additions & 0 deletions tests/swoole_client_async/enableSSL_bad_callback.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
--TEST--
swoole_client_async: enableSSL with bad callback
--SKIPIF--
<?php require __DIR__ . '/../include/skipif.inc'; ?>
--FILE--
<?php
require __DIR__ . '/../include/bootstrap.php';

$cli = new Swoole\Async\Client(SWOOLE_SOCK_TCP);
try {
$res = $cli->enableSSL();
} catch (Exception $e) {
Assert::contains($e->getMessage(), 'require `onSslReady` callback');
}
?>
--EXPECTF--
33 changes: 33 additions & 0 deletions tests/swoole_client_sync/enableSSL.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
--TEST--
swoole_client_async: enableSSL
--SKIPIF--
<?php require __DIR__ . '/../include/skipif.inc'; ?>
--FILE--
<?php
require __DIR__ . '/../include/bootstrap.php';

$cli = new Swoole\Client(SWOOLE_SOCK_TCP);
Assert::true($cli->connect("www.baidu.com", 443, 2.0));

if ($cli->enableSSL()) {
echo "SSL READY\n";
$cli->send("GET / HTTP/1.1\r\nHost: www.baidu.com\r\nConnection: close\r\nUser-Agent: curl/7.50.1-DEV\r\nAccept: */*\r\n\r\n");
}

$resp = '';
while (true) {
$data = $cli->recv();
if ($data == false) {
break;
}
$resp .= $data;
}

Assert::assert(strlen($resp) > 0);
Assert::contains($resp, 'www.baidu.com');
$cli->close();
echo "DONE\n";
?>
--EXPECT--
SSL READY
DONE
18 changes: 18 additions & 0 deletions tests/swoole_client_sync/enableSSL_2.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
--TEST--
swoole_client_async: enableSSL
--SKIPIF--
<?php require __DIR__ . '/../include/skipif.inc'; ?>
--FILE--
<?php
require __DIR__ . '/../include/bootstrap.php';

$cli = new Swoole\Client(SWOOLE_SOCK_TCP);
Assert::true($cli->connect("www.baidu.com", 443, 2.0));

try {
$cli->enableSSL(function (){});
} catch (\Throwable $e) {
Assert::contains($e->getMessage(), 'not support `onSslReady` callback');
}
?>
--EXPECT--

0 comments on commit 5359616

Please sign in to comment.