clangd: Tolerate additional headers
Summary:
The language server protocol specified 2 headers (Content-Length and Content-Type), but does not specify their sequence. It specifies that an empty line ends
headers. Clangd has been updated to handle arbitrary sequences of headers, extracting only the content length.
Patch by puremourning (Ben Jackson).
Reviewers: bkramer, klimek, ilya-biryukov
Reviewed By: ilya-biryukov
Subscribers: cfe-commits, ilya-biryukov
Differential Revision: https://reviews.llvm.org/D37282
llvm-svn: 312483
diff --git a/clang-tools-extra/clangd/JSONRPCDispatcher.cpp b/clang-tools-extra/clangd/JSONRPCDispatcher.cpp
index 5deb75a..2f38e9c 100644
--- a/clang-tools-extra/clangd/JSONRPCDispatcher.cpp
+++ b/clang-tools-extra/clangd/JSONRPCDispatcher.cpp
@@ -136,46 +136,68 @@
JSONRPCDispatcher &Dispatcher,
bool &IsDone) {
while (In.good()) {
- // A Language Server Protocol message starts with a HTTP header, delimited
- // by \r\n.
- std::string Line;
- std::getline(In, Line);
- if (!In.good() && errno == EINTR) {
- In.clear();
- continue;
+ // A Language Server Protocol message starts with a set of HTTP headers,
+ // delimited by \r\n, and terminated by an empty line (\r\n).
+ unsigned long long ContentLength = 0;
+ while (In.good()) {
+ std::string Line;
+ std::getline(In, Line);
+ if (!In.good() && errno == EINTR) {
+ In.clear();
+ continue;
+ }
+
+ llvm::StringRef LineRef(Line);
+
+ // We allow YAML-style comments in headers. Technically this isn't part
+ // of the LSP specification, but makes writing tests easier.
+ if (LineRef.startswith("#"))
+ continue;
+
+ // Content-Type is a specified header, but does nothing.
+ // Content-Length is a mandatory header. It specifies the length of the
+ // following JSON.
+ // It is unspecified what sequence headers must be supplied in, so we
+ // allow any sequence.
+ // The end of headers is signified by an empty line.
+ if (LineRef.consume_front("Content-Length: ")) {
+ if (ContentLength != 0) {
+ Out.log("Warning: Duplicate Content-Length header received. "
+ "The previous value for this message ("
+ + std::to_string(ContentLength)
+ + ") was ignored.\n");
+ }
+
+ llvm::getAsUnsignedInteger(LineRef.trim(), 0, ContentLength);
+ continue;
+ } else if (!LineRef.trim().empty()) {
+ // It's another header, ignore it.
+ continue;
+ } else {
+ // An empty line indicates the end of headers.
+ // Go ahead and read the JSON.
+ break;
+ }
}
- // Skip empty lines.
- llvm::StringRef LineRef(Line);
- if (LineRef.trim().empty())
- continue;
+ if (ContentLength > 0) {
+ // Now read the JSON. Insert a trailing null byte as required by the YAML
+ // parser.
+ std::vector<char> JSON(ContentLength + 1, '\0');
+ In.read(JSON.data(), ContentLength);
- // We allow YAML-style comments. Technically this isn't part of the
- // LSP specification, but makes writing tests easier.
- if (LineRef.startswith("#"))
- continue;
+ // If the stream is aborted before we read ContentLength bytes, In
+ // will have eofbit and failbit set.
+ if (!In) {
+ Out.log("Input was aborted. Read only "
+ + std::to_string(In.gcount())
+ + " bytes of expected "
+ + std::to_string(ContentLength)
+ + ".\n");
+ break;
+ }
- unsigned long long Len = 0;
- // FIXME: Content-Type is a specified header, but does nothing.
- // Content-Length is a mandatory header. It specifies the length of the
- // following JSON.
- if (LineRef.consume_front("Content-Length: "))
- llvm::getAsUnsignedInteger(LineRef.trim(), 0, Len);
-
- // Check if the next line only contains \r\n. If not this is another header,
- // which we ignore.
- char NewlineBuf[2];
- In.read(NewlineBuf, 2);
- if (std::memcmp(NewlineBuf, "\r\n", 2) != 0)
- continue;
-
- // Now read the JSON. Insert a trailing null byte as required by the YAML
- // parser.
- std::vector<char> JSON(Len + 1, '\0');
- In.read(JSON.data(), Len);
-
- if (Len > 0) {
- llvm::StringRef JSONRef(JSON.data(), Len);
+ llvm::StringRef JSONRef(JSON.data(), ContentLength);
// Log the message.
Out.log("<-- " + JSONRef + "\n");
@@ -186,6 +208,9 @@
// If we're done, exit the loop.
if (IsDone)
break;
+ } else {
+ Out.log( "Warning: Missing Content-Length header, or message has zero "
+ "length.\n" );
}
}
}
diff --git a/clang-tools-extra/test/clangd/protocol.test b/clang-tools-extra/test/clangd/protocol.test
new file mode 100644
index 0000000..42dbff7
--- /dev/null
+++ b/clang-tools-extra/test/clangd/protocol.test
@@ -0,0 +1,82 @@
+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# RUN: clangd -run-synchronously < %s 2>&1 | FileCheck -check-prefix=STDERR %s
+# vim: fileformat=dos
+# It is absolutely vital that this file has CRLF line endings.
+#
+# Test protocol parsing
+Content-Length: 125
+Content-Type: application/vscode-jsonrpc; charset-utf-8
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+# Test message with Content-Type after Content-Length
+#
+# CHECK: "jsonrpc":"2.0","id":0,"result":{"capabilities":{
+# CHECK-DAG: "textDocumentSync": 1,
+# CHECK-DAG: "documentFormattingProvider": true,
+# CHECK-DAG: "documentRangeFormattingProvider": true,
+# CHECK-DAG: "documentOnTypeFormattingProvider": {"firstTriggerCharacter":"}","moreTriggerCharacter":[]},
+# CHECK-DAG: "codeActionProvider": true,
+# CHECK-DAG: "completionProvider": {"resolveProvider": false, "triggerCharacters": [".",">",":"]},
+# CHECK-DAG: "definitionProvider": true
+# CHECK: }}
+
+Content-Length: 246
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"struct fake { int a, bb, ccc; int f(int i, const float f) const; };\nint main() {\n fake f;\n f.\n}\n"}}}
+
+Content-Type: application/vscode-jsonrpc; charset-utf-8
+Content-Length: 146
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:/main.cpp"},"position":{"line":3,"character":5}}}
+# Test message with Content-Type before Content-Length
+#
+# CHECK: {"jsonrpc":"2.0","id":1,"result":[
+# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a"}
+# CHECK: ]}
+
+X-Test: Testing
+Content-Type: application/vscode-jsonrpc; charset-utf-8
+Content-Length: 146
+Content-Type: application/vscode-jsonrpc; charset-utf-8
+X-Testing: Test
+
+{"jsonrpc":"2.0","id":2,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:/main.cpp"},"position":{"line":3,"character":5}}}
+
+Content-Type: application/vscode-jsonrpc; charset-utf-8
+Content-Length: 10
+Content-Length: 146
+
+{"jsonrpc":"2.0","id":3,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:/main.cpp"},"position":{"line":3,"character":5}}}
+# Test message with duplicate Content-Length headers
+#
+# CHECK: {"jsonrpc":"2.0","id":3,"result":[
+# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a"}
+# CHECK: ]}
+# STDERR: Warning: Duplicate Content-Length header received. The previous value for this message (10) was ignored.
+
+Content-Type: application/vscode-jsonrpc; charset-utf-8
+Content-Length: 10
+
+{"jsonrpc":"2.0","id":4,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:/main.cpp"},"position":{"line":3,"character":5}}}
+# Test message with malformed Content-Length
+#
+# STDERR: JSON dispatch failed!
+# Ensure we recover by sending another (valid) message
+
+Content-Length: 146
+
+{"jsonrpc":"2.0","id":5,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:/main.cpp"},"position":{"line":3,"character":5}}}
+# Test message with Content-Type before Content-Length
+#
+# CHECK: {"jsonrpc":"2.0","id":5,"result":[
+# CHECK-DAG: {"label":"a","kind":5,"detail":"int","sortText":"00035a","filterText":"a","insertText":"a"}
+# CHECK: ]}
+
+Content-Length: 1024
+
+{"jsonrpc":"2.0","id":5,"method":"textDocument/completion","params":{"textDocument":{"uri":"file:/main.cpp"},"position":{"line":3,"character":5}}}
+# Test message which reads beyond the end of the stream.
+#
+# Ensure this is the last test in the file!
+# STDERR: Input was aborted. Read only {{[0-9]+}} bytes of expected {{[0-9]+}}.
+