From 0e4eeda2f3696948cb933f8ac94a88564c584ad8 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Thu, 12 Dec 2024 22:11:22 +0000 Subject: [PATCH 1/7] ext/sockets: socket_set_option switch from convert_to_long to zval_try_get_long. to be explicit when the expected type is not met. --- ext/sockets/sockets.c | 53 ++++++++++++++----- .../tests/socket_set_option_timeo_error.phpt | 42 +++++++++++++++ 2 files changed, 81 insertions(+), 14 deletions(-) create mode 100644 ext/sockets/tests/socket_set_option_timeo_error.phpt diff --git a/ext/sockets/sockets.c b/ext/sockets/sockets.c index e1b350b4045a4..cfc92123764f8 100644 --- a/ext/sockets/sockets.c +++ b/ext/sockets/sockets.c @@ -1811,6 +1811,7 @@ PHP_FUNCTION(socket_set_option) HashTable *opt_ht; zval *l_onoff, *l_linger; zval *sec, *usec; + bool failed; if (zend_parse_parameters(ZEND_NUM_ARGS(), "Ollz", &arg1, socket_ce, &level, &optname, &arg4) == FAILURE) { RETURN_THROWS(); @@ -1883,11 +1884,19 @@ PHP_FUNCTION(socket_set_option) RETURN_THROWS(); } - convert_to_long(l_onoff); - convert_to_long(l_linger); + zend_long zl_onoff = zval_try_get_long(l_onoff, &failed); + if (failed) { + zend_argument_type_error(4, "\"%s\" must be an int, %s given", l_onoff_key, zend_zval_value_name(l_onoff)); + RETURN_THROWS(); + } + zend_long zl_linger = zval_try_get_long(l_linger, &failed); + if (failed) { + zend_argument_type_error(4, "\"%s\" must be an int, %s given", l_linger_key, zend_zval_value_name(l_linger)); + RETURN_THROWS(); + } - lv.l_onoff = (unsigned short)Z_LVAL_P(l_onoff); - lv.l_linger = (unsigned short)Z_LVAL_P(l_linger); + lv.l_onoff = (unsigned short)zl_onoff; + lv.l_linger = (unsigned short)zl_linger; optlen = sizeof(lv); opt_ptr = &lv; @@ -1898,6 +1907,7 @@ PHP_FUNCTION(socket_set_option) case SO_SNDTIMEO: { const char sec_key[] = "sec"; const char usec_key[] = "usec"; + bool failed; convert_to_array(arg4); opt_ht = Z_ARRVAL_P(arg4); @@ -1911,15 +1921,23 @@ PHP_FUNCTION(socket_set_option) RETURN_THROWS(); } - convert_to_long(sec); - convert_to_long(usec); + zend_long zsec = zval_try_get_long(sec, &failed); + if (failed) { + zend_argument_type_error(4, "\"%s\" must be an int, %s given", sec_key, zend_zval_value_name(sec)); + RETURN_THROWS(); + } + zend_long zusec = zval_try_get_long(usec, &failed); + if (failed) { + zend_argument_type_error(4, "\"%s\" must be an int, %s given", usec_key, zend_zval_value_name(usec)); + RETURN_THROWS(); + } #ifndef PHP_WIN32 - tv.tv_sec = Z_LVAL_P(sec); - tv.tv_usec = Z_LVAL_P(usec); + tv.tv_sec = zsec; + tv.tv_usec = zusec; optlen = sizeof(tv); opt_ptr = &tv; #else - timeout = Z_LVAL_P(sec) * 1000 + Z_LVAL_P(usec) / 1000; + timeout = zsec * 1000 + zusec / 1000; optlen = sizeof(int); opt_ptr = &timeout; #endif @@ -1971,15 +1989,19 @@ PHP_FUNCTION(socket_set_option) #ifdef SO_ATTACH_REUSEPORT_CBPF case SO_ATTACH_REUSEPORT_CBPF: { - convert_to_long(arg4); + zend_long fval = zval_try_get_long(arg4, &failed); + if (failed) { + zend_argument_type_error(4, "must be an int, %s given", zend_zval_value_name(arg4)); + RETURN_THROWS(); + } - if (!Z_LVAL_P(arg4)) { + if (!fval) { ov = 1; optlen = sizeof(ov); opt_ptr = &ov; optname = SO_DETACH_BPF; } else { - uint32_t k = (uint32_t)Z_LVAL_P(arg4); + uint32_t k = (uint32_t)fval; static struct sock_filter cbpf[8] = {0}; static struct sock_fprog bpfprog; @@ -2006,8 +2028,11 @@ PHP_FUNCTION(socket_set_option) default: default_case: - convert_to_long(arg4); - ov = Z_LVAL_P(arg4); + ov = zval_try_get_long(arg4, &failed); + if (failed) { + zend_argument_type_error(4, "must be an int, %s given", zend_zval_value_name(arg4)); + RETURN_THROWS(); + } optlen = sizeof(ov); opt_ptr = &ov; diff --git a/ext/sockets/tests/socket_set_option_timeo_error.phpt b/ext/sockets/tests/socket_set_option_timeo_error.phpt new file mode 100644 index 0000000000000..373b97ade0c7b --- /dev/null +++ b/ext/sockets/tests/socket_set_option_timeo_error.phpt @@ -0,0 +1,42 @@ +--TEST-- +socket_set_option() with SO_RCVTIMEO/SO_SNDTIMEO/SO_LINGER +--EXTENSIONS-- +sockets +--FILE-- + 1, "usec" => "aaaaa"); +$options_2 = array("sec" => new stdClass(), "usec" => "1"); +$options_3 = array("l_onoff" => "aaaa", "l_linger" => "1"); +$options_4 = array("l_onoff" => "1", "l_linger" => []); + +try { + socket_set_option( $socket, SOL_SOCKET, SO_RCVTIMEO, $options_1); +} catch (\TypeError $e) { + echo $e->getMessage() . PHP_EOL; +} + +try { + socket_set_option( $socket, SOL_SOCKET, SO_SNDTIMEO, $options_2); +} catch (\TypeError $e) { + echo $e->getMessage() . PHP_EOL; +} +try { + socket_set_option( $socket, SOL_SOCKET, SO_LINGER, $options_3); +} catch (\TypeError $e) { + echo $e->getMessage() . PHP_EOL; +} +try { + socket_set_option( $socket, SOL_SOCKET, SO_LINGER, $options_4); +} catch (\TypeError $e) { + echo $e->getMessage() . PHP_EOL; +} +?> +--EXPECT-- +socket_set_option(): Argument #4 ($value) "usec" must be an int, string given +socket_set_option(): Argument #4 ($value) "sec" must be an int, stdClass given +socket_set_option(): Argument #4 ($value) "l_onoff" must be an int, string given +socket_set_option(): Argument #4 ($value) "l_linger" must be an int, array given From 45270c639d2cf96a3d5c2f28b096334b77299821 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Thu, 12 Dec 2024 23:02:32 +0000 Subject: [PATCH 2/7] add test --- ext/sockets/tests/socket_reuseport_cbpf.phpt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ext/sockets/tests/socket_reuseport_cbpf.phpt b/ext/sockets/tests/socket_reuseport_cbpf.phpt index 1d4824dca1c81..de68e23d45d70 100644 --- a/ext/sockets/tests/socket_reuseport_cbpf.phpt +++ b/ext/sockets/tests/socket_reuseport_cbpf.phpt @@ -18,6 +18,11 @@ if (!$socket) { } var_dump(socket_set_option( $socket, SOL_SOCKET, SO_REUSEADDR, true)); var_dump(socket_set_option( $socket, SOL_SOCKET, SO_REUSEPORT, true)); +try { + socket_set_option( $socket, SOL_SOCKET, SO_ATTACH_REUSEPORT_CBPF, array()); +} catch (\TypeError $e) { + echo $e->getMessage() . PHP_EOL; +} var_dump(socket_set_option( $socket, SOL_SOCKET, SO_ATTACH_REUSEPORT_CBPF, SKF_AD_CPU)); var_dump(socket_bind($socket, '0.0.0.0')); socket_listen($socket); @@ -26,5 +31,6 @@ socket_close($socket); --EXPECT-- bool(true) bool(true) +socket_set_option(): Argument #4 ($value) must be an int, array given bool(true) bool(true) From 695861ebbff4af47b25407ae85dc0b2c35ea24b1 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 13 Dec 2024 06:50:35 +0000 Subject: [PATCH 3/7] changes from review --- ext/sockets/sockets.c | 33 +++++++++++++++---- ext/sockets/tests/socket_reuseport_cbpf.phpt | 2 +- .../tests/socket_set_option_timeo_error.phpt | 21 +++++++++--- 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/ext/sockets/sockets.c b/ext/sockets/sockets.c index cfc92123764f8..0ca47840ce2cb 100644 --- a/ext/sockets/sockets.c +++ b/ext/sockets/sockets.c @@ -1811,7 +1811,7 @@ PHP_FUNCTION(socket_set_option) HashTable *opt_ht; zval *l_onoff, *l_linger; zval *sec, *usec; - bool failed; + bool failed; if (zend_parse_parameters(ZEND_NUM_ARGS(), "Ollz", &arg1, socket_ce, &level, &optname, &arg4) == FAILURE) { RETURN_THROWS(); @@ -1872,6 +1872,11 @@ PHP_FUNCTION(socket_set_option) const char l_onoff_key[] = "l_onoff"; const char l_linger_key[] = "l_linger"; + if (Z_TYPE_P(arg4) != IS_ARRAY) { + zend_argument_type_error(4, "must be of type array, %s given", zend_zval_value_name(arg4)); + RETURN_THROWS(); + } + convert_to_array(arg4); opt_ht = Z_ARRVAL_P(arg4); @@ -1886,12 +1891,22 @@ PHP_FUNCTION(socket_set_option) zend_long zl_onoff = zval_try_get_long(l_onoff, &failed); if (failed) { - zend_argument_type_error(4, "\"%s\" must be an int, %s given", l_onoff_key, zend_zval_value_name(l_onoff)); + zend_argument_type_error(4, "\"%s\" must be of type int, %s given", l_onoff_key, zend_zval_value_name(l_onoff)); RETURN_THROWS(); } zend_long zl_linger = zval_try_get_long(l_linger, &failed); if (failed) { - zend_argument_type_error(4, "\"%s\" must be an int, %s given", l_linger_key, zend_zval_value_name(l_linger)); + zend_argument_type_error(4, "\"%s\" must be of type int, %s given", l_linger_key, zend_zval_value_name(l_linger)); + RETURN_THROWS(); + } + + if (zl_onoff < 0 || zl_onoff > USHRT_MAX) { + zend_argument_value_error(4, "\"%s\" must be between 0 and %u", l_onoff_key, USHRT_MAX); + RETURN_THROWS(); + } + + if (zl_linger < 0 || zl_linger > USHRT_MAX) { + zend_argument_value_error(4, "\"%s\" must be between 0 and %d", l_linger, USHRT_MAX); RETURN_THROWS(); } @@ -1907,7 +1922,11 @@ PHP_FUNCTION(socket_set_option) case SO_SNDTIMEO: { const char sec_key[] = "sec"; const char usec_key[] = "usec"; - bool failed; + + if (Z_TYPE_P(arg4) != IS_ARRAY) { + zend_argument_type_error(4, "must be of type array, %s given", zend_zval_value_name(arg4)); + RETURN_THROWS(); + } convert_to_array(arg4); opt_ht = Z_ARRVAL_P(arg4); @@ -1923,12 +1942,12 @@ PHP_FUNCTION(socket_set_option) zend_long zsec = zval_try_get_long(sec, &failed); if (failed) { - zend_argument_type_error(4, "\"%s\" must be an int, %s given", sec_key, zend_zval_value_name(sec)); + zend_argument_type_error(4, "\"%s\" must be of type int, %s given", sec_key, zend_zval_value_name(sec)); RETURN_THROWS(); } zend_long zusec = zval_try_get_long(usec, &failed); if (failed) { - zend_argument_type_error(4, "\"%s\" must be an int, %s given", usec_key, zend_zval_value_name(usec)); + zend_argument_type_error(4, "\"%s\" must be of type int, %s given", usec_key, zend_zval_value_name(usec)); RETURN_THROWS(); } #ifndef PHP_WIN32 @@ -1991,7 +2010,7 @@ PHP_FUNCTION(socket_set_option) case SO_ATTACH_REUSEPORT_CBPF: { zend_long fval = zval_try_get_long(arg4, &failed); if (failed) { - zend_argument_type_error(4, "must be an int, %s given", zend_zval_value_name(arg4)); + zend_argument_type_error(4, "must be of type int, %s given", zend_zval_value_name(arg4)); RETURN_THROWS(); } diff --git a/ext/sockets/tests/socket_reuseport_cbpf.phpt b/ext/sockets/tests/socket_reuseport_cbpf.phpt index de68e23d45d70..0e2925779d7e0 100644 --- a/ext/sockets/tests/socket_reuseport_cbpf.phpt +++ b/ext/sockets/tests/socket_reuseport_cbpf.phpt @@ -31,6 +31,6 @@ socket_close($socket); --EXPECT-- bool(true) bool(true) -socket_set_option(): Argument #4 ($value) must be an int, array given +socket_set_option(): Argument #4 ($value) must be of type int, array given bool(true) bool(true) diff --git a/ext/sockets/tests/socket_set_option_timeo_error.phpt b/ext/sockets/tests/socket_set_option_timeo_error.phpt index 373b97ade0c7b..139fb064e8817 100644 --- a/ext/sockets/tests/socket_set_option_timeo_error.phpt +++ b/ext/sockets/tests/socket_set_option_timeo_error.phpt @@ -13,6 +13,12 @@ $options_2 = array("sec" => new stdClass(), "usec" => "1"); $options_3 = array("l_onoff" => "aaaa", "l_linger" => "1"); $options_4 = array("l_onoff" => "1", "l_linger" => []); +try { + socket_set_option( $socket, SOL_SOCKET, SO_RCVTIMEO, new stdClass); +} catch (\TypeError $e) { + echo $e->getMessage() . PHP_EOL; +} + try { socket_set_option( $socket, SOL_SOCKET, SO_RCVTIMEO, $options_1); } catch (\TypeError $e) { @@ -24,6 +30,11 @@ try { } catch (\TypeError $e) { echo $e->getMessage() . PHP_EOL; } +try { + socket_set_option( $socket, SOL_SOCKET, SO_LINGER, "not good"); +} catch (\TypeError $e) { + echo $e->getMessage() . PHP_EOL; +} try { socket_set_option( $socket, SOL_SOCKET, SO_LINGER, $options_3); } catch (\TypeError $e) { @@ -36,7 +47,9 @@ try { } ?> --EXPECT-- -socket_set_option(): Argument #4 ($value) "usec" must be an int, string given -socket_set_option(): Argument #4 ($value) "sec" must be an int, stdClass given -socket_set_option(): Argument #4 ($value) "l_onoff" must be an int, string given -socket_set_option(): Argument #4 ($value) "l_linger" must be an int, array given +socket_set_option(): Argument #4 ($value) must be of type array, stdClass given +socket_set_option(): Argument #4 ($value) "usec" must be of type int, string given +socket_set_option(): Argument #4 ($value) "sec" must be of type int, stdClass given +socket_set_option(): Argument #4 ($value) must be of type array, string given +socket_set_option(): Argument #4 ($value) "l_onoff" must be of type int, string given +socket_set_option(): Argument #4 ($value) "l_linger" must be of type int, array given From 6c5f436285a9fcf28672cdcf0ed7932f7a98f77c Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 13 Dec 2024 07:00:38 +0000 Subject: [PATCH 4/7] add test case --- ext/sockets/tests/socket_set_option_timeo_error.phpt | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/ext/sockets/tests/socket_set_option_timeo_error.phpt b/ext/sockets/tests/socket_set_option_timeo_error.phpt index 139fb064e8817..0bfcce417dc15 100644 --- a/ext/sockets/tests/socket_set_option_timeo_error.phpt +++ b/ext/sockets/tests/socket_set_option_timeo_error.phpt @@ -12,6 +12,7 @@ $options_1 = array("sec" => 1, "usec" => "aaaaa"); $options_2 = array("sec" => new stdClass(), "usec" => "1"); $options_3 = array("l_onoff" => "aaaa", "l_linger" => "1"); $options_4 = array("l_onoff" => "1", "l_linger" => []); +$options_5 = array("l_onoff" => PHP_INT_MAX, "l_linger" => "1"); try { socket_set_option( $socket, SOL_SOCKET, SO_RCVTIMEO, new stdClass); @@ -45,11 +46,17 @@ try { } catch (\TypeError $e) { echo $e->getMessage() . PHP_EOL; } +try { + socket_set_option( $socket, SOL_SOCKET, SO_LINGER, $options_5); +} catch (\ValueError $e) { + echo $e->getMessage() . PHP_EOL; +} ?> ---EXPECT-- +--EXPECTF-- socket_set_option(): Argument #4 ($value) must be of type array, stdClass given socket_set_option(): Argument #4 ($value) "usec" must be of type int, string given socket_set_option(): Argument #4 ($value) "sec" must be of type int, stdClass given socket_set_option(): Argument #4 ($value) must be of type array, string given socket_set_option(): Argument #4 ($value) "l_onoff" must be of type int, string given socket_set_option(): Argument #4 ($value) "l_linger" must be of type int, array given +socket_set_option(): Argument #4 ($value) "l_onoff" must be between 0 and %d From 1a14443b3a63b983bca84f6ddb5961bbe25244ee Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 13 Dec 2024 18:53:50 +0000 Subject: [PATCH 5/7] changes from review also attempt to prevent UB with timeout on windows. --- ext/sockets/sockets.c | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/ext/sockets/sockets.c b/ext/sockets/sockets.c index 0ca47840ce2cb..38db034aa0677 100644 --- a/ext/sockets/sockets.c +++ b/ext/sockets/sockets.c @@ -1877,7 +1877,6 @@ PHP_FUNCTION(socket_set_option) RETURN_THROWS(); } - convert_to_array(arg4); opt_ht = Z_ARRVAL_P(arg4); if ((l_onoff = zend_hash_str_find(opt_ht, l_onoff_key, sizeof(l_onoff_key) - 1)) == NULL) { @@ -1889,29 +1888,29 @@ PHP_FUNCTION(socket_set_option) RETURN_THROWS(); } - zend_long zl_onoff = zval_try_get_long(l_onoff, &failed); + zend_long vall_lonoff = zval_try_get_long(l_onoff, &failed); if (failed) { zend_argument_type_error(4, "\"%s\" must be of type int, %s given", l_onoff_key, zend_zval_value_name(l_onoff)); RETURN_THROWS(); } - zend_long zl_linger = zval_try_get_long(l_linger, &failed); + zend_long vall_linger = zval_try_get_long(l_linger, &failed); if (failed) { zend_argument_type_error(4, "\"%s\" must be of type int, %s given", l_linger_key, zend_zval_value_name(l_linger)); RETURN_THROWS(); } - if (zl_onoff < 0 || zl_onoff > USHRT_MAX) { + if (vall_lonoff < 0 || vall_lonoff > USHRT_MAX) { zend_argument_value_error(4, "\"%s\" must be between 0 and %u", l_onoff_key, USHRT_MAX); RETURN_THROWS(); } - if (zl_linger < 0 || zl_linger > USHRT_MAX) { + if (vall_linger < 0 || vall_linger > USHRT_MAX) { zend_argument_value_error(4, "\"%s\" must be between 0 and %d", l_linger, USHRT_MAX); RETURN_THROWS(); } - lv.l_onoff = (unsigned short)zl_onoff; - lv.l_linger = (unsigned short)zl_linger; + lv.l_onoff = (unsigned short)vall_lonoff; + lv.l_linger = (unsigned short)vall_linger; optlen = sizeof(lv); opt_ptr = &lv; @@ -1928,7 +1927,6 @@ PHP_FUNCTION(socket_set_option) RETURN_THROWS(); } - convert_to_array(arg4); opt_ht = Z_ARRVAL_P(arg4); if ((sec = zend_hash_str_find(opt_ht, sec_key, sizeof(sec_key) - 1)) == NULL) { @@ -1940,23 +1938,35 @@ PHP_FUNCTION(socket_set_option) RETURN_THROWS(); } - zend_long zsec = zval_try_get_long(sec, &failed); + zend_long valsec = zval_try_get_long(sec, &failed); if (failed) { zend_argument_type_error(4, "\"%s\" must be of type int, %s given", sec_key, zend_zval_value_name(sec)); RETURN_THROWS(); } - zend_long zusec = zval_try_get_long(usec, &failed); + zend_long valusec = zval_try_get_long(usec, &failed); if (failed) { zend_argument_type_error(4, "\"%s\" must be of type int, %s given", usec_key, zend_zval_value_name(usec)); RETURN_THROWS(); } #ifndef PHP_WIN32 - tv.tv_sec = zsec; - tv.tv_usec = zusec; + tv.tv_sec = valsec; + tv.tv_usec = valusec; optlen = sizeof(tv); opt_ptr = &tv; #else - timeout = zsec * 1000 + zusec / 1000; + if (valsec < 0 || valsec > INT_MAX / 1000) { + zend_argument_value_error(4, "\"%s\" must be between 0 and %d", sec_key, INT_MAX / 1000); + RETURN_THROWS(); + } + + timeout = valsec * 1000; + + if (valusec < 0 || timeout > INT_MAX - (valusec / 1000)) { + zend_argument_value_error(4, "\"%s\" must be between 0 and %u", usec_key, (DWORD)(INT_MAX - (valusec / 1000))); + RETURN_THROWS(); + } + + timeout += valusec / 1000; optlen = sizeof(int); opt_ptr = &timeout; #endif From e18fbb3dd05914be0df6883840592b90aa6e7f0a Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 13 Dec 2024 19:31:02 +0000 Subject: [PATCH 6/7] remove UB check and using zval_get_long instead --- ext/sockets/sockets.c | 50 +++---------------- ext/sockets/tests/socket_reuseport_cbpf.phpt | 5 +- .../tests/socket_set_option_timeo_error.phpt | 6 +-- 3 files changed, 12 insertions(+), 49 deletions(-) diff --git a/ext/sockets/sockets.c b/ext/sockets/sockets.c index 38db034aa0677..8df049b790e54 100644 --- a/ext/sockets/sockets.c +++ b/ext/sockets/sockets.c @@ -1811,7 +1811,6 @@ PHP_FUNCTION(socket_set_option) HashTable *opt_ht; zval *l_onoff, *l_linger; zval *sec, *usec; - bool failed; if (zend_parse_parameters(ZEND_NUM_ARGS(), "Ollz", &arg1, socket_ce, &level, &optname, &arg4) == FAILURE) { RETURN_THROWS(); @@ -1888,16 +1887,8 @@ PHP_FUNCTION(socket_set_option) RETURN_THROWS(); } - zend_long vall_lonoff = zval_try_get_long(l_onoff, &failed); - if (failed) { - zend_argument_type_error(4, "\"%s\" must be of type int, %s given", l_onoff_key, zend_zval_value_name(l_onoff)); - RETURN_THROWS(); - } - zend_long vall_linger = zval_try_get_long(l_linger, &failed); - if (failed) { - zend_argument_type_error(4, "\"%s\" must be of type int, %s given", l_linger_key, zend_zval_value_name(l_linger)); - RETURN_THROWS(); - } + zend_long vall_lonoff = zval_get_long(l_onoff); + zend_long vall_linger = zval_get_long(l_linger); if (vall_lonoff < 0 || vall_lonoff > USHRT_MAX) { zend_argument_value_error(4, "\"%s\" must be between 0 and %u", l_onoff_key, USHRT_MAX); @@ -1938,35 +1929,16 @@ PHP_FUNCTION(socket_set_option) RETURN_THROWS(); } - zend_long valsec = zval_try_get_long(sec, &failed); - if (failed) { - zend_argument_type_error(4, "\"%s\" must be of type int, %s given", sec_key, zend_zval_value_name(sec)); - RETURN_THROWS(); - } - zend_long valusec = zval_try_get_long(usec, &failed); - if (failed) { - zend_argument_type_error(4, "\"%s\" must be of type int, %s given", usec_key, zend_zval_value_name(usec)); - RETURN_THROWS(); - } + zend_long valsec = zval_get_long(sec); + zend_long valusec = zval_get_long(usec); #ifndef PHP_WIN32 tv.tv_sec = valsec; tv.tv_usec = valusec; optlen = sizeof(tv); opt_ptr = &tv; #else - if (valsec < 0 || valsec > INT_MAX / 1000) { - zend_argument_value_error(4, "\"%s\" must be between 0 and %d", sec_key, INT_MAX / 1000); - RETURN_THROWS(); - } - - timeout = valsec * 1000; + timeout = valsec * 1000 + valusec / 1000; - if (valusec < 0 || timeout > INT_MAX - (valusec / 1000)) { - zend_argument_value_error(4, "\"%s\" must be between 0 and %u", usec_key, (DWORD)(INT_MAX - (valusec / 1000))); - RETURN_THROWS(); - } - - timeout += valusec / 1000; optlen = sizeof(int); opt_ptr = &timeout; #endif @@ -2018,11 +1990,7 @@ PHP_FUNCTION(socket_set_option) #ifdef SO_ATTACH_REUSEPORT_CBPF case SO_ATTACH_REUSEPORT_CBPF: { - zend_long fval = zval_try_get_long(arg4, &failed); - if (failed) { - zend_argument_type_error(4, "must be of type int, %s given", zend_zval_value_name(arg4)); - RETURN_THROWS(); - } + zend_long fval = zval_get_long(arg4); if (!fval) { ov = 1; @@ -2057,11 +2025,7 @@ PHP_FUNCTION(socket_set_option) default: default_case: - ov = zval_try_get_long(arg4, &failed); - if (failed) { - zend_argument_type_error(4, "must be an int, %s given", zend_zval_value_name(arg4)); - RETURN_THROWS(); - } + ov = zval_get_long(arg4); optlen = sizeof(ov); opt_ptr = &ov; diff --git a/ext/sockets/tests/socket_reuseport_cbpf.phpt b/ext/sockets/tests/socket_reuseport_cbpf.phpt index 0e2925779d7e0..2210c4438f000 100644 --- a/ext/sockets/tests/socket_reuseport_cbpf.phpt +++ b/ext/sockets/tests/socket_reuseport_cbpf.phpt @@ -28,9 +28,10 @@ var_dump(socket_bind($socket, '0.0.0.0')); socket_listen($socket); socket_close($socket); ?> ---EXPECT-- +--EXPECTF-- bool(true) bool(true) -socket_set_option(): Argument #4 ($value) must be of type int, array given + +Warning: socket_set_option(): Unable to set socket option [2]: No such file or directory in %s on line %d bool(true) bool(true) diff --git a/ext/sockets/tests/socket_set_option_timeo_error.phpt b/ext/sockets/tests/socket_set_option_timeo_error.phpt index 0bfcce417dc15..cb37578c71ae8 100644 --- a/ext/sockets/tests/socket_set_option_timeo_error.phpt +++ b/ext/sockets/tests/socket_set_option_timeo_error.phpt @@ -54,9 +54,7 @@ try { ?> --EXPECTF-- socket_set_option(): Argument #4 ($value) must be of type array, stdClass given -socket_set_option(): Argument #4 ($value) "usec" must be of type int, string given -socket_set_option(): Argument #4 ($value) "sec" must be of type int, stdClass given + +Warning: Object of class stdClass could not be converted to int in %s on line %d socket_set_option(): Argument #4 ($value) must be of type array, string given -socket_set_option(): Argument #4 ($value) "l_onoff" must be of type int, string given -socket_set_option(): Argument #4 ($value) "l_linger" must be of type int, array given socket_set_option(): Argument #4 ($value) "l_onoff" must be between 0 and %d From d6e64e696bc9d889fc08bf2d5c18f682f9163bd2 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sun, 15 Dec 2024 23:15:30 +0000 Subject: [PATCH 7/7] other changes from review --- ext/sockets/sockets.c | 44 ++++++++++++------- .../tests/socket_set_option_timeo_error.phpt | 14 ++++-- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/ext/sockets/sockets.c b/ext/sockets/sockets.c index 8df049b790e54..e6e231e2e5e7e 100644 --- a/ext/sockets/sockets.c +++ b/ext/sockets/sockets.c @@ -1872,11 +1872,15 @@ PHP_FUNCTION(socket_set_option) const char l_linger_key[] = "l_linger"; if (Z_TYPE_P(arg4) != IS_ARRAY) { - zend_argument_type_error(4, "must be of type array, %s given", zend_zval_value_name(arg4)); - RETURN_THROWS(); + if (UNEXPECTED(Z_TYPE_P(arg4) != IS_OBJECT)) { + zend_argument_type_error(4, "must be of type array when argument #3 ($option) is SO_LINGER, %s given", zend_zval_value_name(arg4)); + RETURN_THROWS(); + } else { + opt_ht = Z_OBJPROP_P(arg4); + } + } else { + opt_ht = Z_ARRVAL_P(arg4); } - - opt_ht = Z_ARRVAL_P(arg4); if ((l_onoff = zend_hash_str_find(opt_ht, l_onoff_key, sizeof(l_onoff_key) - 1)) == NULL) { zend_argument_value_error(4, "must have key \"%s\"", l_onoff_key); @@ -1887,21 +1891,21 @@ PHP_FUNCTION(socket_set_option) RETURN_THROWS(); } - zend_long vall_lonoff = zval_get_long(l_onoff); - zend_long vall_linger = zval_get_long(l_linger); + zend_long val_lonoff = zval_get_long(l_onoff); + zend_long val_linger = zval_get_long(l_linger); - if (vall_lonoff < 0 || vall_lonoff > USHRT_MAX) { + if (val_lonoff < 0 || val_lonoff > USHRT_MAX) { zend_argument_value_error(4, "\"%s\" must be between 0 and %u", l_onoff_key, USHRT_MAX); RETURN_THROWS(); } - if (vall_linger < 0 || vall_linger > USHRT_MAX) { + if (val_linger < 0 || val_linger > USHRT_MAX) { zend_argument_value_error(4, "\"%s\" must be between 0 and %d", l_linger, USHRT_MAX); RETURN_THROWS(); } - lv.l_onoff = (unsigned short)vall_lonoff; - lv.l_linger = (unsigned short)vall_linger; + lv.l_onoff = (unsigned short)val_lonoff; + lv.l_linger = (unsigned short)val_linger; optlen = sizeof(lv); opt_ptr = &lv; @@ -1914,12 +1918,18 @@ PHP_FUNCTION(socket_set_option) const char usec_key[] = "usec"; if (Z_TYPE_P(arg4) != IS_ARRAY) { - zend_argument_type_error(4, "must be of type array, %s given", zend_zval_value_name(arg4)); - RETURN_THROWS(); + if (UNEXPECTED(Z_TYPE_P(arg4) != IS_OBJECT)) { + zend_argument_type_error(4, "must be of type array when argument #3 ($option) is %s, %s given", + optname == SO_RCVTIMEO ? "SO_RCVTIMEO" : "SO_SNDTIMEO", + zend_zval_value_name(arg4)); + RETURN_THROWS(); + } else { + opt_ht = Z_OBJPROP_P(arg4); + } + } else { + opt_ht = Z_ARRVAL_P(arg4); } - opt_ht = Z_ARRVAL_P(arg4); - if ((sec = zend_hash_str_find(opt_ht, sec_key, sizeof(sec_key) - 1)) == NULL) { zend_argument_value_error(4, "must have key \"%s\"", sec_key); RETURN_THROWS(); @@ -1990,15 +2000,15 @@ PHP_FUNCTION(socket_set_option) #ifdef SO_ATTACH_REUSEPORT_CBPF case SO_ATTACH_REUSEPORT_CBPF: { - zend_long fval = zval_get_long(arg4); + zend_long cbpf_val = zval_get_long(arg4); - if (!fval) { + if (!cbpf_val) { ov = 1; optlen = sizeof(ov); opt_ptr = &ov; optname = SO_DETACH_BPF; } else { - uint32_t k = (uint32_t)fval; + uint32_t k = (uint32_t)cbpf_val; static struct sock_filter cbpf[8] = {0}; static struct sock_fprog bpfprog; diff --git a/ext/sockets/tests/socket_set_option_timeo_error.phpt b/ext/sockets/tests/socket_set_option_timeo_error.phpt index cb37578c71ae8..1db5e01c36228 100644 --- a/ext/sockets/tests/socket_set_option_timeo_error.phpt +++ b/ext/sockets/tests/socket_set_option_timeo_error.phpt @@ -16,7 +16,7 @@ $options_5 = array("l_onoff" => PHP_INT_MAX, "l_linger" => "1"); try { socket_set_option( $socket, SOL_SOCKET, SO_RCVTIMEO, new stdClass); -} catch (\TypeError $e) { +} catch (\ValueError $e) { echo $e->getMessage() . PHP_EOL; } @@ -32,7 +32,12 @@ try { echo $e->getMessage() . PHP_EOL; } try { - socket_set_option( $socket, SOL_SOCKET, SO_LINGER, "not good"); + socket_set_option( $socket, SOL_SOCKET, SO_RCVTIMEO, "not good"); +} catch (\TypeError $e) { + echo $e->getMessage() . PHP_EOL; +} +try { + socket_set_option( $socket, SOL_SOCKET, SO_LINGER, "not good neither"); } catch (\TypeError $e) { echo $e->getMessage() . PHP_EOL; } @@ -53,8 +58,9 @@ try { } ?> --EXPECTF-- -socket_set_option(): Argument #4 ($value) must be of type array, stdClass given +socket_set_option(): Argument #4 ($value) must have key "sec" Warning: Object of class stdClass could not be converted to int in %s on line %d -socket_set_option(): Argument #4 ($value) must be of type array, string given +socket_set_option(): Argument #4 ($value) must be of type array when argument #3 ($option) is SO_RCVTIMEO, string given +socket_set_option(): Argument #4 ($value) must be of type array when argument #3 ($option) is SO_LINGER, string given socket_set_option(): Argument #4 ($value) "l_onoff" must be between 0 and %d