From 6a894465c3f974fb26f153f79dab0d4909e42bbc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Christian=20L=C3=BCck?= <christian@clue.engineering>
Date: Mon, 30 Dec 2019 20:59:03 +0100
Subject: [PATCH] Fix reporting refused connections with StreamSelectLoop on
 Windows

We do not usually use or expose the `exceptfds` parameter passed to the
underlying `select`. However, Windows does not report failed connection
attempts in `writefds` passed to `select` like most other platforms.
Instead, it uses `writefds` only for successful connection attempts and
`exceptfds` for failed connection attempts. See also
https://docs.microsoft.com/de-de/windows/win32/api/winsock2/nf-winsock2-select

We work around this by adding all sockets that look like a pending
connection attempt to `exceptfds` automatically on Windows and merge it
back later. This ensures the public API matches other loop
implementations across all platforms (see also test suite or rather test
matrix). Lacking better APIs, every write-only socket that has not yet
read any data is assumed to be in a pending connection attempt state.
---
 src/StreamSelectLoop.php   | 22 +++++++++++++++++++++-
 tests/AbstractLoopTest.php |  7 +------
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/src/StreamSelectLoop.php b/src/StreamSelectLoop.php
index 3362d3e5..4426844a 100644
--- a/src/StreamSelectLoop.php
+++ b/src/StreamSelectLoop.php
@@ -269,10 +269,30 @@ private function waitForStreamActivity($timeout)
     private function streamSelect(array &$read, array &$write, $timeout)
     {
         if ($read || $write) {
+            // We do not usually use or expose the `exceptfds` parameter passed to the underlying `select`.
+            // However, Windows does not report failed connection attempts in `writefds` passed to `select` like most other platforms.
+            // Instead, it uses `writefds` only for successful connection attempts and `exceptfds` for failed connection attempts.
+            // We work around this by adding all sockets that look like a pending connection attempt to `exceptfds` automatically on Windows and merge it back later.
+            // This ensures the public API matches other loop implementations across all platforms (see also test suite or rather test matrix).
+            // Lacking better APIs, every write-only socket that has not yet read any data is assumed to be in a pending connection attempt state.
+            // @link https://docs.microsoft.com/de-de/windows/win32/api/winsock2/nf-winsock2-select
             $except = null;
+            if (\DIRECTORY_SEPARATOR === '\\') {
+                $except = array();
+                foreach ($write as $key => $socket) {
+                    if (!isset($read[$key]) && @\ftell($socket) === 0) {
+                        $except[$key] = $socket;
+                    }
+                }
+            }
 
             // suppress warnings that occur, when stream_select is interrupted by a signal
-            return @\stream_select($read, $write, $except, $timeout === null ? null : 0, $timeout);
+            $ret = @\stream_select($read, $write, $except, $timeout === null ? null : 0, $timeout);
+
+            if ($except) {
+                $write = \array_merge($write, $except);
+            }
+            return $ret;
         }
 
         if ($timeout > 0) {
diff --git a/tests/AbstractLoopTest.php b/tests/AbstractLoopTest.php
index 44157d2c..9b55f959 100644
--- a/tests/AbstractLoopTest.php
+++ b/tests/AbstractLoopTest.php
@@ -111,16 +111,11 @@ public function testAddWriteStreamTriggersWhenSocketConnectionSucceeds()
 
     public function testAddWriteStreamTriggersWhenSocketConnectionRefused()
     {
-        // @link https://github.com/reactphp/event-loop/issues/206
-        if ($this->loop instanceof StreamSelectLoop && DIRECTORY_SEPARATOR === '\\') {
-            $this->markTestIncomplete('StreamSelectLoop does not currently support detecting connection refused errors on Windows');
-        }
-
         // first verify the operating system actually refuses the connection and no firewall is in place
         // use higher timeout because Windows retires multiple times and has a noticeable delay
         // @link https://stackoverflow.com/questions/19440364/why-do-failed-attempts-of-socket-connect-take-1-sec-on-windows
         $errno = $errstr = null;
-        if (@stream_socket_client('127.0.0.1:1', $errno, $errstr, 10.0) !== false || $errno !== SOCKET_ECONNREFUSED) {
+        if (@stream_socket_client('127.0.0.1:1', $errno, $errstr, 10.0) !== false || (defined('SOCKET_ECONNREFUSED') && $errno !== SOCKET_ECONNREFUSED)) {
             $this->markTestSkipped('Expected host to refuse connection, but got error ' . $errno . ': ' . $errstr);
         }