Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore: Drop $loop usages in favor of Loop::get() #221

Draft
wants to merge 2 commits into
base: 3.x
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@ jobs:
- 7.3
- 7.2
- 7.1
- 7.0
- 5.6
- 5.5
- 5.4
- 5.3
steps:
- uses: actions/checkout@v4
- uses: shivammathur/setup-php@v2
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
}
],
"require": {
"php": ">=5.3.0",
"php": ">=7.1",
"react/cache": "^1.0 || ^0.6 || ^0.5",
"react/event-loop": "^1.2",
"react/promise": "^3.0 || ^2.7 || ^1.2.1"
Expand Down
12 changes: 1 addition & 11 deletions src/Model/Message.php
Original file line number Diff line number Diff line change
Expand Up @@ -126,23 +126,13 @@ public static function createResponseWithAnswersForQuery(Query $query, array $an
* DNS response messages can not guess the message ID to avoid possible
* cache poisoning attacks.
*
* The `random_int()` function is only available on PHP 7+ or when
* https://github.com/paragonie/random_compat is installed. As such, using
* the latest supported PHP version is highly recommended. This currently
* falls back to a less secure random number generator on older PHP versions
* in the hope that this system is properly protected against outside
* attackers, for example by using one of the common local DNS proxy stubs.
*
* @return int
* @see self::getId()
* @codeCoverageIgnore
*/
private static function generateId()
{
if (function_exists('random_int')) {
return random_int(0, 0xffff);
}
return mt_rand(0, 0xffff);
return \random_int(0, 0xffff);
}

/**
Expand Down
22 changes: 9 additions & 13 deletions src/Query/TcpTransportExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
use React\Dns\Protocol\BinaryDumper;
use React\Dns\Protocol\Parser;
use React\EventLoop\Loop;
use React\EventLoop\LoopInterface;
use React\Promise\Deferred;

/**
Expand Down Expand Up @@ -78,7 +77,6 @@
class TcpTransportExecutor implements ExecutorInterface
{
private $nameserver;
private $loop;
private $parser;
private $dumper;

Expand Down Expand Up @@ -132,9 +130,8 @@ class TcpTransportExecutor implements ExecutorInterface

/**
* @param string $nameserver
* @param ?LoopInterface $loop
*/
public function __construct($nameserver, LoopInterface $loop = null)
public function __construct($nameserver)
{
if (\strpos($nameserver, '[') === false && \substr_count($nameserver, ':') >= 2 && \strpos($nameserver, '://') === false) {
// several colons, but not enclosed in square brackets => enclose IPv6 address in square brackets
Expand All @@ -147,7 +144,6 @@ public function __construct($nameserver, LoopInterface $loop = null)
}

$this->nameserver = 'tcp://' . $parts['host'] . ':' . (isset($parts['port']) ? $parts['port'] : 53);
$this->loop = $loop ?: Loop::get();
$this->parser = new Parser();
$this->dumper = new BinaryDumper();
}
Expand Down Expand Up @@ -190,15 +186,15 @@ public function query(Query $query)
}

if ($this->idleTimer !== null) {
$this->loop->cancelTimer($this->idleTimer);
Loop::get()->cancelTimer($this->idleTimer);
$this->idleTimer = null;
}

// wait for socket to become writable to actually write out data
$this->writeBuffer .= $queryData;
if (!$this->writePending) {
$this->writePending = true;
$this->loop->addWriteStream($this->socket, array($this, 'handleWritable'));
Loop::get()->addWriteStream($this->socket, array($this, 'handleWritable'));
}

$names =& $this->names;
Expand Down Expand Up @@ -243,7 +239,7 @@ public function handleWritable()
}

$this->readPending = true;
$this->loop->addReadStream($this->socket, array($this, 'handleRead'));
Loop::get()->addReadStream($this->socket, array($this, 'handleRead'));
}

