Skip to content

support for search options in /etc/resolv.conf #199

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

Closed
Chrisdowson wants to merge 10 commits into from
Closed

support for search options in /etc/resolv.conf #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
fa1684d
support for search options in /etc/resolv.conf
Chrisdowson May 21, 2022
54ed2cf
1. fix test case and optimized code
Chrisdowson May 28, 2022
1a1b72d
fix test case
Chrisdowson May 29, 2022
0ba2010
fix test case
Chrisdowson May 29, 2022
2202726
Merge branch '1.x' into 1.x
Chrisdowson Nov 9, 2022
38c9526
1.revert
Chrisdowson Dec 24, 2022
23cb5e6
Merge branch 'reactphp:1.x' into 1.x
Chrisdowson Dec 24, 2022
8980bdd
1.fix bug
Chrisdowson Dec 24, 2022
b3e980a
Optimize code
Chrisdowson Jan 2, 2023
e675f09
Merge branch '1.x' into 1.x
Chrisdowson Dec 7, 2023
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
9 changes: 8 additions & 1 deletion src/Config/Config.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,13 @@ public static function loadResolvConfBlocking($path = null)
$config->nameservers[] = $ip;
}
}

$matches = array();
preg_match_all('/^search.*\s*$/m', $contents, $matches);
if(isset($matches[0][0])){
$searches = preg_split('/\s+/', trim($matches[0][0]));
unset($searches[0]);
$config->searches= array_values($searches);
}
return $config;
}

Expand Down Expand Up @@ -134,4 +140,5 @@ public static function loadWmicBlocking($command = null)
}

public $nameservers = array();
public $searches = array();
}
2 changes: 2 additions & 0 deletions src/Query/RetryExecutor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ final class RetryExecutor implements ExecutorInterface
{
private $executor;
private $retries;
private $config;

public function __construct(ExecutorInterface $executor, $retries = 2)
{
$this->executor = $executor;
$this->retries = $retries;
$this->config = \React\Dns\Config\Config::loadSystemConfigBlocking();
}

public function query(Query $query)
Expand Down
3 changes: 3 additions & 0 deletions src/Query/TimeoutExecutor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace React\Dns\Query;

use React\Dns\Model\Message;
use React\EventLoop\Loop;
use React\EventLoop\LoopInterface;
use React\Promise\Promise;
Expand All @@ -11,12 +12,14 @@ final class TimeoutExecutor implements ExecutorInterface
private $executor;
private $loop;
private $timeout;
private $config;

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

public function query(Query $query)
Expand Down
9 changes: 8 additions & 1 deletion src/Query/UdpTransportExecutor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use React\Dns\Model\Message;
use React\Dns\Protocol\BinaryDumper;
use React\Dns\Protocol\Parser;
use React\Dns\RecordNotFoundException;
use React\EventLoop\Loop;
use React\EventLoop\LoopInterface;
use React\Promise\Deferred;
Expand Down Expand Up @@ -208,7 +209,13 @@ public function query(Query $query)
));
return;
}

if($response->rcode == Message::RCODE_NAME_ERROR){
$deferred->reject(new RecordNotFoundException(
'DNS query for ' . $query->describe() . ' returned an error response (Non-Existent Domain / NXDOMAIN)',
$response->rcode
));
return;
}
$deferred->resolve($response);
});

