Skip to content

Commit 4b6d14b

Browse files
sebthompisv
authored andcommitted
fix(jsonrpc): correct newline state machine in StreamMessageProducer
The listener sets `newLine = false` after parsing a blank line (end-of-headers) and then **unconditionally** sets it back to `true`, leaving the state inconsistent. This caused a lone trailing CRLF between messages to be misinterpreted as an additional blank line, triggering a "Missing header Content-Length" error via `fireError`. This fix prevents spurious errors and ensures robust parsing across message boundaries.
1 parent 33a6e15 commit 4b6d14b

File tree

2 files changed

+59
-16
lines changed

2 files changed

+59
-16
lines changed

org.eclipse.lsp4j.jsonrpc/src/main/java/org/eclipse/lsp4j/jsonrpc/json/StreamMessageProducer.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,14 @@ public void listen(MessageConsumer callback) {
101101
}
102102
headers = new Headers();
103103
debugBuilder = null;
104-
} else if (headerBuilder != null) {
105-
// A single newline ends a header line
106-
parseHeader(headerBuilder.toString(), headers);
107-
headerBuilder = null;
104+
} else {
105+
if (headerBuilder != null) {
106+
// A single newline ends a header line
107+
parseHeader(headerBuilder.toString(), headers);
108+
headerBuilder = null;
109+
}
110+
newLine = true;
108111
}
109-
newLine = true;
110112
} else if (c != '\r') {
111113
// Add the input to the current header line
112114
if (headerBuilder == null)

org.eclipse.lsp4j.jsonrpc/src/test/java/org/eclipse/lsp4j/jsonrpc/test/json/MessageProducerTest.java

Lines changed: 52 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,25 @@
11
/******************************************************************************
22
* Copyright (c) 2018 TypeFox and others.
3-
*
3+
*
44
* This program and the accompanying materials are made available under the
55
* terms of the Eclipse Public License v. 2.0 which is available at
66
* http://www.eclipse.org/legal/epl-2.0,
77
* or the Eclipse Distribution License v. 1.0 which is available at
88
* http://www.eclipse.org/org/documents/edl-v10.php.
9-
*
9+
*
1010
* SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause
1111
******************************************************************************/
1212
package org.eclipse.lsp4j.jsonrpc.test.json;
1313

14+
import static org.junit.Assert.assertEquals;
15+
16+
import java.io.ByteArrayInputStream;
1417
import java.io.IOException;
1518
import java.io.InputStream;
1619
import java.io.InterruptedIOException;
1720
import java.net.SocketException;
1821
import java.nio.channels.ClosedChannelException;
22+
import java.util.ArrayList;
1923
import java.util.Collections;
2024
import java.util.concurrent.ExecutionException;
2125
import java.util.concurrent.ExecutorService;
@@ -32,27 +36,27 @@
3236
import org.junit.Test;
3337

3438
public class MessageProducerTest {
35-
39+
3640
private static final long TIMEOUT = 2000;
37-
41+
3842
private ExecutorService executorService;
3943
private Level logLevel;
40-
44+
4145
@Before
4246
public void setup() {
4347
executorService = Executors.newCachedThreadPool();
4448
Logger logger = Logger.getLogger(StreamMessageProducer.class.getName());
4549
logLevel = logger.getLevel();
4650
logger.setLevel(Level.SEVERE);
4751
}
48-
52+
4953
@After
5054
public void teardown() {
5155
executorService.shutdown();
5256
Logger logger = Logger.getLogger(StreamMessageProducer.class.getName());
5357
logger.setLevel(logLevel);
5458
}
55-
59+
5660
@Test
5761
public void testStopOnInterrrupt() throws Exception {
5862
executorService.submit(() -> {
@@ -68,7 +72,7 @@ public int read() throws IOException {
6872
messageProducer.close();
6973
}).get(TIMEOUT, TimeUnit.MILLISECONDS);
7074
}
71-
75+
7276
@Test
7377
public void testStopOnChannelClosed() throws Exception {
7478
executorService.submit(() -> {
@@ -84,7 +88,7 @@ public int read() throws IOException {
8488
messageProducer.close();
8589
}).get(TIMEOUT, TimeUnit.MILLISECONDS);
8690
}
87-
91+
8892
@Test
8993
public void testStopOnSocketClosed() throws Throwable {
9094
executorService.submit(() -> {
@@ -100,8 +104,8 @@ public int read() throws IOException {
100104
messageProducer.close();
101105
}).get(TIMEOUT, TimeUnit.MILLISECONDS);
102106
}
103-
104-
@Test(expected=JsonRpcException.class)
107+
108+
@Test(expected = JsonRpcException.class)
105109
public void testIOException() throws Throwable {
106110
try {
107111
executorService.submit(() -> {
@@ -121,4 +125,41 @@ public int read() throws IOException {
121125
}
122126
}
123127

128+
@Test
129+
public void testSpuriousSingleCRLFBetweenTwoMessages() throws Exception {
130+
executorService.submit(() -> {
131+
// Two valid framed messages with a single extra CRLF inserted between
132+
// the end of content of the first and the start of the next header block.
133+
String msg1 = "{\"jsonrpc\":\"2.0\",\"method\":\"ping\",\"params\":null}";
134+
String msg2 = "{\"jsonrpc\":\"2.0\",\"method\":\"pong\",\"params\":null}";
135+
String inputStr = header(msg1.length()) + msg1
136+
+ "\r\n" // single extra newline between messages
137+
+ header(msg2.length()) + msg2;
138+
139+
// Override fireError to surface the bug as a failure if triggered.
140+
class TestProducer extends StreamMessageProducer {
141+
public TestProducer(InputStream input, MessageJsonHandler jsonHandler) {
142+
super(input, jsonHandler);
143+
}
144+
145+
@Override
146+
protected void fireError(Throwable error) {
147+
throw new AssertionError("fireError was called: " + error, error);
148+
}
149+
}
150+
151+
var jsonHandler = new MessageJsonHandler(Collections.emptyMap());
152+
var producer = new TestProducer( new ByteArrayInputStream(inputStr.getBytes()), jsonHandler);
153+
var received = new ArrayList<>();
154+
producer.listen(received::add);
155+
producer.close();
156+
157+
assertEquals("Both messages should be delivered", 2, received.size());
158+
}).get(TIMEOUT, TimeUnit.MILLISECONDS);
159+
}
160+
161+
private static String header(int contentLength) {
162+
return "Content-Length: " + contentLength + "\r\n\r\n";
163+
}
164+
124165
}

0 commit comments

Comments
 (0)