Skip to content

Commit 618fa46

Browse files
ChadSikorraChadSikorra
committed
If an error occurs during a search and an operation exception is thrown, return the result code as part of the SearchResultDone message. This is how LDAP clients would expect it and how the RFC specifies that it would be done.
Currently, the operation exception comes as an unexpected message after the SearchResultDone which would be considered an unsolicited message from a clients perspective and would cause an immediate disconnected from the server.
1 parent 51935ed commit 618fa46

File tree

6 files changed

+198
-31
lines changed

6 files changed

+198
-31
lines changed

‎src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerPagingHandler.php‎

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -81,15 +81,36 @@ public function handleRequest(
8181
);
8282
$pagingRequest = $this->findOrMakePagingRequest($message);
8383

84-
$response = $this->handlePaging(
85-
$context,
86-
$pagingRequest,
87-
$message
88-
);
84+
$response = null;
85+
$controls = [];
86+
try {
87+
$response = $this->handlePaging(
88+
$context,
89+
$pagingRequest,
90+
$message
91+
);
92+
$searchResult = ServerSearchResult::makeSuccessResult($response->getEntries());
93+
$controls[] = new PagingControl(
94+
$response->getRemaining(),
95+
$response->isComplete()
96+
? ''
97+
: $pagingRequest->getNextCookie()
98+
);
99+
} catch (OperationException $e) {
100+
$searchResult = ServerSearchResult::makeErrorResult(
101+
$e->getCode(),
102+
'',
103+
$e->getMessage()
104+
);
105+
$controls[] = new PagingControl(
106+
0,
107+
$pagingRequest->getCookie()
108+
);
109+
}
89110

90111
$pagingRequest->markProcessed();
91112