Expand Down
49 changes: 26 additions & 23 deletions tests/FunctionalResolverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ public function testResolveAllLocalhostResolvesWithArray()
/**
* @group internet
*/
public function testResolveGoogleResolves()
public function testResolveBingResolves()
{
$promise = $this->resolver->resolve('google.com');
$promise = $this->resolver->resolve('bing.com');
Copy link
Member

@SimonFrings SimonFrings Jan 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see why we should change google.com to bing.com here, seems unnecessary. Same goes for the other tests.

Copy link
Author

@Chrisdowson Chrisdowson Jan 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In China,we can not visit google.com, so tests will not pass on my computer,But bing.com we can

Copy link
Member

@SimonFrings SimonFrings Jan 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you that we should use something that works for everyone, in some of our projects we started using reactphp.org instead, but I don't think this belongs in this pull request. Maybe use bing.com for testing purposes on your system without committing it in the end.

What do you think?

Copy link
Member

@WyriHaximus WyriHaximus Jan 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SimonFrings IMHO it makes sense to change everything to reactphp.org to align this across the board. Will file PoC PR for that to take this discussion there.

@Chrisdowson I assume you can reacht reactphp.org just fine?

Copy link
Author

@Chrisdowson Chrisdowson Jan 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes,I can reach reactphp.org

Copy link
Author

@Chrisdowson Chrisdowson Jan 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you that we should use something that works for everyone, in some of our projects we started using reactphp.org instead, but I don't think this belongs in this pull request. Maybe use bing.com for testing purposes on your system without committing it in the end.

What do you think?

I agree with WyriHaximus, How about I replace bing.com with reactphp.org now?

Copy link
Member

@SimonFrings SimonFrings Jan 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WyriHaximus @Chrisdowson I agree with both of you, I am not sure if this is something we should tackle in this pull request as it doesn't fit to the other changes made in here and could seem a bit random. It also keeps the diff significantly smaller and lays focus on the actual changes. That's the reason I suggest a (follow-up) pull request where we change google.com to reactphp.org.

Copy link
Member

@WyriHaximus WyriHaximus Jan 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SimonFrings @Chrisdowson just opened reactphp/event-loop#263 to kick off the discussion.

$promise->then($this->expectCallableOnce(), $this->expectCallableNever());

Loop::run();
Expand All @@ -49,12 +49,12 @@ public function testResolveGoogleResolves()
/**
* @group internet
*/
public function testResolveGoogleOverUdpResolves()
public function testResolveBingleOverUdpResolves()
{
$factory = new Factory();
$this->resolver = $factory->create('udp://8.8.8.8');

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

Loop::run();
Expand All @@ -63,12 +63,12 @@ public function testResolveGoogleOverUdpResolves()
/**
* @group internet
*/
public function testResolveGoogleOverTcpResolves()
public function testResolveBingOverTcpResolves()
{
$factory = new Factory();
$this->resolver = $factory->create('tcp://8.8.8.8');

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

Loop::run();
Expand All @@ -77,25 +77,25 @@ public function testResolveGoogleOverTcpResolves()
/**
* @group internet
*/
public function testResolveAllGoogleMxResolvesWithCache()
public function testResolveAllBingMxResolvesWithCache()
{
$factory = new Factory();
$this->resolver = $factory->createCached('8.8.8.8');

$promise = $this->resolver->resolveAll('google.com', Message::TYPE_MX);
$promise = $this->resolver->resolveAll('bing.com', Message::TYPE_MX);
$promise->then($this->expectCallableOnceWith($this->isType('array')), $this->expectCallableNever());

Loop::run();
}
/**
* @group internet
*/
public function testResolveAllGoogleCaaResolvesWithCache()
public function testResolveAllbingCaaResolvesWithCache()
{
$factory = new Factory();
$this->resolver = $factory->createCached('8.8.8.8');

$promise = $this->resolver->resolveAll('google.com', Message::TYPE_CAA);
$promise = $this->resolver->resolveAll('bing.com', Message::TYPE_CAA);
$promise->then($this->expectCallableOnceWith($this->isType('array')), $this->expectCallableNever());

Loop::run();
Expand All @@ -114,19 +114,21 @@ public function testResolveInvalidRejects()
$promise->then(null, function ($reason) use (&$exception) {
$exception = $reason;
});

/** @var \React\Dns\RecordNotFoundException $exception */
$this->assertInstanceOf('React\Dns\RecordNotFoundException', $exception);
$this->assertEquals('DNS query for example.invalid (A) returned an error response (Non-Existent Domain / NXDOMAIN)', $exception->getMessage());
$this->assertEquals(Message::RCODE_NAME_ERROR, $exception->getCode());
$config = \React\Dns\Config\Config::loadSystemConfigBlocking();
if (!count($config->searches)) {
/** @var \React\Dns\RecordNotFoundException $exception */
$this->assertInstanceOf('React\Dns\RecordNotFoundException', $exception);
$this->assertEquals('DNS query for example.invalid (A) returned an error response (Non-Existent Domain / NXDOMAIN)', $exception->getMessage());
$this->assertEquals(Message::RCODE_NAME_ERROR, $exception->getCode());
}
}

public function testResolveCancelledRejectsImmediately()
{
// max_nesting_level was set to 100 for PHP Versions < 5.4 which resulted in failing test for legacy PHP
ini_set('xdebug.max_nesting_level', 256);

$promise = $this->resolver->resolve('google.com');
$promise = $this->resolver->resolve('bing.com');
$promise->cancel();

$time = microtime(true);
Expand All @@ -142,15 +144,15 @@ public function testResolveCancelledRejectsImmediately()

/** @var \React\Dns\Query\CancellationException $exception */
$this->assertInstanceOf('React\Dns\Query\CancellationException', $exception);
$this->assertEquals('DNS query for google.com (A) has been cancelled', $exception->getMessage());
$this->assertEquals('DNS query for bing.com (A) has been cancelled', $exception->getMessage());
}

/**
* @group internet
*/
public function testResolveAllInvalidTypeRejects()
{
$promise = $this->resolver->resolveAll('google.com', Message::TYPE_PTR);
$promise = $this->resolver->resolveAll('bing.com', Message::TYPE_PTR);

Loop::run();

Expand All @@ -161,17 +163,18 @@ public function testResolveAllInvalidTypeRejects()

/** @var \React\Dns\RecordNotFoundException $exception */
$this->assertInstanceOf('React\Dns\RecordNotFoundException', $exception);
$this->assertEquals('DNS query for google.com (PTR) did not return a valid answer (NOERROR / NODATA)', $exception->getMessage());
$this->assertEquals('DNS query for bing.com (PTR) did not return a valid answer (NOERROR / NODATA)', $exception->getMessage());
$this->assertEquals(0, $exception->getCode());
}

public function testInvalidResolverDoesNotResolveGoogle()
public function testInvalidResolverDoesNotResolvebing()
{
$factory = new Factory();
$this->resolver = $factory->create('255.255.255.255');

$promise = $this->resolver->resolve('google.com');
$promise = $this->resolver->resolve('bing.com');
$promise->then($this->expectCallableNever(), $this->expectCallableOnce());
Loop::run();
}

public function testResolveShouldNotCauseGarbageReferencesWhenUsingInvalidNameserver()
Expand Down Expand Up @@ -231,7 +234,7 @@ public function testCancelResolveShouldNotCauseGarbageReferences()
// collect all garbage cycles
}

$promise = $this->resolver->resolve('google.com');
$promise = $this->resolver->resolve('bing.com');
$promise->cancel();
$promise = null;

Expand All @@ -251,7 +254,7 @@ public function testCancelResolveCachedShouldNotCauseGarbageReferences()
// collect all garbage cycles
}

$promise = $this->resolver->resolve('google.com');
$promise = $this->resolver->resolve('bing.com');
$promise->cancel();
$promise = null;

Expand Down
6 changes: 3 additions & 3 deletions tests/Query/TcpTransportExecutorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ public function testQueryRejectsWhenServerSendsInvalidMessage()
$address = stream_socket_get_name($server, false);
$executor = new TcpTransportExecutor($address);

$query = new Query('google.com', Message::TYPE_A, Message::CLASS_IN);
$query = new Query('bing.com', Message::TYPE_A, Message::CLASS_IN);

$exception = null;
$executor->query($query)->then(
Expand All @@ -552,7 +552,7 @@ function ($e) use (&$exception) {

/** @var \RuntimeException $exception */
$this->assertInstanceOf('RuntimeException', $exception);
$this->assertEquals('DNS query for google.com (A) failed: Invalid message received from DNS server tcp://' . $address, $exception->getMessage());
$this->assertEquals('DNS query for bing.com (A) failed: Invalid message received from DNS server tcp://' . $address, $exception->getMessage());
}

public function testQueryRejectsWhenServerSendsInvalidId()
Expand Down Expand Up @@ -688,7 +688,7 @@ public function testQueryResolvesIfServerSendsValidResponse()

$this->assertInstanceOf('React\Dns\Model\Message', $response);
}

Copy link
Member

@SimonFrings SimonFrings Jan 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a leftover.

public function testQueryRejectsIfSocketIsClosedAfterPreviousQueryThatWasStillPending()
{
$loop = $this->getMockBuilder('React\EventLoop\LoopInterface')->getMock();
Expand Down