Skip to content

Optionally accept explicit udp:// and tcp:// scheme in Factory #146

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.

Sign up for GitHub

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

Merged
WyriHaximus merged 1 commit into from
Aug 10, 2019
Merged

Optionally accept explicit udp:// and tcp:// scheme in Factory #146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions src/Query/TcpTransportExecutor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,13 @@ class TcpTransportExecutor implements ExecutorInterface
*/
public function __construct($nameserver, LoopInterface $loop)
{
if (\strpos($nameserver, '[') === false && \substr_count($nameserver, ':') >= 2) {
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
$nameserver = '[' . $nameserver . ']';
}

$parts = \parse_url('tcp://' . $nameserver);
if (!isset($parts['scheme'], $parts['host']) || !\filter_var(\trim($parts['host'], '[]'), \FILTER_VALIDATE_IP)) {
$parts = \parse_url((\strpos($nameserver, '://') === false ? 'tcp://' : '') . $nameserver);
if (!isset($parts['scheme'], $parts['host']) || $parts['scheme'] !== 'tcp' || !\filter_var(\trim($parts['host'], '[]'), \FILTER_VALIDATE_IP)) {
throw new \InvalidArgumentException('Invalid nameserver address given');
}

Expand Down
6 changes: 3 additions & 3 deletions src/Query/UdpTransportExecutor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,13 @@ final class UdpTransportExecutor implements ExecutorInterface
*/
public function __construct($nameserver, LoopInterface $loop)
{
if (strpos($nameserver, '[') === false && substr_count($nameserver, ':') >= 2) {
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
$nameserver = '[' . $nameserver . ']';
}

$parts = parse_url('udp://' . $nameserver);
if (!isset($parts['scheme'], $parts['host']) || $parts['scheme'] !== 'udp') {
$parts = \parse_url((\strpos($nameserver, '://') === false ? 'udp://' : '') . $nameserver);
if (!isset($parts['scheme'], $parts['host']) || $parts['scheme'] !== 'udp' || !\filter_var(\trim($parts['host'], '[]'), \FILTER_VALIDATE_IP)) {
throw new \InvalidArgumentException('Invalid nameserver address given');
}

Expand Down
43 changes: 25 additions & 18 deletions src/Resolver/Factory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use React\Dns\Query\ExecutorInterface;
use React\Dns\Query\HostsFileExecutor;
use React\Dns\Query\RetryExecutor;
use React\Dns\Query\TcpTransportExecutor;
use React\Dns\Query\TimeoutExecutor;
use React\Dns\Query\UdpTransportExecutor;
use React\EventLoop\LoopInterface;
Expand All @@ -23,7 +24,7 @@ final class Factory
*/
public function create($nameserver, LoopInterface $loop)
{
$executor = $this->decorateHostsFileExecutor($this->createRetryExecutor($nameserver, $loop));
$executor = $this->decorateHostsFileExecutor($this->createExecutor($nameserver, $loop));

return new Resolver($executor);
}
Expand All @@ -41,7 +42,9 @@ public function createCached($nameserver, LoopInterface $loop, CacheInterface $c
$cache = new ArrayCache(256);
}

$executor = $this->decorateHostsFileExecutor($this->createCachedExecutor($nameserver, $loop, $cache));
$executor = $this->createExecutor($nameserver, $loop);
$executor = new CachingExecutor($executor, $cache);
$executor = $this->decorateHostsFileExecutor($executor);

return new Resolver($executor);
}
Expand Down Expand Up @@ -78,23 +81,27 @@ private function decorateHostsFileExecutor(ExecutorInterface $executor)

private function createExecutor($nameserver, LoopInterface $loop)
{
return new TimeoutExecutor(
new UdpTransportExecutor(
$nameserver,
$loop
),
5.0,
$loop
);
}
$parts = \parse_url($nameserver);

private function createRetryExecutor($namserver, LoopInterface $loop)
{
return new CoopExecutor(new RetryExecutor($this->createExecutor($namserver, $loop)));
}
if (isset($parts['scheme']) && $parts['scheme'] === 'tcp') {
$executor = new TimeoutExecutor(
new TcpTransportExecutor($nameserver, $loop),
5.0,
$loop
);
} else {
$executor = new RetryExecutor(
new TimeoutExecutor(
new UdpTransportExecutor(
$nameserver,
$loop
),
5.0,
$loop
)
);
}

private function createCachedExecutor($namserver, LoopInterface $loop, CacheInterface $cache)
{
return new CachingExecutor($this->createRetryExecutor($namserver, $loop), $cache);
return new CoopExecutor($executor);
}
}
28 changes: 28 additions & 0 deletions tests/FunctionalResolverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,34 @@ public function testResolveGoogleResolves()
$this->loop->run();
}

/**
* @group internet
*/
public function testResolveGoogleOverUdpResolves()
{
$factory = new Factory($this->loop);
$this->resolver = $factory->create('udp://8.8.8.8', $this->loop);

$promise = $this->resolver->resolve('google.com');
$promise->then($this->expectCallableOnce(), $this->expectCallableNever());

$this->loop->run();
}

/**
* @group internet
*/
public function testResolveGoogleOverTcpResolves()
{
$factory = new Factory($this->loop);
$this->resolver = $factory->create('tcp://8.8.8.8', $this->loop);

$promise = $this->resolver->resolve('google.com');
$promise->then($this->expectCallableOnce(), $this->expectCallableNever());

$this->loop->run();
}

/**
* @group internet
*/
Expand Down
38 changes: 34 additions & 4 deletions tests/Query/TcpTransportExecutorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,30 @@ public function testCtorShouldAcceptNameserverAddresses($input, $expected)
public static function provideDefaultPortProvider()
{
return array(
array('8.8.8.8', '8.8.8.8:53'),
array('1.2.3.4:5', '1.2.3.4:5'),
array('::1', '[::1]:53'),
array('[::1]:53', '[::1]:53')
array(
'8.8.8.8',
'8.8.8.8:53'
),
array(
'1.2.3.4:5',
'1.2.3.4:5'
),
array(
'tcp://1.2.3.4',
'1.2.3.4:53'
),
array(
'tcp://1.2.3.4:53',
'1.2.3.4:53'
),
array(
'::1',
'[::1]:53'
),
array(
'[::1]:53',
'[::1]:53'
)
);
}

Expand All @@ -60,6 +80,16 @@ public function testCtorShouldThrowWhenNameserverAddressContainsHostname()
new TcpTransportExecutor('localhost', $loop);
}

