Skip to content

Commit dbf4fcd

Browse files
clueclue
committed
Remove broken TCP code, do not retry with invalid TCP query
1 parent cc9d2fb commit dbf4fcd

2 files changed

Lines changed: 31 additions & 143 deletions

File tree

‎src/Query/Executor.php‎

Lines changed: 18 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22

33
namespace React\Dns\Query;
44

5-
use React\Dns\BadServerException;
65
use React\Dns\Model\Message;
76
use React\Dns\Protocol\Parser;
87
use React\Dns\Protocol\BinaryDumper;
98
use React\EventLoop\LoopInterface;
109
use React\Promise\Deferred;
1110
use React\Socket\Connection;
11+
use React\Promise;
1212

1313
class Executor implements ExecutorInterface
1414
{
@@ -56,10 +56,24 @@ public function prepareRequest(Query $query)
5656

5757
public function doQuery($nameserver, $transport, $queryData, $name)
5858
{
59+
// we only support UDP right now
60+
if ($transport !== 'udp') {
61+
return Promise\reject(new \RuntimeException(
62+
'Only UDP transport supported in this version'
63+
));
64+
}
65+
5966
$that = $this;
6067
$parser = $this->parser;
6168
$loop = $this->loop;
6269

70+
// UDP connections are instant, so try this without a timer
71+
try {
72+
$conn = $this->createConnection($nameserver, $transport);
73+
} catch (\Exception $e) {
74+
return Promise\reject(new \RuntimeException('Unable to connect to DNS server: ' . $e->getMessage(), 0, $e));
75+
}
76+
6377
$deferred = new Deferred(function ($resolve, $reject) use (&$timer, $loop, &$conn, $name) {
6478
$reject(new CancellationException(sprintf('DNS query for %s has been cancelled', $name)));
6579

@@ -69,10 +83,6 @@ public function doQuery($nameserver, $transport, $queryData, $name)
6983
$conn->close();
7084
});
7185

72-
$retryWithTcp = function () use ($that, $nameserver, $queryData, $name) {
73-
return $that->doQuery($nameserver, 'tcp', $queryData, $name);
74-
};
75-
7686
$timer = null;
7787
if ($this->timeout !== null) {
7888
$timer = $this->loop->addTimer($this->timeout, function () use (&$conn, $name, $deferred) {
@@ -81,54 +91,24 @@ public function doQuery($nameserver, $transport, $queryData, $name)
8191
});
8292
}
8393

84-
try {
85-
try {
86-
$conn = $this->createConnection($nameserver, $transport);
87-
} catch (\Exception $e) {
88-
if ($transport === 'udp') {
89-
// UDP failed => retry with TCP
90-
$transport = 'tcp';
91-
$conn = $this->createConnection($nameserver, $transport);
92-
} else {
93-
// TCP failed (UDP must already have been checked before)
94-
throw $e;
95-
}
96-
}
97-
} catch (\Exception $e) {
98-
// both UDP and TCP failed => reject
99-
if ($timer !== null) {
100-
$loop->cancelTimer($timer);
101-
}
102-
$deferred->reject(new \RuntimeException('Unable to connect to DNS server: ' . $e->getMessage(), 0, $e));
103-
104-
return $deferred->promise();
105-
}
106-
107-
$conn->on('data', function ($data) use ($retryWithTcp, $conn, $parser, $transport, $deferred, $timer, $loop) {
94+
$conn->on('data', function ($data) use ($conn, $parser, $deferred, $timer, $loop) {
95+
$conn->end();
10896
if ($timer !== null) {
10997
$loop->cancelTimer($timer);
11098
}
11199

112100
try {
113101
$response = $parser->parseMessage($data);
114102
} catch (\Exception $e) {
115-
$conn->end();
116103
$deferred->reject($e);
117104
return;
118105
}
119106

120107
if ($response->header->isTruncated()) {
121-
if ('tcp' === $transport) {
122-
$deferred->reject(new BadServerException('The server set the truncated bit although we issued a TCP request'));
123-
} else {
124-
$conn->end();
125-
$deferred->resolve($retryWithTcp());
126-
}
127-
108+
$deferred->reject(new \RuntimeException('The server returned a truncated result for the UDP query, retrying via TCP is currently not supported'));
128109
return;
129110
}
130111

131-
$conn->end();
132112
$deferred->resolve($response);
133113
});
134114
$conn->write($queryData);

‎tests/Query/ExecutorTest.php‎

Lines changed: 13 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -46,23 +46,15 @@ public function queryShouldCreateUdpRequest()
4646
}
4747

4848
/** @test */
49-
public function resolveShouldCreateTcpRequestIfRequestIsLargerThan512Bytes()
49+
public function resolveShouldRejectIfRequestIsLargerThan512Bytes()
5050
{
51-
$timer = $this->getMockBuilder('React\EventLoop\Timer\TimerInterface')->getMock();
52-
$this->loop
53-
->expects($this->any())
54-
->method('addTimer')
55-
->will($this->returnValue($timer));
56-
57-
$this->executor = $this->createExecutorMock();
58-
$this->executor
59-
->expects($this->once())
60-
->method('createConnection')
61-
->with('8.8.8.8:53', 'tcp')
62-
->will($this->returnNewConnectionMock(false));
63-
6451
$query = new Query(str_repeat('a', 512).'.igor.io', Message::TYPE_A, Message::CLASS_IN, 1345656451);
65-
$this->executor->query('8.8.8.8:53', $query);
52+
$promise = $this->executor->query('8.8.8.8:53', $query);
53+
54+
$promise->then(
55+
null,
56+
$this->expectCallableOnce()
57+
);
6658
}
6759

6860
/** @test */
@@ -129,7 +121,7 @@ public function resolveShouldNotStartOrCancelTimerWhenCancelledWithTimeoutIsNull
129121
}
130122

131123
/** @test */
132-
public function resolveShouldRetryWithTcpIfResponseIsTruncated()
124+
public function resolveShouldRejectIfResponseIsTruncated()
133125
{
134126
$timer = $this->getMockBuilder('React\EventLoop\Timer\TimerInterface')->getMock();
135127

@@ -139,86 +131,38 @@ public function resolveShouldRetryWithTcpIfResponseIsTruncated()
139131
->will($this->returnValue($timer));
140132

141133
$this->parser
142-
->expects($this->at(0))
134+
->expects($this->once())
143135
->method('parseMessage')
144136
->will($this->returnTruncatedResponse());
145-
$this->parser
146-
->expects($this->at(1))
147-
->method('parseMessage')
148-
->will($this->returnStandardResponse());
149137

150138
$this->executor = $this->createExecutorMock();
151139
$this->executor
152-
->expects($this->at(0))
153-
->method('createConnection')
154-
->with('8.8.8.8:53', 'udp')
155-
->will($this->returnNewConnectionMock());
156-
$this->executor
157-
->expects($this->at(1))
158-
->method('createConnection')
159-
->with('8.8.8.8:53', 'tcp')
160-
->will($this->returnNewConnectionMock());
161-
162-
$query = new Query('igor.io', Message::TYPE_A, Message::CLASS_IN, 1345656451);
163-
$this->executor->query('8.8.8.8:53', $query);
164-
}
165-
166-
/** @test */
167-
public function resolveShouldRetryWithTcpIfUdpThrows()
168-
{
169-
$timer = $this->getMockBuilder('React\EventLoop\Timer\TimerInterface')->getMock();
170-
171-
$this->loop
172140
->expects($this->once())
173-
->method('addTimer')
174-
->will($this->returnValue($timer));
175-
176-
$this->parser
177-
->expects($this->once())
178-
->method('parseMessage')
179-
->will($this->returnStandardResponse());
180-
181-
$this->executor = $this->createExecutorMock();
182-
$this->executor
183-
->expects($this->at(0))
184141
->method('createConnection')
185142
->with('8.8.8.8:53', 'udp')
186-
->will($this->throwException(new \Exception()));
187-
$this->executor
188-
->expects($this->at(1))
189-
->method('createConnection')
190-
->with('8.8.8.8:53', 'tcp')
191143
->will($this->returnNewConnectionMock());
192144

193145
$query = new Query('igor.io', Message::TYPE_A, Message::CLASS_IN, 1345656451);
194146
$this->executor->query('8.8.8.8:53', $query);
195147
}
196148

197149
/** @test */
198-
public function resolveShouldFailIfBothUdpAndTcpThrow()
150+
public function resolveShouldFailIfUdpThrow()
199151
{
200-
$timer = $this->getMockBuilder('React\EventLoop\Timer\TimerInterface')->getMock();
201-
202152
$this->loop
203-
->expects($this->once())
204-
->method('addTimer')
205-
->will($this->returnValue($timer));
153+
->expects($this->never())
154+
->method('addTimer');
206155

207156
$this->parser
208157
->expects($this->never())
209158
->method('parseMessage');
210159

211160
$this->executor = $this->createExecutorMock();
212161
$this->executor
213-
->expects($this->at(0))
162+
->expects($this->once())
214163
->method('createConnection')
215164
->with('8.8.8.8:53', 'udp')
216165
->will($this->throwException(new \Exception()));
217-
$this->executor
218-
->expects($this->at(1))
219-
->method('createConnection')
220-
->with('8.8.8.8:53', 'tcp')
221-
->will($this->throwException(new \Exception()));
222166

223167
$query = new Query('igor.io', Message::TYPE_A, Message::CLASS_IN, 1345656451);
224168
$promise = $this->executor->query('8.8.8.8:53', $query);
@@ -235,42 +179,6 @@ public function resolveShouldFailIfBothUdpAndTcpThrow()
235179
$promise->then($this->expectCallableNever(), $mock);
236180
}
237181

238-
/** @test */
239-
public function resolveShouldFailIfResponseIsTruncatedAfterTcpRequest()
240-
{
241-
$timer = $this->getMockBuilder('React\EventLoop\Timer\TimerInterface')->getMock();
242-
243-
$this->loop
244-
->expects($this->any())
245-
->method('addTimer')
246-
->will($this->returnValue($timer));
247-
248-
$this->parser
249-
->expects($this->once())
250-
->method('parseMessage')
251-
->will($this->returnTruncatedResponse());
252-
253-
$this->executor = $this->createExecutorMock();
254-
$this->executor
255-
->expects($this->once())
256-
->method('createConnection')
257-
->with('8.8.8.8:53', 'tcp')
258-
->will($this->returnNewConnectionMock());
259-
260-
$mock = $this->createCallableMock();
261-
$mock
262-
->expects($this->once())
263-
->method('__invoke')
264-
->with($this->callback(function($e) {
265-
return $e instanceof \React\Dns\BadServerException &&
266-
'The server set the truncated bit although we issued a TCP request' === $e->getMessage();
267-
}));
268-
269-
$query = new Query(str_repeat('a', 512).'.igor.io', Message::TYPE_A, Message::CLASS_IN, 1345656451);
270-
$this->executor->query('8.8.8.8:53', $query)
271-
->then($this->expectCallableNever(), $mock);
272-
}
273-
274182
/** @test */
275183
public function resolveShouldCancelTimerWhenFullResponseIsReceived()
276184
{

0 commit comments

Comments
 (0)