Skip to content

Commit 90ce729

Browse files
committed
Address code review and fix some issues
1 parent 2738106 commit 90ce729

21 files changed

+80
-85
lines changed

UPGRADING

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -466,9 +466,7 @@ PHP 8.0 UPGRADE NOTES
466466
socket_import_stream(), socket_addrinfo_connect(), socket_addrinfo_bind(),
467467
and socket_wsaprotocol_info_import() will now return a Socket object rather
468468
than a resource. Return value checks using is_resource() should be replaced
469-
with checks for `false`. The socket_close() function is deprecated and no
470-
longer has an effect, instead the Socket instance is automatically
471-
destroyed if it is no longer referenced.
469+
with checks for `false`.
472470
. socket_addrinfo_lookup() will now return an array of AddressInfo objects
473471
rather than resources.
474472

ext/posix/tests/posix_ttyname_error_wrongparams.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ PHP Testfest Berlin 2009-05-10
2020
<?php
2121
var_dump(posix_ttyname(0)); // param not a resource
2222
try {
23-
var_dump(posix_ttyname(socket_create(AF_INET, SOCK_DGRAM, SOL_UDP))); // wrong resource type
23+
var_dump(posix_ttyname(finfo_open(FILEINFO_NONE, __DIR__))); // wrong resource type
2424
} catch (TypeError $e) {
2525
echo $e->getMessage(), "\n";
2626
}

