Skip to content

Commit 783b6e7

Browse files
committed
Fix potential NPE in RemoteEndpoint.handleRequest
Fixes #890.
1 parent dee7e1b commit 783b6e7

File tree

4 files changed

+53
-3
lines changed

4 files changed

+53
-3
lines changed

org.eclipse.lsp4j.jsonrpc/src/main/java/org/eclipse/lsp4j/jsonrpc/Endpoint.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,22 @@
1818
*/
1919
public interface Endpoint {
2020

21+
/**
22+
* Accepts the given request for further processing.
23+
*
24+
* @param method a request method, may not be <code>null</code>
25+
* @param parameter a request parameter
26+
* @return an instance of CompletableFuture representing the result computed in response to the request.
27+
* May not be <code>null</code>.
28+
*/
2129
CompletableFuture<?> request(String method, Object parameter);
2230

31+
/**
32+
* Accepts the given notification for further processing.
33+
*
34+
* @param method a notification method, may not be <code>null</code>
35+
* @param parameter a notification parameter
36+
*/
2337
void notify(String method, Object parameter);
2438

2539
}

org.eclipse.lsp4j.jsonrpc/src/main/java/org/eclipse/lsp4j/jsonrpc/RemoteEndpoint.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,9 @@ protected void handleRequest(RequestMessage requestMessage) {
269269
try {
270270
// Forward the request to the local endpoint
271271
future = localEndpoint.request(requestMessage.getMethod(), requestMessage.getParams());
272+
if (future == null) {
273+
throw new IllegalStateException("Local endpoint returned null from its request method, whereas an instance of CompletableFuture is expected");
274+
}
272275
} catch (Throwable throwable) {
273276
// The local endpoint has failed handling the request - reply with an error response
274277
ResponseError errorObject = exceptionHandler.apply(throwable);

org.eclipse.lsp4j.jsonrpc/src/main/java/org/eclipse/lsp4j/jsonrpc/services/JsonRequest.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,13 @@
2121
* Annotation to mark a request method on an interface or class.
2222
* <p>
2323
* A request method must have the return type {@link CompletableFuture} with an
24-
* object parameter type or Void and have zero or one argument.
24+
* object parameter type or Void, and have zero or one argument.
2525
* <p>
26-
* According to jsonrpc an argument must be an 'object' (a java bean, not e,g.
26+
* According to jsonrpc an argument must be an 'object' (a java bean, not e.g. a
2727
* String).
2828
* <p>
29+
* A request method is not allowed to return <code>null</code>.
30+
* <p>
2931
* The name of the jsonrpc request will be the optional segment, followed by the
3032
* name of the Java method that is annotated with JsonRequest. The name of the
3133
* jsonrpc request can be customized by using the {@link #value()} field of this

org.eclipse.lsp4j.jsonrpc/src/test/java/org/eclipse/lsp4j/jsonrpc/test/RemoteEndpointTest.java

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -464,4 +464,35 @@ public CompletableFuture<Object> request(String method, Object parameter) {
464464
logMessages.unregister();
465465
}
466466
}
467-
}
467+
468+
@Test
469+
public void testEndpointReturningNull() {
470+
final var logMessages = new LogMessageAccumulator();
471+
try {
472+
// Don't show the exception in the test execution log
473+
logMessages.registerTo(RemoteEndpoint.class);
474+
475+
final var endp = new TestEndpoint() {
476+
@Override
477+
public CompletableFuture<Object> request(String method, Object parameter) {
478+
return null;
479+
}
480+
};
481+
final var consumer = new TestMessageConsumer();
482+
final var endpoint = new RemoteEndpoint(consumer, endp);
483+
484+
endpoint.consume(init(new RequestMessage(), it -> {
485+
it.setId("1");
486+
it.setMethod("foo");
487+
it.setParams("myparam");
488+
}));
489+
490+
assertEquals("Check some response received", 1, consumer.messages.size());
491+
final var response = (ResponseMessage) consumer.messages.get(0);
492+
assertNotNull("Check response has error", response.getError());
493+
assertEquals(ResponseErrorCode.InternalError.getValue(), response.getError().getCode());
494+
} finally {
495+
logMessages.unregister();
496+
}
497+
}
498+
}

0 commit comments

Comments
 (0)