Skip to content

Commit b8a544f

Browse files
Merge branch 'master' into fix/server/invalid-params
2 parents 86226b7 + 82ce908 commit b8a544f

File tree

8 files changed

+39
-43
lines changed

8 files changed

+39
-43
lines changed

src/Client/Dispatcher/LspDispatcher.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public async Task<bool> TryHandleEmptyNotification(string method)
9898
/// <returns>
9999
/// <c>true</c>, if a notification handler was registered for specified method; otherwise, <c>false</c>.
100100
/// </returns>
101-
public async Task<bool> TryHandleNotification(string method, JObject notification)
101+
public async Task<bool> TryHandleNotification(string method, JToken notification)
102102
{
103103
if (string.IsNullOrWhiteSpace(method))
104104
throw new ArgumentException($"Argument cannot be null, empty, or entirely composed of whitespace: {nameof(method)}.", nameof(method));
@@ -130,7 +130,7 @@ public async Task<bool> TryHandleNotification(string method, JObject notificatio
130130
/// <returns>
131131
/// If a registered handler was found, a <see cref="Task"/> representing the operation; otherwise, <c>null</c>.
132132
/// </returns>
133-
public Task<object> TryHandleRequest(string method, JObject request, CancellationToken cancellationToken)
133+
public Task<object> TryHandleRequest(string method, JToken request, CancellationToken cancellationToken)
134134
{
135135
if (string.IsNullOrWhiteSpace(method))
136136
throw new ArgumentException($"Argument cannot be null, empty, or entirely composed of whitespace: {nameof(method)}.", nameof(method));
@@ -157,7 +157,7 @@ public Task<object> TryHandleRequest(string method, JObject request, Cancellatio
157157
/// <returns>
158158
/// The deserialised payload (if one is present and expected).
159159
/// </returns>
160-
object DeserializePayload(Type payloadType, JObject payload)
160+
object DeserializePayload(Type payloadType, JToken payload)
161161
{
162162
if (payloadType == null)
163163
throw new ArgumentNullException(nameof(payloadType));

src/Client/Protocol/ServerMessage.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,13 @@ public class ServerMessage
3131
/// The request / notification message, if the message represents a request or a notification.
3232
/// </summary>
3333
[Optional]
34-
public JObject Params { get; set; }
34+
public JToken Params { get; set; }
3535

3636
/// <summary>
3737
/// The response message, if the message represents a response.
3838
/// </summary>
3939
[Optional]
40-
public JObject Result { get; set; }
40+
public JToken Result { get; set; }
4141

4242
/// <summary>
4343
/// The response error (if any).

src/JsonRpc/Reciever.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ protected virtual Renor GetRenor(JToken @object)
8888
// that we don't fall over and throw an error.
8989
if (@params?.Type == JTokenType.Null)
9090
{
91-
@params = null;
91+
@params = new JObject();
9292
}
9393

9494
// id == request

src/JsonRpc/ReflectionRequestHandlers.cs

+2-10
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using System.Reflection;
1+
using System.Reflection;
22
using System.Threading;
33
using System.Threading.Tasks;
44

@@ -22,14 +22,6 @@ public static Task HandleNotification(IHandlerDescriptor instance, object @param
2222
return (Task)method.Invoke(instance.Handler, new[] { @params });
2323
}
2424

25-
public static Task HandleRequest(IHandlerDescriptor instance, CancellationToken token)
26-
{
27-
var method = instance.HandlerType.GetTypeInfo()
28-
.GetMethod(nameof(IRequestHandler<object>.Handle), BindingFlags.Public | BindingFlags.Instance);
29-
30-
return (Task)method.Invoke(instance.Handler, new object[] { token });
31-
}
32-
3325
public static Task HandleRequest(IHandlerDescriptor instance, object @params, CancellationToken token)
3426
{
3527
var method = instance.HandlerType.GetTypeInfo()
@@ -38,4 +30,4 @@ public static Task HandleRequest(IHandlerDescriptor instance, object @params, Ca
3830
return (Task)method.Invoke(instance.Handler, new[] { @params, token });
3931
}
4032
}
41-
}
33+
}

src/JsonRpc/RequestRouter.cs

+1-9
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,6 @@ protected virtual async Task<ErrorResponse> RouteRequest(IHandlerDescriptor hand
5656
return new MethodNotFound(request.Id, request.Method);
5757
}
5858

59-
Task result;
60-
if (handler.Params is null)
61-
{
62-
result = ReflectionRequestHandlers.HandleRequest(handler, token);
63-
}
64-
else
65-
{
6659
object @params;
6760
try
6861
{
@@ -73,8 +66,7 @@ protected virtual async Task<ErrorResponse> RouteRequest(IHandlerDescriptor hand
7366
return new InvalidParams(request.Id);
7467
}
7568

76-
result = ReflectionRequestHandlers.HandleRequest(handler, @params, token);
77-
}
69+
var result = ReflectionRequestHandlers.HandleRequest(handler, @params, token);
7870

7971
await result.ConfigureAwait(false);
8072

src/Server/LspRequestRouter.cs

+9-16
Original file line numberDiff line numberDiff line change
@@ -111,27 +111,20 @@ public async Task<ErrorResponse> RouteRequest(IHandlerDescriptor descriptor, Req
111111
return new MethodNotFound(request.Id, request.Method);
112112
}
113113

114-
Task result;
115-
if (descriptor.Params is null)
114+
object @params;
115+
try
116116
{
117-
result = ReflectionRequestHandlers.HandleRequest(descriptor, cts.Token);
117+
@params = request.Params?.ToObject(descriptor.Params, _serializer.JsonSerializer);
118118
}
119-
else
119+
catch (Exception cannotDeserializeRequestParams)
120120
{
121-
object @params;
122-
try
123-
{
124-
@params = request.Params?.ToObject(descriptor.Params, _serializer.JsonSerializer);
125-
}
126-
catch
127-
{
128-
return new InvalidParams(request.Id);
129-
}
130-
131-
result = ReflectionRequestHandlers.HandleRequest(descriptor, @params, cts.Token);
121+
_logger.LogError(new EventId(-32602), cannotDeserializeRequestParams, "Failed to deserialise request parameters.");
122+
123+
return new InvalidParams(request.Id);
132124
}
133125

134-
await result.ConfigureAwait(false);
126+
var result = ReflectionRequestHandlers.HandleRequest(descriptor, @params, cts.Token).ConfigureAwait(false);
127+
await result;
135128

136129
object responseValue = null;
137130
if (result.GetType().GetTypeInfo().IsGenericType)

test/JsonRpc.Tests/Server/SpecifictionRecieverTests.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public override IEnumerable<ValueTuple<string, Renor[]>> GetValues()
7171
@"{""jsonrpc"": ""2.0"", ""method"": ""subtract"", ""params"": null, ""id"": 4}",
7272
new Renor[]
7373
{
74-
new Request(4, "subtract", null)
74+
new Request(4, "subtract", new JObject())
7575
});
7676

7777
yield return (
@@ -96,7 +96,7 @@ public override IEnumerable<ValueTuple<string, Renor[]>> GetValues()
9696
@"{""jsonrpc"": ""2.0"", ""method"": ""foobar"", ""params"": null}",
9797
new Renor[]
9898
{
99-
new Notification("foobar", null)
99+
new Notification("foobar", new JObject())
100100
});
101101

102102
yield return (

test/Lsp.Tests/LspRequestRouterTests.cs

+19
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,25 @@ public async Task ShouldRouteToCorrect_Request_WithManyHandlers()
146146
await codeActionHandler2.Received(0).Handle(Arg.Any<CodeActionParams>(), Arg.Any<CancellationToken>());
147147
}
148148

149+
[Fact]
150+
public async Task ShouldRouteTo_CorrectRequestWhenGivenNullParams()
151+
{
152+
var handler = Substitute.For<IShutdownHandler>();
153+
handler
154+
.Handle(Arg.Any<object>(), Arg.Any<CancellationToken>())
155+
.Returns(Task.CompletedTask);
156+
157+
var collection = new HandlerCollection { handler };
158+
var mediator = new LspRequestRouter(collection, _testLoggerFactory, _handlerMatcherCollection, new Serializer());
159+
160+
var id = Guid.NewGuid().ToString();
161+
var request = new Request(id, GeneralNames.Shutdown, new JObject());
162+
163+
await mediator.RouteRequest(mediator.GetDescriptor(request), request);
164+
165+
await handler.Received(1).Handle(Arg.Any<object>(), Arg.Any<CancellationToken>());
166+
}
167+
149168
[Fact]
150169
public async Task ShouldHandle_Request_WithNullParameters()
151170
{

0 commit comments

Comments
 (0)