php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #78038 Socket_select fails when resource array contains references
Submitted: 2019-05-20 09:27 UTC Modified: 2019-05-23 09:16 UTC
From: ogdg at protonmail dot com Assigned: nikic (profile)
Status: Closed Package: Sockets related
PHP Version: 7.3.5 OS: Fedora 30
Private report: No CVE-ID: None
 [2019-05-20 09:27 UTC] ogdg at protonmail dot com
Description:
------------
This bug occurs in PHP CLI 7.3.5!

When socket connection resources are indexed in an array by non-sequential numeric keys, socket_select throws a warning and does not actually accept the array even though it contains valid socket resources. By manually re-indexing the array to contain sequentially numbered keys, the problem disappears.

This could a problem with large key values, or non sequential keys, or both. The problem did not exist in previous versions of PHP, and it is not valid for socket_select to require arrays that are sequentially numbered, as there are reasons to index connection arrays by process ID or some other meaningful number.

Test script:
---------------
# Assume each $connN variable is a valid socket resource
# Note that array keys are INT and not string type

$connections = array(1235 => $conn1, 2084 => $conn2, 3509 => $conn3);

$w = $e = NULL;
$r = $connections;

$check = socket_select($r, $w, $e, 0);

# The ABOVE code fails with bogus warnings in PHP 7.3.5
# The BELOW code works
$w = $e = NULL;

foreach($connections as $v) $r[] = $v;

$check = socket_select($r, $w, $e, 0);

Expected result:
----------------
Expect socket_select to work as intended and return a value for $check and a modified connection resource array if appropriate.

Actual result:
--------------
Result is two warnings:

socket_select(): supplied argument is not a valid Socket resource
socket_select(): no resource arrays were passed to select

Previous versions of PHP, including PHP 7 did not have this problem.

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2019-05-20 10:00 UTC] daverandom@php.net
-Status: Open +Status: Feedback
 [2019-05-20 10:00 UTC] daverandom@php.net
Thank you for this bug report. To properly diagnose the problem, we
need a short but complete example script to be able to reproduce
this bug ourselves. 

A proper reproducing script starts with <?php and ends with ?>,
is max. 10-20 lines long and does not require any external 
resources such as databases, etc. If the script requires a 
database to demonstrate the issue, please make sure it creates 
all necessary tables, stored procedures etc.

Please avoid embedding huge scripts into the report.

I cannot reproduce this in 7.3.5 or any other version. Additionally, none of the relevant code in php-src has been changed for several years in 7.3[1] or master[2].

Please recheck your code, var_dump($r) and verify that it contains at least one valid socket resource, and if so please provide a complete reproducing script.

There is a lot of code in the wild which utilises a pattern roughly like below - it's likely that if it were broken then a lot of people would be shouting about it :-)

$socket = socket_create(/* stuff */); // ...set up the socket
$sockets[(int)$socket] = $socket; // Add socket to array indexed by resource ID

$w = $e = NULL;
$r = $sockets;
$check = socket_select($r, $w, $e, 0);

[1] https://github.com/php/php-src/blame/PHP-7.3/ext/sockets/sockets.c#L915-L977
[2] https://github.com/php/php-src/blame/master/ext/sockets/sockets.c#L921-L983
 [2019-05-22 07:44 UTC] nikic@php.net
One recent change comes to mind here: https://github.com/php/php-src/commit/bdac9ef10d90997f9576564dde693cbef6594142#diff-9df410db21c230cd2ec04a4c95ccd20f

However, it is in stream_select(), not socket_select().

When you say "previous versions" do you mean 7.3.4 or some older version?
 [2019-05-23 07:33 UTC] ogdg at protonmail dot com
-Status: Feedback +Status: Open
 [2019-05-23 07:33 UTC] ogdg at protonmail dot com
@nikic
The latest version I was using that did not have this issue was 7.2, and it never happened in any of the version 5 series. I am using the version that is included with Fedora 30 standard repos without any modifications or custom extensions. Just the default install.

@daverandom
I made a short script to simulate what my application does but the error does not occur. The actual application is using AF_UNIX type sockets for IPC. I do not have the free time to reproduce a full-blown script of this sort right now, but the bug is still present.

The application where this issue happens has the basic structure like this:
1) Main process creates AF_UNIX socket
2) Sub process launched, create their own AF_UNIX socket and connect to the main process socket
3) Main process does a socket_accept for new IPC connections
4) Main process uses a this to get PID of child process: 
$pid = socket_get_option($conn, SOL_SOCKET, 17);
$pid = (int)$pid;

5) The main process keeps a copy of the resource in an array indexed by PIDs.
$socketArray['ipc'][$pid];

6a) Problem happens when I take that array and do this:

$e = $w = NULL;
$r = $socketArray['ipc'];
socket_select($r, $w, $e, 0);

6b) To fix this, I had to manually loop through the resource array:

foreach($socketArray['ipc'] as $v) $r[] = $v;
...

So in this case, 6a fails with the warnings listed in the original report, while 6b works fine. In the actual application, the socket array is passed to a custom function by reference...but I did try passing it directly and the result was the same in 6a.

Both print_r and var_dump show that the array is being correctly assigned in 6a, and that $r does indeed contain resources, which is why the fix in 6b works. I can't really tell what the problem is, but it's definitely there and seems to appear in very specific conditions.

My first guess was that the $pid wasn't being cast to INT and was being treated as a string, so I cast it to INT and that didn't help.

The other thing I thought could be a problem is the large numeric values for the $pid, meaning that for some reason socket_select chokes when the index values of any connection array are above a certain number, but I couldn't reproduce this in a standalone script just using large index values for socket_select...also, the standalone script is not using pcntl_fork as the actual application does.
 [2019-05-23 07:58 UTC] nikic@php.net
> So in this case, 6a fails with the warnings listed in the original report, while 6b works fine. In the actual application, the socket array is passed to a custom function by reference...but I did try passing it directly and the result was the same in 6a.

From looking at the code, I suspect that the problem does have something to do with references being introduced somewhere. At least we're missing a DEREF in here:

https://github.com/php/php-src/blob/2e4686b5667e9ccbfbfab10452c42f5b818fcfdb/ext/sockets/sockets.c#L906
 [2019-05-23 09:10 UTC] nikic@php.net
-Summary: Socket_Select fails when resource array contains non-sequential keys +Summary: Socket_select fails when resource array contains references
 [2019-05-23 09:13 UTC] nikic@php.net
Automatic comment on behalf of nikita.ppv@gmail.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=9a74b23297167ca7956fd21428c8b987e7d99aa5
Log: Fixed bug #78038 socket_select with references
 [2019-05-23 09:13 UTC] nikic@php.net
-Status: Open +Status: Closed
 [2019-05-23 09:16 UTC] nikic@php.net
-Assigned To: +Assigned To: nikic
 [2019-05-23 09:16 UTC] nikic@php.net
I've fixed the issue with the reference handling, and think it's pretty likely that this is what's causing the problem for you. There's likely some singleton references lying around in your socket array that won't even show up in var_dump (e.g. if you do a by-ref iteration over the sockets or similar).

If the issue persists after this fix, let's take another look.
 
PHP Copyright © 2001-2019 The PHP Group
All rights reserved.
Last updated: Mon Oct 21 12:01:26 2019 UTC