ext/sockets: Additional refactorings to socket_set_option() (#17186)

* ext/socket: Reduce scope of variables

* ext/socket: Throw TypeErrors when not passing a string for options requiring one

* ext/sockets: Throw ValueError when string has null bytes for options not passing string length

* ext/sockets: Throw ValueError when string is too long

And replace calls to strlcpy to memcpy

ext/sockets/sockets.c

@@ -1888,8 +1888,6 @@ PHP_FUNCTION(socket_set_option)
 	zend_long					level, optname;
 	void 					*opt_ptr;
 	HashTable		 		*opt_ht;
-	zval 					*l_onoff, *l_linger;
-	zval		 			*sec, *usec;
 
 	ZEND_PARSE_PARAMETERS_START(4, 4)
 		Z_PARAM_OBJECT_OF_CLASS(arg1, socket_ce)
@@ -1930,13 +1928,12 @@ PHP_FUNCTION(socket_set_option)
 		switch (optname) {
 #ifdef TCP_CONGESTION
 		case TCP_CONGESTION: {
-			if (Z_TYPE_P(arg4) == IS_STRING) {
+			if (Z_TYPE_P(arg4) != IS_STRING) {
-				opt_ptr = Z_STRVAL_P(arg4);
+				zend_argument_type_error(4, "must be of type string when argument #3 ($option) is TCP_CONGESTION, %s given", zend_zval_value_name(arg4));
-				optlen = Z_STRLEN_P(arg4);
+				RETURN_THROWS();
-			} else {
-				opt_ptr = "";
-				optlen = 0;
 			}
+			opt_ptr = Z_STRVAL_P(arg4);
+			optlen = Z_STRLEN_P(arg4);
 			if (setsockopt(php_sock->bsd_socket, level, optname, opt_ptr, optlen) != 0) {
 				PHP_SOCKET_ERROR(php_sock, "Unable to set socket option", errno);
 				RETURN_FALSE;
@@ -1949,11 +1946,23 @@ PHP_FUNCTION(socket_set_option)
 #ifdef TCP_FUNCTION_BLK
 		case TCP_FUNCTION_BLK: {
 			if (Z_TYPE_P(arg4) != IS_STRING) {
-				php_error_docref(NULL, E_WARNING, "Invalid tcp stack name argument type");
+				zend_argument_type_error(4, "must be of type string when argument #3 ($option) is TCP_FUNCTION_BLK, %s given", zend_zval_value_name(arg4));
-				RETURN_FALSE;
+				RETURN_THROWS();
+			}
+			if (zend_str_has_nul_byte(Z_STR_P(arg4))) {
+				zend_argument_value_error(4, "must not contain null bytes when argument #3 ($option) is TCP_FUNCTION_BLK");
+				RETURN_THROWS();
+			}
+			/* [...] the unique name of the TCP stack, and should be no longer than
+			 * TCP_FUNCTION_NAME_LEN_MAX-1 characters in length.
+			 * https://github.com/freebsd/freebsd-src/blob/da2c88dfcf4f425e6e0a58d6df3a7c8e88d8df92/share/man/man9/tcp_functions.9#L193C1-L194C50 */
+			if (Z_STRLEN_P(arg4) >= TCP_FUNCTION_NAME_LEN_MAX) {
+				zend_argument_value_error(4, "must be less than %zu bytes when argument #3 ($option) is TCP_FUNCTION_BLK", TCP_FUNCTION_NAME_LEN_MAX);
+				RETURN_THROWS();
 			}
 			struct tcp_function_set tfs = {0};
-			strlcpy(tfs.function_set_name, Z_STRVAL_P(arg4), TCP_FUNCTION_NAME_LEN_MAX);
+			/* Copy the trailing nul byte */
+			memcpy(tfs.function_set_name, Z_STRVAL_P(arg4), Z_STRLEN_P(arg4)+1);
 
 			optlen = sizeof(tfs);
 			opt_ptr = &tfs;
@@ -1976,6 +1985,7 @@ PHP_FUNCTION(socket_set_option)
 		case SO_LINGER: {
 			const char l_onoff_key[] = "l_onoff";
 			const char l_linger_key[] = "l_linger";
+			zval *l_onoff, *l_linger;
 
 			if (Z_TYPE_P(arg4) != IS_ARRAY) {
 				if (UNEXPECTED(Z_TYPE_P(arg4) != IS_OBJECT)) {
@@ -2022,6 +2032,7 @@ PHP_FUNCTION(socket_set_option)
 		case SO_SNDTIMEO: {
 			const char sec_key[] = "sec";
 			const char usec_key[] = "usec";
+			zval *sec, *usec;
 
 			if (Z_TYPE_P(arg4) != IS_ARRAY) {
 				if (UNEXPECTED(Z_TYPE_P(arg4) != IS_OBJECT)) {
@@ -2078,13 +2089,12 @@ PHP_FUNCTION(socket_set_option)
 		}
 #ifdef SO_BINDTODEVICE
 		case SO_BINDTODEVICE: {
-			if (Z_TYPE_P(arg4) == IS_STRING) {
+			if (Z_TYPE_P(arg4) != IS_STRING) {
-				opt_ptr = Z_STRVAL_P(arg4);
+				zend_argument_type_error(4, "must be of type string when argument #3 ($option) is SO_BINDTODEVICE, %s given", zend_zval_value_name(arg4));
-				optlen = Z_STRLEN_P(arg4);
+				RETURN_THROWS();
-			} else {
-				opt_ptr = "";
-				optlen = 0;
 			}
+			opt_ptr = Z_STRVAL_P(arg4);
+			optlen = Z_STRLEN_P(arg4);
 			break;
 		}
 #endif
@@ -2092,11 +2102,20 @@ PHP_FUNCTION(socket_set_option)
 #ifdef SO_ACCEPTFILTER
 		case SO_ACCEPTFILTER: {
 			if (Z_TYPE_P(arg4) != IS_STRING) {
-				php_error_docref(NULL, E_WARNING, "Invalid filter argument type");
+				zend_argument_type_error(4, "must be of type string when argument #3 ($option) is SO_ACCEPTFILTER, %s given", zend_zval_value_name(arg4));
-				RETURN_FALSE;
+				RETURN_THROWS();
+			}
+			if (zend_str_has_nul_byte(Z_STR_P(arg4))) {
+				zend_argument_value_error(4, "must not contain null bytes when argument #3 ($option) is SO_ACCEPTFILTER");
+				RETURN_THROWS();
 			}
 			struct accept_filter_arg af = {0};
-			strlcpy(af.af_name, Z_STRVAL_P(arg4), sizeof(af.af_name));
+			if (Z_STRLEN_P(arg4) >= sizeof(af.af_name)) {
+				zend_argument_value_error(4, "must be less than %zu bytes when argument #3 ($option) is SO_ACCEPTFILTER", sizeof(af.af_name));
+				RETURN_THROWS();
+			}
+			/* Copy the trailing nul byte */
+			memcpy(af.af_name, Z_STRVAL_P(arg4), Z_STRLEN_P(arg4)+1);
 			opt_ptr = ⁡
 			optlen = sizeof(af);
 			break;
@@ -2111,8 +2130,8 @@ PHP_FUNCTION(socket_set_option)
 				RETURN_FALSE;
 			}
 			if (Z_TYPE_P(arg4) != IS_STRING) {
-				php_error_docref(NULL, E_WARNING, "Invalid filter argument type");
+				zend_argument_type_error(4, "must be of type string when argument #3 ($option) is FIL_ATTACH or FIL_DETACH, %s given", zend_zval_value_name(arg4));
-				RETURN_FALSE;
+				RETURN_THROWS();
 			}
 			opt_ptr = Z_STRVAL_P(arg4);
 			optlen = Z_STRLEN_P(arg4);

ext/sockets/tests/socket_set_option_acf.phpt

@@ -21,17 +21,16 @@ if (!$socket) {
 }
 try {
 	var_dump(socket_set_option( $socket, SOL_SOCKET, SO_ACCEPTFILTER, 1));
-} catch (\ValueError $e) {
+} catch (\Throwable $e) {
-    echo $e->getMessage() . \PHP_EOL;
+    echo $e::class, ': ', $e->getMessage(), \PHP_EOL;
 }
 socket_listen($socket);
 var_dump(socket_set_option( $socket, SOL_SOCKET, SO_ACCEPTFILTER, "httpready"));
 var_dump(socket_get_option( $socket, SOL_SOCKET, SO_ACCEPTFILTER));
 socket_close($socket);
 ?>
---EXPECTF--
+--EXPECT--
-Warning: socket_set_option(): Invalid filter argument type in %s on line %d
+TypeError: socket_set_option(): Argument #4 ($value) must be of type string when argument #3 ($option) is SO_ACCEPTFILTER, int given
-bool(false)
 bool(true)
 array(1) {
   ["af_name"]=>

ext/sockets/tests/socket_set_option_invalid_value_for_SOL_FILTER_SO_ATTACH.phpt

@@ -0,0 +1,44 @@
+--TEST--
+socket_set_option($socket, SOL_SOCKET, SO_BINDTODEVICE, INVALID_TYPE_FOR_OPTION)
+--EXTENSIONS--
+sockets
+--SKIPIF--
+<?php
+
+if (!defined('SOL_FILTER')) {
+    die('skip SOL_FILTER not available.');
+}
+if (!defined('FIL_ATTACH')) {
+    die('skip FIL_ATTACH not available.');
+}
+if (!defined('FIL_DETACH')) {
+    die('skip FIL_DETACH not available.');
+}
+
+?>
+--FILE--
+<?php
+$socket = socket_create(AF_INET, SOCK_STREAM, SOL_TCP);
+if (!$socket) {
+    die('Unable to create AF_INET socket [socket]');
+}
+
+// TODO Warning when using FIL_ATTACH/FIL_DETACH option when level is not SOL_FILTER
+try {
+    $ret = socket_set_option($socket, SOL_FILTER, FIL_ATTACH, new stdClass());
+    var_dump($ret);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    $ret = socket_set_option($socket, SOL_FILTER, FIL_DETACH, new stdClass());
+    var_dump($ret);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+socket_close($socket);
+?>
+--EXPECT--
+TypeError: socket_set_option(): Argument #4 ($value) must be of type string when argument #3 ($option) is FIL_ATTACH or FIL_DETACH, stdClass given
+TypeError: socket_set_option(): Argument #4 ($value) must be of type string when argument #3 ($option) is FIL_ATTACH or FIL_DETACH, stdClass given

ext/sockets/tests/socket_set_option_invalid_value_for_SOL_SOCKET_SO_ACCEPTFILTER.phpt

@@ -0,0 +1,47 @@
+--TEST--
+socket_set_option($socket, SOL_SOCKET, SO_ACCEPTFILTER, INVALID_TYPE_FOR_OPTION)
+--EXTENSIONS--
+sockets
+--SKIPIF--
+<?php
+
+if (!defined('SOL_SOCKET')) {
+    die('skip SOL_SOCKET not available.');
+}
+if (!defined('SO_ACCEPTFILTER')) {
+    die('skip SO_ACCEPTFILTER not available.');
+}
+
+?>
+--FILE--
+<?php
+$socket = socket_create(AF_INET, SOCK_STREAM, SOL_TCP);
+if (!$socket) {
+    die('Unable to create AF_INET socket [socket]');
+}
+
+try {
+    $ret = socket_set_option($socket, SOL_SOCKET, SO_ACCEPTFILTER, new stdClass());
+    var_dump($ret);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    $ret = socket_set_option($socket, SOL_SOCKET, SO_ACCEPTFILTER, "string\0with\0null\0bytes");
+    var_dump($ret);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    $ret = socket_set_option($socket, SOL_SOCKET, SO_ACCEPTFILTER, str_repeat("a", 2048));
+    var_dump($ret);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+socket_close($socket);
+?>
+--EXPECTF--
+TypeError: socket_set_option(): Argument #4 ($value) must be of type string when argument #3 ($option) is SO_ACCEPTFILTER, stdClass given
+ValueError: socket_set_option(): Argument #4 ($value) must not contain null bytes when argument #3 ($option) is SO_ACCEPTFILTER
+ValueError: socket_set_option(): Argument #4 ($value) must be less than %d bytes when argument #3 ($option) is SO_ACCEPTFILTER

ext/sockets/tests/socket_set_option_invalid_value_for_SOL_SOCKET_SO_BINDTODEVICE.phpt

@@ -0,0 +1,33 @@
+--TEST--
+socket_set_option($socket, SOL_SOCKET, SO_BINDTODEVICE, INVALID_TYPE_FOR_OPTION)
+--EXTENSIONS--
+sockets
+--SKIPIF--
+<?php
+
+if (!defined('SOL_SOCKET')) {
+    die('skip SOL_SOCKET not available.');
+}
+if (!defined('SO_BINDTODEVICE')) {
+    die('skip SO_BINDTODEVICE not available.');
+}
+
+?>
+--FILE--
+<?php
+$socket = socket_create(AF_INET, SOCK_STREAM, SOL_TCP);
+if (!$socket) {
+    die('Unable to create AF_INET socket [socket]');
+}
+
+try {
+    $ret = socket_set_option($socket, SOL_SOCKET, SO_BINDTODEVICE, new stdClass());
+    var_dump($ret);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+socket_close($socket);
+?>
+--EXPECT--
+TypeError: socket_set_option(): Argument #4 ($value) must be of type string when argument #3 ($option) is SO_BINDTODEVICE, stdClass given

ext/sockets/tests/socket_set_option_invalid_value_for_SOL_TCP_TCP_CONGESTION.phpt

@@ -0,0 +1,30 @@
+--TEST--
+socket_set_option($socket, SOL_TCP, TCP_CONGESTION, INVALID_TYPE_FOR_OPTION)
+--EXTENSIONS--
+sockets
+--SKIPIF--
+<?php
+
+if (!defined('TCP_CONGESTION')) {
+    die('skip TCP_CONGESTION not available.');
+}
+
+?>
+--FILE--
+<?php
+$socket = socket_create(AF_INET, SOCK_STREAM, SOL_TCP);
+if (!$socket) {
+    die('Unable to create AF_INET socket [socket]');
+}
+
+try {
+    $ret = socket_set_option($socket, SOL_TCP, TCP_CONGESTION, new stdClass());
+    var_dump($ret);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+socket_close($socket);
+?>
+--EXPECT--
+TypeError: socket_set_option(): Argument #4 ($value) must be of type string when argument #3 ($option) is TCP_CONGESTION, stdClass given

ext/sockets/tests/socket_set_option_invalid_value_for_SOL_TCP_TCP_FUNCTION_BLK.phpt

@@ -0,0 +1,44 @@
+--TEST--
+socket_set_option($socket, SOL_TCP, TCP_FUNCTION_BLK, INVALID_TYPE_FOR_OPTION)
+--EXTENSIONS--
+sockets
+--SKIPIF--
+<?php
+
+if (!defined('TCP_FUNCTION_BLK')) {
+    die('skip TCP_FUNCTION_BLK not available.');
+}
+
+?>
+--FILE--
+<?php
+$socket = socket_create(AF_INET, SOCK_STREAM, SOL_TCP);
+if (!$socket) {
+    die('Unable to create AF_INET socket [socket]');
+}
+
+try {
+    $ret = socket_set_option($socket, SOL_TCP, TCP_FUNCTION_BLK, new stdClass());
+    var_dump($ret);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    $ret = socket_set_option($socket, SOL_TCP, TCP_FUNCTION_BLK, "string\0with\0null\0bytes");
+    var_dump($ret);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    $ret = socket_set_option($socket, SOL_TCP, TCP_FUNCTION_BLK, str_repeat("a", 2048));
+    var_dump($ret);
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+socket_close($socket);
+?>
+--EXPECTF--
+TypeError: socket_set_option(): Argument #4 ($value) must be of type string when argument #3 ($option) is TCP_FUNCTION_BLK, stdClass given
+ValueError: socket_set_option(): Argument #4 ($value) must not contain null bytes when argument #3 ($option) is TCP_FUNCTION_BLK
+ValueError: socket_set_option(): Argument #4 ($value) must be less than %d bytes when argument #3 ($option) is TCP_FUNCTION_BLK