92-
if ($response->isComplete()) {
113+
if ($response && $response->isComplete()) {
93114
$this->requestHistory->pagingRequest()
94115
->remove($pagingRequest);
95116
$this->pagingHandler->remove(
@@ -99,13 +120,10 @@ public function handleRequest(
99120
}
100121

101122
$this->sendEntriesToClient(
102-
$response->getEntries(),
123+
$searchResult,
103124
$message,
104125
$queue,
105-
new PagingControl(
106-
$response->getRemaining(),
107-
$response->isComplete() ? '' : $pagingRequest->getNextCookie()
108-
)
126+
...$controls
109127
);
110128
}
111129

‎src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerPagingUnsupportedHandler.php‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,20 @@ public function handleRequest(
6060
);
6161
}
6262

63-
$entries = $dispatcher->search(
64-
$context,
65-
$request
66-
);
63+
try {
64+
$searchResult = ServerSearchResult::makeSuccessResult($dispatcher->search(
65+
$context,
66+
$request
67+
));
68+
} catch (OperationException $e) {
69+
$searchResult = ServerSearchResult::makeErrorResult(
70+
$e->getCode(),
71+
$e->getMessage()
72+
);
73+
}
6774

6875
$this->sendEntriesToClient(
69-
$entries,
76+
$searchResult,
7077
$message,
7178
$queue
7279
);

‎src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerSearchHandler.php‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace FreeDSx\Ldap\Protocol\ServerProtocolHandler;
1313

14+
use FreeDSx\Ldap\Exception\OperationException;
1415
use FreeDSx\Ldap\Protocol\LdapMessageRequest;
1516
use FreeDSx\Ldap\Protocol\Queue\ServerQueue;
1617
use FreeDSx\Ldap\Server\RequestContext;
@@ -42,13 +43,20 @@ public function handleRequest(
4243
);
4344
$request = $this->getSearchRequestFromMessage($message);
4445

45-
$entries = $dispatcher->search(
46-
$context,
47-
$request
48-
);
46+
try {
47+
$searchResult = ServerSearchResult::makeSuccessResult($dispatcher->search(
48+
$context,
49+
$request
50+
));
51+
} catch (OperationException $e) {
52+
$searchResult = ServerSearchResult::makeErrorResult(
53+
$e->getCode(),
54+
$e->getMessage()
55+
);
56+
}
4957

5058
$this->sendEntriesToClient(
51-
$entries,
59+
$searchResult,
5260
$message,
5361
$queue
5462
);

‎src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerSearchResult.php‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* This file is part of the FreeDSx LDAP package.
7+
*
8+
* (c) Chad Sikorra <Chad.Sikorra@gmail.com>
9+
*
10+
* For the full copyright and license information, please view the LICENSE
11+
* file that was distributed with this source code.
12+
*/
13+
14+
namespace FreeDSx\Ldap\Protocol\ServerProtocolHandler;
15+
16+
use FreeDSx\Ldap\Entry\Entries;
17+
use FreeDSx\Ldap\Exception\InvalidArgumentException;
18+
use FreeDSx\Ldap\Operation\ResultCode;
19+
20+
final class ServerSearchResult
21+
{
22+
/**
23+
* @var Entries|null
24+
*/
25+
private $entries;
26+
27+
/**
28+
* @var string
29+
*/
30+
private $baseDn;
31+
32+
/**
33+
* @var int
34+
*/
35+
private $resultCode;
36+
37+
/**
38+
* @var string
39+
*/
40+
private $diagnosticMessage;
41+
42+
private function __construct(
43+
?Entries $entries,
44+
string $baseDn = '',
45+
int $resultCode = ResultCode::SUCCESS,
46+
string $diagnosticMessage = ''
47+
) {
48+
$this->entries = $entries;
49+
$this->baseDn = $baseDn;
50+
$this->resultCode = $resultCode;
51+
$this->diagnosticMessage = $diagnosticMessage;
52+
}
53+
54+
/**
55+
* Make a successful server search result representation.
56+
*/
57+
public static function makeSuccessResult(
58+
Entries $entries,
59+
string $baseDn = '',
60+
string $diagnosticMessage = ''
61+
): self {
62+
return new self(
63+
$entries,
64+
$baseDn,
65+
ResultCode::SUCCESS,
66+
$diagnosticMessage
67+
);
68+
}
69+
70+
/**
71+
* Make an error result for server search result representation. This could occur for any reason, such as a base DN
72+
* not existing. This result MUST not return a success result code.
73+
*/
74+
public static function makeErrorResult(
75+
int $resultCode,
76+
string $baseDn = '',
77+
string $diagnosticMessage = '',
78+
?Entries $entries = null
79+
): self {
80+
if ($resultCode === ResultCode::SUCCESS) {
81+
throw new InvalidArgumentException('You must not return a success result code on a search error.');
82+
}
83+
84+
return new self(
85+
$entries,
86+
$baseDn,
87+
$resultCode,
88+
$diagnosticMessage
89+
);
90+
}
91+
92+
public function getEntries(): ?Entries
93+
{
94+
return $this->entries;
95+
}
96+
97+
public function getResultCode(): int
98+
{
99+
return $this->resultCode;
100+
}
101+
102+
public function getDiagnosticMessage(): string
103+
{
104+
return $this->diagnosticMessage;
105+
}
106+
107+
public function getBaseDn(): string
108+
{
109+
return $this->baseDn;
110+
}
111+
}

‎src/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerSearchTrait.php‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,29 +27,36 @@
2727
trait ServerSearchTrait
2828
{
2929
/**
30-
* @param Entries $entries
30+
* @param ServerSearchResult $searchResult
3131
* @param LdapMessageRequest $message
3232
* @param ServerQueue $queue
3333
* @return void
3434
*/
3535
private function sendEntriesToClient(
36-
Entries $entries,
36+
ServerSearchResult $searchResult,
3737
LdapMessageRequest $message,
3838
ServerQueue $queue,
3939
Control ...$controls
4040
): void {
4141
$messages = [];
42+
$entries = $searchResult->getEntries();
4243

43-
foreach ($entries->toArray() as $entry) {
44-
$messages[] = new LdapMessageResponse(
45-
$message->getMessageId(),
46-
new SearchResultEntry($entry)
47-
);
44+
if ($entries !== null) {
45+
foreach ($entries->toArray() as $entry) {
46+
$messages[] = new LdapMessageResponse(
47+
$message->getMessageId(),
48+
new SearchResultEntry($entry)
49+
);
50+
}
4851
}
4952

5053
$messages[] = new LdapMessageResponse(
5154
$message->getMessageId(),
52-
new SearchResultDone(ResultCode::SUCCESS),
55+
new SearchResultDone(
56+
$searchResult->getResultCode(),
57+
$searchResult->getBaseDn(),
58+
$searchResult->getDiagnosticMessage()
59+
),
5360
...$controls
5461
);
5562

‎tests/spec/FreeDSx/Ldap/Protocol/ServerProtocolHandler/ServerPagingHandlerSpec.php‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
use FreeDSx\Ldap\Entry\Entry;
1919
use FreeDSx\Ldap\Exception\OperationException;
2020
use FreeDSx\Ldap\Operation\Request\SearchRequest;
21+
use FreeDSx\Ldap\Operation\Response\SearchResultDone;
2122
use FreeDSx\Ldap\Operation\Response\SearchResultEntry;
23+
use FreeDSx\Ldap\Operation\ResultCode;
2224
use FreeDSx\Ldap\Protocol\LdapMessageRequest;
2325
use FreeDSx\Ldap\Protocol\LdapMessageResponse;
2426
use FreeDSx\Ldap\Protocol\Queue\ServerQueue;
@@ -209,13 +211,27 @@ public function it_throws_an_exception_if_the_old_and_new_paging_requests_are_di
209211
$pagingHandler->remove(Argument::any(), Argument::any())
210212
->shouldNotBeCalled();
211213

212-
$this->shouldThrow(new OperationException("The search request and controls must be identical between paging requests."))->during('handleRequest', [
214+
$queue->sendMessage(new LdapMessageResponse(
215+
$message->getMessageId(),
216+
new SearchResultDone(
217+
ResultCode::OPERATIONS_ERROR,
218+
'',
219+
"The search request and controls must be identical between paging requests."
220+
),
221+
...[new PagingControl(
222+
0,
223+
'bar'
224+
)]
225+
))->shouldBeCalled()
226+
->willReturn($queue);
227+
228+
$this->handleRequest(
213229
$message,
214230
$token,
215231
$handler,
216232
$queue,
217233
[]
218-
]);
234+
);
219235
}
220236

221237
public function it_throws_an_exception_if_the_paging_cookie_does_not_exist(

0 commit comments

Comments
 (0)