Skip to content

Commit daf3cb3

Browse files
committed
[clangd][index-sever] Limit results in repsonse
This is to prevent server from being DOS'd by possible malicious parties issuing requests that can yield huge responses. One possible drawback is on rename workflow. As it really requests all occurences, but it has an internal limit on 50 files currently. We are putting the limit on 10000 elements per response So for rename to regress one should have 10k refs to a symbol in less than 50 files. This seems unlikely and we fix it if there are complaints by giving up on the response based on the number of files covered instead. Differential Revision: https://reviews.llvm.org/D101914
1 parent d69bccf commit daf3cb3

File tree

2 files changed

+69
-1
lines changed

2 files changed

+69
-1
lines changed

clang-tools-extra/clangd/index/remote/server/Server.cpp

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,12 @@ llvm::cl::opt<size_t> IdleTimeoutSeconds(
9393
llvm::cl::desc("Maximum time a channel may stay idle until server closes "
9494
"the connection, in seconds. Defaults to 480."));
9595

96+
llvm::cl::opt<size_t> LimitResults(
97+
"limit-results", llvm::cl::init(10000),
98+
llvm::cl::desc("Maximum number of results to stream as a response to "
99+
"single request. Limit is to keep the server from being "
100+
"DOS'd. Defaults to 10000."));
101+
96102
static Key<grpc::ServerContext *> CurrentRequest;
97103

98104
class RemoteIndexServer final : public v1::SymbolIndex::Service {
@@ -123,7 +129,12 @@ class RemoteIndexServer final : public v1::SymbolIndex::Service {
123129
}
124130
unsigned Sent = 0;
125131
unsigned FailedToSend = 0;
132+
bool HasMore = false;
126133
Index.lookup(*Req, [&](const clangd::Symbol &Item) {
134+
if (Sent >= LimitResults) {
135+
HasMore = true;
136+
return;
137+
}
127138
auto SerializedItem = ProtobufMarshaller->toProtobuf(Item);
128139
if (!SerializedItem) {
129140
elog("Unable to convert Symbol to protobuf: {0}",
@@ -137,8 +148,10 @@ class RemoteIndexServer final : public v1::SymbolIndex::Service {
137148
Reply->Write(NextMessage);
138149
++Sent;
139150
});
151+
if (HasMore)
152+
log("[public] Limiting result size for Lookup request.");
140153
LookupReply LastMessage;
141-
LastMessage.mutable_final_result()->set_has_more(true);
154+
LastMessage.mutable_final_result()->set_has_more(HasMore);
142155
logResponse(LastMessage);
143156
Reply->Write(LastMessage);
144157
SPAN_ATTACH(Tracer, "Sent", Sent);
@@ -160,6 +173,11 @@ class RemoteIndexServer final : public v1::SymbolIndex::Service {
160173
Req.takeError());
161174
return grpc::Status::CANCELLED;
162175
}
176+
if (!Req->Limit || *Req->Limit > LimitResults) {
177+
log("[public] Limiting result size for FuzzyFind request from {0} to {1}",
178+
Req->Limit, LimitResults);
179+
Req->Limit = LimitResults;
180+
}
163181
unsigned Sent = 0;
164182
unsigned FailedToSend = 0;
165183
bool HasMore = Index.fuzzyFind(*Req, [&](const clangd::Symbol &Item) {
@@ -197,6 +215,11 @@ class RemoteIndexServer final : public v1::SymbolIndex::Service {
197215
elog("Can not parse RefsRequest from protobuf: {0}", Req.takeError());
198216
return grpc::Status::CANCELLED;
199217
}
218+
if (!Req->Limit || *Req->Limit > LimitResults) {
219+
log("[public] Limiting result size for Refs request from {0} to {1}.",
220+
Req->Limit, LimitResults);
221+
Req->Limit = LimitResults;
222+
}
200223
unsigned Sent = 0;
201224
unsigned FailedToSend = 0;
202225
bool HasMore = Index.refs(*Req, [&](const clangd::Ref &Item) {
@@ -236,6 +259,12 @@ class RemoteIndexServer final : public v1::SymbolIndex::Service {
236259
Req.takeError());
237260
return grpc::Status::CANCELLED;
238261
}
262+
if (!Req->Limit || *Req->Limit > LimitResults) {
263+
log("[public] Limiting result size for Relations request from {0} to "
264+
"{1}.",
265+
Req->Limit, LimitResults);
266+
Req->Limit = LimitResults;
267+
}
239268
unsigned Sent = 0;
240269
unsigned FailedToSend = 0;
241270
Index.relations(
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# REQUIRES: clangd-remote-index
2+
# RUN: rm -rf %t
3+
# RUN: clangd-indexer %S/Inputs/Source.cpp > %t.idx
4+
# RUN: %python %S/pipeline_helper.py --input-file-name=%s --server-arg=--limit-results=1 --project-root=%S --index-file=%t.idx | FileCheck %s
5+
6+
{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
7+
---
8+
# Ensure there's a single result.
9+
{"jsonrpc":"2.0","id":1,"method":"workspace/symbol","params":{"query":"c"}}
10+
# CHECK: {
11+
# CHECK: "id": 1,
12+
# CHECK: "jsonrpc": "2.0",
13+
# CHECK: "result": [
14+
# CHECK: {
15+
# CHECK: "containerName": "{{.*}}",
16+
# CHECK: "kind": {{.*}},
17+
# CHECK: "location": {
18+
# CHECK: "range": {
19+
# CHECK: "end": {
20+
# CHECK: "character": {{.*}},
21+
# CHECK: "line": {{.*}}
22+
# CHECK: },
23+
# CHECK: "start": {
24+
# CHECK: "character": {{.*}},
25+
# CHECK: "line": {{.*}}
26+
# CHECK: }
27+
# CHECK: },
28+
# CHECK: "uri": "{{.*}}"
29+
# CHECK: },
30+
# CHECK: "name": "{{.*}}",
31+
# CHECK: "score": {{.*}}
32+
# CHECK: }
33+
# CHECK: ]
34+
# CHECK: }
35+
---
36+
{"jsonrpc":"2.0","id":4,"method":"shutdown"}
37+
---
38+
{"jsonrpc":"2.0","method":"exit"}
39+

0 commit comments

Comments
 (0)