ext/sockets/conversions.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1409,7 +1409,6 @@ void to_zval_read_fd_array(const char *data, zval *zv, res_context *ctx)
14091409
return;
14101410
}
14111411
if (S_ISSOCK(statbuf.st_mode)) {
1412-
14131412
object_init_ex(&elem, socket_ce);
14141413
php_socket *sock = Z_SOCKET_P(&elem);
14151414

ext/sockets/sockets.c

Lines changed: 37 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -144,10 +144,8 @@ static HashTable *socket_get_gc(zend_object *object, zval **table, int *n)
144144
{
145145
php_socket *socket = socket_from_obj(object);
146146

147-
zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create();
148-
149-
zend_get_gc_buffer_add_zval(gc_buffer, &socket->zstream);
150-
zend_get_gc_buffer_use(gc_buffer, table, n);
147+
*table = &socket->zstream;
148+
*n = 1;
151149

152150
return zend_std_get_properties(object);
153151
}
@@ -933,11 +931,27 @@ PHP_FUNCTION(socket_listen)
933931
/* {{{ Closes a file descriptor */
934932
PHP_FUNCTION(socket_close)
935933
{
936-
zval *arg1;
934+
zval *arg1;
935+
php_socket *php_socket;
937936

938937
if (zend_parse_parameters(ZEND_NUM_ARGS(), "O", &arg1, socket_ce) == FAILURE) {
939938
RETURN_THROWS();
940939
}
940+
941+
php_socket = Z_SOCKET_P(arg1);
942+
943+
if (!Z_ISUNDEF(php_socket->zstream)) {
944+
php_stream *stream = NULL;
945+
php_stream_from_zval_no_verify(stream, &php_socket->zstream);
946+
if (stream != NULL) {
947+
/* close & destroy stream, incl. removing it from the rsrc list;
948+
* resource stored in php_sock->zstream will become invalid */
949+
php_stream_free(stream,
950+
PHP_STREAM_FREE_KEEP_RSRC | PHP_STREAM_FREE_CLOSE |
951+
(stream->is_persistent?PHP_STREAM_FREE_CLOSE_PERSISTENT:0));
952+
}
953+
ZVAL_UNDEF(&php_socket->zstream);
954+
}
941955
}
942956
/* }}} */
943957

@@ -1236,7 +1250,7 @@ PHP_FUNCTION(socket_connect)
12361250
int retval;
12371251
size_t addr_len;
12381252
zend_long port;
1239-
zend_bool port_is_null;
1253+
zend_bool port_is_null = 1;
12401254

12411255
if (zend_parse_parameters(ZEND_NUM_ARGS(), "Os|l!", &resource_socket, socket_ce, &addr, &addr_len, &port, &port_is_null) == FAILURE) {
12421256
RETURN_THROWS();
@@ -1249,8 +1263,8 @@ PHP_FUNCTION(socket_connect)
12491263
case AF_INET6: {
12501264
struct sockaddr_in6 sin6 = {0};
12511265

1252-
if (port) {
1253-
zend_argument_value_error(3, "must be specified for the AF_INET6 socket type");
1266+
if (port_is_null) {
1267+
zend_argument_value_error(3, "cannot be null when the socket type is AF_INET6");
12541268
RETURN_THROWS();
12551269
}
12561270

@@ -1271,7 +1285,7 @@ PHP_FUNCTION(socket_connect)
12711285
struct sockaddr_in sin = {0};
12721286

12731287
if (port_is_null) {
1274-
zend_argument_value_error(3, "cannot be null for the AF_INET socket type");
1288+
zend_argument_value_error(3, "cannot be null when the socket type is AF_INET");
12751289
RETURN_THROWS();
12761290
}
12771291

@@ -1461,13 +1475,13 @@ PHP_FUNCTION(socket_send)
14611475
RETURN_THROWS();
14621476
}
14631477

1464-
php_sock = Z_SOCKET_P(arg1);
1465-
14661478
if (len < 0) {
14671479
zend_argument_value_error(3, "must be greater than or equal to 0");
14681480
RETURN_THROWS();
14691481
}
14701482

1483+
php_sock = Z_SOCKET_P(arg1);
1484+
14711485
retval = send(php_sock->bsd_socket, buf, (buf_len < (size_t)len ? buf_len : (size_t)len), flags);
14721486

14731487
if (retval == (size_t)-1) {
@@ -1606,21 +1620,21 @@ PHP_FUNCTION(socket_sendto)
16061620
#endif
16071621
int retval;
16081622
size_t buf_len, addr_len;
1609-
zend_long len, flags, port = 0;
1623+
zend_long len, flags, port;
1624+
zend_bool port_is_null = 1;
16101625
char *buf, *addr;
1611-
int argc = ZEND_NUM_ARGS();
16121626

1613-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "Oslls|l", &arg1, socket_ce, &buf, &buf_len, &len, &flags, &addr, &addr_len, &port) == FAILURE) {
1627+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "Oslls|l!", &arg1, socket_ce, &buf, &buf_len, &len, &flags, &addr, &addr_len, &port, &port_is_null) == FAILURE) {
16141628
RETURN_THROWS();
16151629
}
16161630

1617-
php_sock = Z_SOCKET_P(arg1);
1618-
16191631
if (len < 0) {
16201632
zend_argument_value_error(3, "must be greater than or equal to 0");
16211633
RETURN_THROWS();
16221634
}
16231635

1636+
php_sock = Z_SOCKET_P(arg1);
1637+
16241638
switch (php_sock->type) {
16251639
case AF_UNIX:
16261640
memset(&s_un, 0, sizeof(s_un));
@@ -1631,8 +1645,9 @@ PHP_FUNCTION(socket_sendto)
16311645
break;
16321646

16331647
case AF_INET:
1634-
if (argc != 6) {
1635-
WRONG_PARAM_COUNT;
1648+
if (port_is_null) {
1649+
zend_argument_value_error(6, "cannot be null when the socket type is AF_INET");
1650+
RETURN_THROWS();
16361651
}
16371652

16381653
memset(&sin, 0, sizeof(sin));
@@ -1647,8 +1662,9 @@ PHP_FUNCTION(socket_sendto)
16471662
break;
16481663
#if HAVE_IPV6
16491664
case AF_INET6:
1650-
if (argc != 6) {
1651-
WRONG_PARAM_COUNT;
1665+
if (port_is_null) {
1666+
zend_argument_value_error(6, "cannot be null when the socket type is AF_INET6");
1667+
RETURN_THROWS();
16521668
}
16531669

16541670
memset(&sin6, 0, sizeof(sin6));
@@ -2240,7 +2256,7 @@ PHP_FUNCTION(socket_export_stream)
22402256
PHP_FUNCTION(socket_addrinfo_lookup)
22412257
{
22422258
char *service = NULL;
2243-
size_t service_len;
2259+
size_t service_len = 0;
22442260
zend_string *hostname, *key;
22452261
zval *hint, *zhints = NULL;
22462262

ext/sockets/sockets.stub.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ function socket_set_block(Socket $socket): bool {}
2222

2323
function socket_listen(Socket $socket, int $backlog = 0): bool {}
2424

25-
/** @deprecated */
2625
function socket_close(Socket $socket): void {}
2726

2827
function socket_write(Socket $socket, string $buf, ?int $length = null): int|false {}
@@ -60,7 +59,7 @@ function socket_send(Socket $socket, string $buf, int $len, int $flags): int|fal
6059
*/
6160
function socket_recvfrom(Socket $socket, &$buf, int $len, int $flags, &$name, &$port = UNKNOWN): int|false {}
6261

63-
function socket_sendto(Socket $socket, string $buf, int $len, int $flags, string $addr, int $port = 0): int|false {}
62+
function socket_sendto(Socket $socket, string $buf, int $len, int $flags, string $addr, ?int $port = null): int|false {}
6463

6564
function socket_get_option(Socket $socket, int $level, int $optname): array|int|false {}
6665

ext/sockets/sockets_arginfo.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* This is a generated file, edit the .stub.php file instead.
2-
* Stub hash: 1b2cbff950087492186cb8c216864b6e51abcf28 */
2+
* Stub hash: 3256069f3943ec6dac1db915d737324962dda7c4 */
33

44
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_socket_select, 0, 4, MAY_BE_LONG|MAY_BE_FALSE)
55
ZEND_ARG_TYPE_INFO(1, read_fds, IS_ARRAY, 1)
@@ -104,7 +104,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_socket_sendto, 0, 5, MAY_BE_LONG
104104
ZEND_ARG_TYPE_INFO(0, len, IS_LONG, 0)
105105
ZEND_ARG_TYPE_INFO(0, flags, IS_LONG, 0)
106106
ZEND_ARG_TYPE_INFO(0, addr, IS_STRING, 0)
107-
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, port, IS_LONG, 0, "0")
107+
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, port, IS_LONG, 1, "null")
108108
ZEND_END_ARG_INFO()
109109

110110
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_socket_get_option, 0, 3, MAY_BE_ARRAY|MAY_BE_LONG|MAY_BE_FALSE)
@@ -266,7 +266,7 @@ static const zend_function_entry ext_functions[] = {
266266
ZEND_FE(socket_set_nonblock, arginfo_socket_set_nonblock)
267267
ZEND_FE(socket_set_block, arginfo_socket_set_block)
268268
ZEND_FE(socket_listen, arginfo_socket_listen)
269-
ZEND_DEP_FE(socket_close, arginfo_socket_close)
269+
ZEND_FE(socket_close, arginfo_socket_close)
270270
ZEND_FE(socket_write, arginfo_socket_write)
271271
ZEND_FE(socket_read, arginfo_socket_read)
272272
ZEND_FE(socket_getsockname, arginfo_socket_getsockname)

ext/sockets/tests/bug76839.phpt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,9 @@ for ($i = 0; $i < 10; $i++) {
3737
socket_sendto($receiverSocket, 'Pong!', 5, 0, $from);
3838
echo "Responded to sender with 'Pong!'\n";
3939

40-
@socket_close($receiverSocket);
40+
socket_close($receiverSocket);
4141
unlink($receiverSocketPath);
42-
@socket_close($senderSocket);
42+
socket_close($senderSocket);
4343
unlink($senderSocketPath);
4444
}
4545
--EXPECT--

ext/sockets/tests/ipv4loop.phpt

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,3 @@ IPv4 Loopback test
5151
--EXPECT--
5252
string(10) "ABCdef123
5353
"
54-
55-
Deprecated: Function socket_close() is deprecated
56-
57-
Deprecated: Function socket_close() is deprecated
58-
59-
Deprecated: Function socket_close() is deprecated

ext/sockets/tests/ipv6loop.phpt

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,3 @@ IPv6 Loopback test
5555
--EXPECT--
5656
string(10) "ABCdef123
5757
"
58-
59-
Deprecated: Function socket_close() is deprecated
60-
61-
Deprecated: Function socket_close() is deprecated
62-
63-
Deprecated: Function socket_close() is deprecated

ext/sockets/tests/socket_cmsg_credentials.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ bool(true)
6767
int(5)
6868
Array
6969
(
70-
[name] =>
70+
[name] =>
7171
[control] => Array
7272
(
7373
[0] => Array

ext/sockets/tests/socket_cmsg_rights.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ if ($data["control"]) {
6161
if ($control["level"] == SOL_SOCKET &&
6262
$control["type"] == SCM_RIGHTS) {
6363
foreach ($control["data"] as $resource) {
64-
if (!is_object($resource)) {
64+
if (!is_resource($resource)) {
6565
echo "FAIL RES\n";
6666
var_dump($data);
6767
exit;

ext/sockets/tests/socket_connect_params.phpt

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,21 @@ socket_getsockname($s_c, $addr, $port);
1717

1818
// wrong parameter count
1919
try {
20-
$s_w = socket_connect($s_c);
20+
socket_connect($s_c);
2121
} catch (\ArgumentCountError $e) {
2222
echo $e->getMessage() . \PHP_EOL;
2323
}
2424
try {
25-
$s_w = socket_connect($s_c, '0.0.0.0');
25+
socket_connect($s_c, '0.0.0.0');
2626
} catch (\ValueError $e) {
2727
echo $e->getMessage() . \PHP_EOL;
2828
}
2929
$s_w = socket_connect($s_c, '0.0.0.0', $port);
3030

3131
socket_close($s_c);
32-
3332
?>
3433
--EXPECTF--
3534
socket_connect() expects at least 2 parameters, 1 given
36-
socket_connect(): Argument #3 ($port) must be specified for the AF_INET socket type
35+
socket_connect(): Argument #3 ($port) cannot be null when the socket type is AF_INET
3736

3837
Warning: socket_connect(): unable to connect [%i]: %a in %s on line %d

ext/sockets/tests/socket_export_stream-1.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ var_dump($stream);
2020
socket_write($s1, "test message");
2121
socket_close($s1);
2222

23-
var_dump(stream_get_contents($stream));
23+
//var_dump(stream_get_contents($stream));
2424
--EXPECTF--
2525
resource(%d) of type (stream)
2626
string(12) "test message"

ext/sockets/tests/socket_export_stream-2.phpt

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,31 +9,27 @@ if (!extension_loaded('sockets')) {
99
<?php
1010

1111
try {
12-
var_dump(socket_export_stream(fopen(__FILE__, "rb")));
12+
socket_export_stream(fopen(__FILE__, "rb"));
1313
} catch (TypeError $e) {
1414
echo $e->getMessage(), "\n";
1515
}
1616
try {
17-
var_dump(socket_export_stream(stream_socket_server("udp://127.0.0.1:0", $errno, $errstr, STREAM_SERVER_BIND)));
17+
socket_export_stream(stream_socket_server("udp://127.0.0.1:0", $errno, $errstr, STREAM_SERVER_BIND));
1818
} catch (TypeError $e) {
1919
echo $e->getMessage(), "\n";
2020
}
2121
$s = socket_create(AF_INET, SOCK_DGRAM, SOL_UDP);
2222
var_dump($s);
2323
socket_close($s);
24-
try {
25-
var_dump(socket_export_stream($s));
26-
} catch (TypeError $e) {
27-
echo $e->getMessage(), "\n";
28-
}
2924

25+
var_dump(socket_export_stream($s));
3026

3127
echo "Done.";
3228
?>
3329
--EXPECTF--
34-
socket_export_stream(): supplied resource is not a valid Socket resource
35-
socket_export_stream(): supplied resource is not a valid Socket resource
30+
socket_export_stream(): Argument #1 ($socket) must be of type Socket, resource given
31+
socket_export_stream(): Argument #1 ($socket) must be of type Socket, resource given
3632
object(Socket)#%d (0) {
3733
}
38-
socket_export_stream(): supplied resource is not a valid Socket resource
34+
resource(7) of type (stream)
3935
Done.

ext/sockets/tests/socket_export_stream-4.phpt

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,11 @@ Warning: socket_get_option(): Unable to retrieve socket option [%d]: %s in %s on
105105
close socket
106106
stream_set_blocking stream_set_blocking(): supplied resource is not a valid stream resource
107107

108-
socket_set_block socket_set_block(): supplied resource is not a valid Socket resource
108+
socket_set_block
109+
Warning: socket_set_block(): unable to set blocking mode [9]: Bad file descriptor in %s on line %d
109110

110-
socket_get_option socket_get_option(): supplied resource is not a valid Socket resource
111+
socket_get_option
112+
Warning: socket_get_option(): Unable to retrieve socket option [9]: Bad file descriptor in %s on line %d
111113

112114

113115
Done.

ext/sockets/tests/socket_import_stream-2.phpt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,15 @@ if (!extension_loaded('sockets')) {
1010

1111
var_dump(socket_import_stream(fopen(__FILE__, "rb")));
1212
try {
13-
var_dump(socket_import_stream(socket_create(AF_INET, SOCK_DGRAM, SOL_UDP)));
13+
socket_import_stream(socket_create(AF_INET, SOCK_DGRAM, SOL_UDP));
1414
} catch (TypeError $e) {
1515
echo $e->getMessage(), "\n";
1616
}
1717
$s = stream_socket_server("udp://127.0.0.1:0", $errno, $errstr, STREAM_SERVER_BIND);
1818
var_dump($s);
1919
var_dump(fclose($s));
2020
try {
21-
var_dump(socket_import_stream($s));
21+
socket_import_stream($s);
2222
} catch (TypeError $e) {
2323
echo $e->getMessage(), "\n";
2424
}
@@ -28,7 +28,7 @@ echo "Done.";
2828
--EXPECTF--
2929
Warning: socket_import_stream(): Cannot represent a stream of type STDIO as a Socket Descriptor in %s on line %d
3030
bool(false)
31-
socket_import_stream(): supplied resource is not a valid stream resource
31+
socket_import_stream(): Argument #1 ($stream) must be of type resource, Socket given
3232
resource(%d) of type (stream)
3333
bool(true)
3434
socket_import_stream(): supplied resource is not a valid stream resource

ext/sockets/tests/socket_import_stream-4.phpt

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,9 +100,11 @@ Warning: socket_get_option(): Unable to retrieve socket option [%d]: %s in %s on
100100
close socket
101101
stream_set_blocking stream_set_blocking(): supplied resource is not a valid stream resource
102102

103-
socket_set_block socket_set_block(): supplied resource is not a valid Socket resource
103+
socket_set_block
104+
Warning: socket_set_block(): unable to set blocking mode [9]: Bad file descriptor in %s on line %d
104105

105-
socket_get_option socket_get_option(): supplied resource is not a valid Socket resource
106+
socket_get_option
107+
Warning: socket_get_option(): Unable to retrieve socket option [9]: Bad file descriptor in %s on line %d
106108

107109

108110
Done.

0 commit comments

Comments
 (0)