diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cf214c83..4fc766a0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,6 +33,19 @@ jobs: - run: vendor/bin/phpunit --coverage-text -c phpunit.xml.legacy if: ${{ matrix.php < 7.3 }} + PHPUnit-macOS: + name: PHPUnit (macOS) + runs-on: macos-10.15 + continue-on-error: true + steps: + - uses: actions/checkout@v2 + - uses: shivammathur/setup-php@v2 + with: + php-version: 8.0 + coverage: xdebug + - run: composer install + - run: vendor/bin/phpunit --coverage-text + PHPUnit-hhvm: name: PHPUnit (HHVM) runs-on: ubuntu-18.04 diff --git a/src/Query/UdpTransportExecutor.php b/src/Query/UdpTransportExecutor.php index 62ac2183..ff3169db 100644 --- a/src/Query/UdpTransportExecutor.php +++ b/src/Query/UdpTransportExecutor.php @@ -92,6 +92,13 @@ final class UdpTransportExecutor implements ExecutorInterface private $parser; private $dumper; + /** + * maximum UDP packet size to send and receive + * + * @var int + */ + private $maxPacketSize = 512; + /** * @param string $nameserver * @param LoopInterface $loop @@ -119,7 +126,7 @@ public function query(Query $query) $request = Message::createRequestForQuery($query); $queryData = $this->dumper->toBinary($request); - if (isset($queryData[512])) { + if (isset($queryData[$this->maxPacketSize])) { return \React\Promise\reject(new \RuntimeException( 'DNS query for ' . $query->name . ' failed: Query too large for UDP transport', \defined('SOCKET_EMSGSIZE') ? \SOCKET_EMSGSIZE : 90 @@ -137,7 +144,20 @@ public function query(Query $query) // set socket to non-blocking and immediately try to send (fill write buffer) \stream_set_blocking($socket, false); - \fwrite($socket, $queryData); + $written = @\fwrite($socket, $queryData); + + if ($written !== \strlen($queryData)) { + // Write may potentially fail, but most common errors are already caught by connection check above. + // Among others, macOS is known to report here when trying to send to broadcast address. + // This can also be reproduced by writing data exceeding `stream_set_chunk_size()` to a server refusing UDP data. + // fwrite(): send of 8192 bytes failed with errno=111 Connection refused + $error = \error_get_last(); + \preg_match('/errno=(\d+) (.+)/', $error['message'], $m); + return \React\Promise\reject(new \RuntimeException( + 'DNS query for ' . $query->name . ' failed: Unable to send query to DNS server (' . (isset($m[2]) ? $m[2] : $error['message']) . ')', + isset($m[1]) ? (int) $m[1] : 0 + )); + } $loop = $this->loop; $deferred = new Deferred(function () use ($loop, $socket, $query) { @@ -148,11 +168,15 @@ public function query(Query $query) throw new CancellationException('DNS query for ' . $query->name . ' has been cancelled'); }); + $max = $this->maxPacketSize; $parser = $this->parser; - $loop->addReadStream($socket, function ($socket) use ($loop, $deferred, $query, $parser, $request) { + $loop->addReadStream($socket, function ($socket) use ($loop, $deferred, $query, $parser, $request, $max) { // try to read a single data packet from the DNS server // ignoring any errors, this is uses UDP packets and not a stream of data - $data = @\fread($socket, 512); + $data = @\fread($socket, $max); + if ($data === false) { + return; + } try { $response = $parser->parseMessage($data); diff --git a/tests/Query/UdpTransportExecutorTest.php b/tests/Query/UdpTransportExecutorTest.php index 78a85bb8..5a202ace 100644 --- a/tests/Query/UdpTransportExecutorTest.php +++ b/tests/Query/UdpTransportExecutorTest.php @@ -120,7 +120,75 @@ public function testQueryRejectsIfServerConnectionFails() $promise = $executor->query($query); $this->assertInstanceOf('React\Promise\PromiseInterface', $promise); - $promise->then(null, $this->expectCallableOnce()); + + $exception = null; + $promise->then(null, function ($reason) use (&$exception) { + $exception = $reason; + }); + + // PHP (Failed to parse address "///") differs from HHVM (Name or service not known) + $this->setExpectedException('RuntimeException', 'Unable to connect to DNS server'); + throw $exception; + } + + public function testQueryRejectsIfSendToServerFailsAfterConnection() + { + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->never())->method('addReadStream'); + + $executor = new UdpTransportExecutor('0.0.0.0', $loop); + + // increase hard-coded maximum packet size to allow sending excessive data + $ref = new \ReflectionProperty($executor, 'maxPacketSize'); + $ref->setAccessible(true); + $ref->setValue($executor, PHP_INT_MAX); + + $query = new Query(str_repeat('a.', 100000) . '.example', Message::TYPE_A, Message::CLASS_IN); + $promise = $executor->query($query); + + $this->assertInstanceOf('React\Promise\PromiseInterface', $promise); + + $exception = null; + $promise->then(null, function ($reason) use (&$exception) { + $exception = $reason; + }); + + // ECONNREFUSED( Connection refused) on Linux, EMSGSIZE (Message too long) on macOS + $this->setExpectedException('RuntimeException', 'Unable to send query to DNS server'); + throw $exception; + } + + public function testQueryKeepsPendingIfReadFailsBecauseServerRefusesConnection() + { + $socket = null; + $callback = null; + $loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock(); + $loop->expects($this->once())->method('addReadStream')->with($this->callback(function ($ref) use (&$socket) { + $socket = $ref; + return true; + }), $this->callback(function ($ref) use (&$callback) { + $callback = $ref; + return true; + })); + + $executor = new UdpTransportExecutor('0.0.0.0', $loop); + + $query = new Query('reactphp.org', Message::TYPE_A, Message::CLASS_IN); + $promise = $executor->query($query); + + $this->assertNotNull($socket); + $callback($socket); + + $this->assertInstanceOf('React\Promise\PromiseInterface', $promise); + + $pending = true; + $promise->then(function () use (&$pending) { + $pending = false; + }, function () use (&$pending) { + $pending = false; + }); + + $this->assertTrue($pending); } /** @@ -142,27 +210,6 @@ public function testQueryRejectsOnCancellation() $promise->then(null, $this->expectCallableOnce()); } - public function testQueryKeepsPendingIfServerRejectsNetworkPacket() - { - $loop = Factory::create(); - - $executor = new UdpTransportExecutor('127.0.0.1:1', $loop); - - $query = new Query('google.com', Message::TYPE_A, Message::CLASS_IN); - - $wait = true; - $promise = $executor->query($query)->then( - null, - function ($e) use (&$wait) { - $wait = false; - throw $e; - } - ); - - \Clue\React\Block\sleep(0.2, $loop); - $this->assertTrue($wait); - } - public function testQueryKeepsPendingIfServerSendsInvalidMessage() { $loop = Factory::create();