$errno = 0;
Expand Down Expand Up @@ -271,7 +267,7 @@ public function handleWritable()
if (isset($this->writeBuffer[$written])) {
$this->writeBuffer = \substr($this->writeBuffer, $written);
} else {
$this->loop->removeWriteStream($this->socket);
Loop::get()->removeWriteStream($this->socket);
$this->writePending = false;
$this->writeBuffer = '';
}
Expand Down Expand Up @@ -336,18 +332,18 @@ public function closeError($reason, $code = 0)
{
$this->readBuffer = '';
if ($this->readPending) {
$this->loop->removeReadStream($this->socket);
Loop::get()->removeReadStream($this->socket);
$this->readPending = false;
}

$this->writeBuffer = '';
if ($this->writePending) {
$this->loop->removeWriteStream($this->socket);
Loop::get()->removeWriteStream($this->socket);
$this->writePending = false;
}

if ($this->idleTimer !== null) {
$this->loop->cancelTimer($this->idleTimer);
Loop::get()->cancelTimer($this->idleTimer);
$this->idleTimer = null;
}

Expand All @@ -370,7 +366,7 @@ public function checkIdle()
{
if ($this->idleTimer === null && !$this->names) {
$that = $this;
$this->idleTimer = $this->loop->addTimer($this->idlePeriod, function () use ($that) {
$this->idleTimer = Loop::get()->addTimer($this->idlePeriod, function () use ($that) {
$that->closeError('Idle timeout');
});
}
Expand Down
18 changes: 7 additions & 11 deletions src/Query/TimeoutExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,39 +3,35 @@
namespace React\Dns\Query;

use React\EventLoop\Loop;
use React\EventLoop\LoopInterface;
use React\Promise\Promise;

final class TimeoutExecutor implements ExecutorInterface
{
private $executor;
private $loop;
private $timeout;

public function __construct(ExecutorInterface $executor, $timeout, LoopInterface $loop = null)
public function __construct(ExecutorInterface $executor, $timeout)
{
$this->executor = $executor;
$this->loop = $loop ?: Loop::get();
$this->timeout = $timeout;
}

public function query(Query $query)
{
$promise = $this->executor->query($query);

$loop = $this->loop;
$time = $this->timeout;
return new Promise(function ($resolve, $reject) use ($loop, $time, $promise, $query) {
return new Promise(function ($resolve, $reject) use ($time, $promise, $query) {
$timer = null;
$promise = $promise->then(function ($v) use (&$timer, $loop, $resolve) {
$promise = $promise->then(function ($v) use (&$timer, $resolve) {
if ($timer) {
$loop->cancelTimer($timer);
Loop::get()->cancelTimer($timer);
}
$timer = false;
$resolve($v);
}, function ($v) use (&$timer, $loop, $reject) {
}, function ($v) use (&$timer, $reject) {
if ($timer) {
$loop->cancelTimer($timer);
Loop::get()->cancelTimer($timer);
}
$timer = false;
$reject($v);
Expand All @@ -47,7 +43,7 @@ public function query(Query $query)
}

// start timeout timer which will cancel the pending promise
$timer = $loop->addTimer($time, function () use ($time, &$promise, $reject, $query) {
$timer = Loop::get()->addTimer($time, function () use ($time, &$promise, $reject, $query) {
$reject(new TimeoutException(
'DNS query for ' . $query->describe() . ' timed out'
));
Expand Down
17 changes: 6 additions & 11 deletions src/Query/UdpTransportExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
use React\Dns\Protocol\BinaryDumper;
use React\Dns\Protocol\Parser;
use React\EventLoop\Loop;
use React\EventLoop\LoopInterface;
use React\Promise\Deferred;

/**
Expand Down Expand Up @@ -83,7 +82,6 @@
final class UdpTransportExecutor implements ExecutorInterface
{
private $nameserver;
private $loop;
private $parser;
private $dumper;

Expand All @@ -95,10 +93,9 @@ final class UdpTransportExecutor implements ExecutorInterface
private $maxPacketSize = 512;

/**
* @param string $nameserver
* @param ?LoopInterface $loop
* @param string $nameserver
*/
public function __construct($nameserver, LoopInterface $loop = null)
public function __construct($nameserver)
{
if (\strpos($nameserver, '[') === false && \substr_count($nameserver, ':') >= 2 && \strpos($nameserver, '://') === false) {
// several colons, but not enclosed in square brackets => enclose IPv6 address in square brackets
Expand All @@ -111,7 +108,6 @@ public function __construct($nameserver, LoopInterface $loop = null)
}

$this->nameserver = 'udp://' . $parts['host'] . ':' . (isset($parts['port']) ? $parts['port'] : 53);
$this->loop = $loop ?: Loop::get();
$this->parser = new Parser();
$this->dumper = new BinaryDumper();
}
Expand Down Expand Up @@ -163,10 +159,9 @@ public function query(Query $query)
));
}

$loop = $this->loop;
$deferred = new Deferred(function () use ($loop, $socket, $query) {
$deferred = new Deferred(function () use ($socket, $query) {
// cancellation should remove socket from loop and close socket
$loop->removeReadStream($socket);
Loop::get()->removeReadStream($socket);
\fclose($socket);

throw new CancellationException('DNS query for ' . $query->describe() . ' has been cancelled');
Expand All @@ -175,7 +170,7 @@ public function query(Query $query)
$max = $this->maxPacketSize;
$parser = $this->parser;
$nameserver = $this->nameserver;
$loop->addReadStream($socket, function ($socket) use ($loop, $deferred, $query, $parser, $request, $max, $nameserver) {
Loop::get()->addReadStream($socket, function ($socket) use ($deferred, $query, $parser, $request, $max, $nameserver) {
// 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, $max);
Expand All @@ -198,7 +193,7 @@ public function query(Query $query)
}

// we only react to the first valid message, so remove socket from loop and close
$loop->removeReadStream($socket);
Loop::get()->removeReadStream($socket);
\fclose($socket);

if ($response->tc) {
Expand Down
Loading
Loading