/**
* @expectedException InvalidArgumentException
*/
public function testCtorShouldThrowWhenNameserverSchemeIsInvalid()
{
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();

new TcpTransportExecutor('udp://1.2.3.4', $loop);
}

public function testQueryRejectsIfMessageExceedsMaximumMessageSize()
{
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
Expand Down
50 changes: 44 additions & 6 deletions tests/Query/UdpTransportExecutorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,30 @@ public function testCtorShouldAcceptNameserverAddresses($input, $expected)
public static function provideDefaultPortProvider()
{
return array(
array('8.8.8.8', 'udp://8.8.8.8:53'),
array('1.2.3.4:5', 'udp://1.2.3.4:5'),
array('localhost', 'udp://localhost:53'),
array('localhost:1234', 'udp://localhost:1234'),
array('::1', 'udp://[::1]:53'),
array('[::1]:53', 'udp://[::1]:53')
array(
'8.8.8.8',
'udp://8.8.8.8:53'
),
array(
'1.2.3.4:5',
'udp://1.2.3.4:5'
),
array(
'udp://1.2.3.4',
'udp://1.2.3.4:53'
),
array(
'udp://1.2.3.4:53',
'udp://1.2.3.4:53'
),
array(
'::1',
'udp://[::1]:53'
),
array(
'[::1]:53',
'udp://[::1]:53'
)
);
}

Expand All @@ -52,6 +70,26 @@ public function testCtorShouldThrowWhenNameserverAddressIsInvalid()
new UdpTransportExecutor('///', $loop);
}

/**
* @expectedException InvalidArgumentException
*/
public function testCtorShouldThrowWhenNameserverAddressContainsHostname()
{
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();

new UdpTransportExecutor('localhost', $loop);
}

