Skip to content

Commit df8dc31

Browse files
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 df8dc31

File tree

8 files changed

+303
-31
lines changed

8 files changed

+303
-31
lines changed
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 SearchResult
21+
{
22+
/**
23+
* @var Entries
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 ?? new 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/ServerPagingHandler.php

Lines changed: 43 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -80,16 +80,51 @@ public function handleRequest(
8080
$token
8181
);
8282
$pagingRequest = $this->findOrMakePagingRequest($message);
83+
$searchRequest = $this->getSearchRequestFromMessage($message);
8384

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

90115
$pagingRequest->markProcessed();
91116

92-
if ($response->isComplete()) {
117+
/**
118+
* Per Section 3 of RFC 2696:
119+
*
120+
* If, for any reason, the server cannot resume a paged search operation
121+
* for a client, then it SHOULD return the appropriate error in a
122+
* searchResultDone entry. If this occurs, both client and server should
123+
* assume the paged result set is closed and no longer resumable.
124+
*
125+
* If a search result is anything other than success, we remove the paging request.
126+
*/
127+
if (($response && $response->isComplete()) || $searchResult->getResultCode() !== ResultCode::SUCCESS) {
93128
$this->requestHistory->pagingRequest()
94129
->remove($pagingRequest);
95130
$this->pagingHandler->remove(
@@ -99,13 +134,10 @@ public function handleRequest(
99134
}
100135

101136
$this->sendEntriesToClient(
102-
$response->getEntries(),
137+
$searchResult,
103138
$message,
104139
$queue,
105-
new PagingControl(
106-
$response->getRemaining(),
107-
$response->isComplete() ? '' : $pagingRequest->getNextCookie()
108-
)
140+
...$controls
109141
);
110142
}
111143

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

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

63-
$entries = $dispatcher->search(
64-
$context,
65-
$request
66-
);
63+
try {
64+
$searchResult = SearchResult::makeSuccessResult(
65+
$dispatcher->search(
66+
$context,
67+
$request
68+
),
69+
(string) $request->getBaseDn()
70+
);
71+
} catch (OperationException $e) {
72+
$searchResult = SearchResult::makeErrorResult(
73+
$e->getCode(),
74+
(string) $request->getBaseDn(),
75+
$e->getMessage()
76+
);
77+
}
6778

6879
$this->sendEntriesToClient(
69-
$entries,
80+
$searchResult,
7081
$message,
7182
$queue
7283
);

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

Lines changed: 17 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,24 @@ public function handleRequest(
4243
);
4344
$request = $this->getSearchRequestFromMessage($message);
4445

45-
$entries = $dispatcher->search(
46-
$context,
47-
$request
48-
);
46+
try {
47+
$searchResult = SearchResult::makeSuccessResult(
48+
$dispatcher->search(
49+
$context,
50+
$request
51+
),
52+
(string) $request->getBaseDn()
53+
);
54+
} catch (OperationException $e) {
55+
$searchResult = SearchResult::makeErrorResult(
56+
$e->getCode(),
57+
(string) $request->getBaseDn(),
58+
$e->getMessage()
59+
);
60+
}
4961

5062
$this->sendEntriesToClient(
51-
$entries,
63+
$searchResult,
5264
$message,
5365
$queue
5466
);

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

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

4344
foreach ($entries->toArray() as $entry) {
4445
$messages[] = new LdapMessageResponse(
@@ -49,7 +50,11 @@ private function sendEntriesToClient(
4950

5051
$messages[] = new LdapMessageResponse(
5152
$message->getMessageId(),
52-
new SearchResultDone(ResultCode::SUCCESS),
53+
new SearchResultDone(
54+
$searchResult->getResultCode(),
55+
$searchResult->getBaseDn(),
56+
$searchResult->getDiagnosticMessage()
57+
),
5358
...$controls
5459
);
5560

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

Lines changed: 20 additions & 4 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;
@@ -191,7 +193,7 @@ public function it_should_send_the_correct_response_if_paging_is_abandoned(
191193
);
192194
}
193195

194-
public function it_throws_an_exception_if_the_old_and_new_paging_requests_are_different(
196+
public function it_sends_a_result_code_error_in_SearchResultDone_if_the_old_and_new_paging_requests_are_different(
195197
ServerQueue $queue,
196198
RequestHandlerInterface $handler,
197199
TokenInterface $token,
@@ -207,15 +209,29 @@ public function it_throws_an_exception_if_the_old_and_new_paging_requests_are_di
207209
$pagingHandler->page(Argument::any(), Argument::any())
208210
->shouldNotBeCalled();
209211
$pagingHandler->remove(Argument::any(), Argument::any())
210-
->shouldNotBeCalled();
212+
->shouldBeCalled();
213+
214+
$queue->sendMessage(new LdapMessageResponse(
215+
$message->getMessageId(),
216+
new SearchResultDone(
217+
ResultCode::OPERATIONS_ERROR,
218+
'dc=foo,dc=bar',
219+
"The search request and controls must be identical between paging requests."
220+
),
221+
...[new PagingControl(
222+
0,
223+
''
224+
)]
225+
))->shouldBeCalled()
226+
->willReturn($queue);
211227

212-
$this->shouldThrow(new OperationException("The search request and controls must be identical between paging requests."))->during('handleRequest', [
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)