/**
* @expectedException InvalidArgumentException
*/
public function testCtorShouldThrowWhenNameserverSchemeIsInvalid()
{
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();

new UdpTransportExecutor('tcp://1.2.3.4', $loop);
}

public function testQueryRejectsIfMessageExceedsUdpSize()
{
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
Expand Down
94 changes: 94 additions & 0 deletions tests/Resolver/FactoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,100 @@ public function createShouldCreateResolver()
$this->assertInstanceOf('React\Dns\Resolver\Resolver', $resolver);
}


/** @test */
public function createWithoutSchemeShouldCreateResolverWithUdpExecutorStack()
{
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();

$factory = new Factory();
$resolver = $factory->create('8.8.8.8:53', $loop);

$this->assertInstanceOf('React\Dns\Resolver\Resolver', $resolver);

$coopExecutor = $this->getResolverPrivateExecutor($resolver);

$this->assertInstanceOf('React\Dns\Query\CoopExecutor', $coopExecutor);

$ref = new \ReflectionProperty($coopExecutor, 'executor');
$ref->setAccessible(true);
$retryExecutor = $ref->getValue($coopExecutor);

$this->assertInstanceOf('React\Dns\Query\RetryExecutor', $retryExecutor);

$ref = new \ReflectionProperty($retryExecutor, 'executor');
$ref->setAccessible(true);
$timeoutExecutor = $ref->getValue($retryExecutor);

$this->assertInstanceOf('React\Dns\Query\TimeoutExecutor', $timeoutExecutor);

$ref = new \ReflectionProperty($timeoutExecutor, 'executor');
$ref->setAccessible(true);
$udpExecutor = $ref->getValue($timeoutExecutor);

$this->assertInstanceOf('React\Dns\Query\UdpTransportExecutor', $udpExecutor);
}

/** @test */
public function createWithUdpSchemeShouldCreateResolverWithUdpExecutorStack()
{
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();

$factory = new Factory();
$resolver = $factory->create('udp://8.8.8.8:53', $loop);

$this->assertInstanceOf('React\Dns\Resolver\Resolver', $resolver);

$coopExecutor = $this->getResolverPrivateExecutor($resolver);

$this->assertInstanceOf('React\Dns\Query\CoopExecutor', $coopExecutor);

$ref = new \ReflectionProperty($coopExecutor, 'executor');
$ref->setAccessible(true);
$retryExecutor = $ref->getValue($coopExecutor);

$this->assertInstanceOf('React\Dns\Query\RetryExecutor', $retryExecutor);

$ref = new \ReflectionProperty($retryExecutor, 'executor');
$ref->setAccessible(true);
$timeoutExecutor = $ref->getValue($retryExecutor);

$this->assertInstanceOf('React\Dns\Query\TimeoutExecutor', $timeoutExecutor);

$ref = new \ReflectionProperty($timeoutExecutor, 'executor');
$ref->setAccessible(true);
$udpExecutor = $ref->getValue($timeoutExecutor);

$this->assertInstanceOf('React\Dns\Query\UdpTransportExecutor', $udpExecutor);
}

/** @test */
public function createWithTcpSchemeShouldCreateResolverWithTcpExecutorStack()
{
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();

$factory = new Factory();
$resolver = $factory->create('tcp://8.8.8.8:53', $loop);

$this->assertInstanceOf('React\Dns\Resolver\Resolver', $resolver);

$coopExecutor = $this->getResolverPrivateExecutor($resolver);

$this->assertInstanceOf('React\Dns\Query\CoopExecutor', $coopExecutor);

$ref = new \ReflectionProperty($coopExecutor, 'executor');
$ref->setAccessible(true);
$timeoutExecutor = $ref->getValue($coopExecutor);

$this->assertInstanceOf('React\Dns\Query\TimeoutExecutor', $timeoutExecutor);

$ref = new \ReflectionProperty($timeoutExecutor, 'executor');
$ref->setAccessible(true);
$tcpExecutor = $ref->getValue($timeoutExecutor);

$this->assertInstanceOf('React\Dns\Query\TcpTransportExecutor', $tcpExecutor);
}

/**
* @test
* @expectedException InvalidArgumentException
Expand Down