Merge pull request #12 from Lercher/omnisharp-content-length

BugFix: content-length is counted in bytes

Author: david-driscoll

sample/SampleServer/Program.cs

@@ -24,7 +24,7 @@ static async Task MainAsync(string[] args)
             //    await Task.Delay(100);
             //}
 
-            var server = new LanguageServer(Console.In, Console.Out);
+            var server = new LanguageServer(Console.OpenStandardInput(), Console.OpenStandardOutput());
 
             server.AddHandler(new TextDocumentHandler(server));
 

src/JsonRpc/Connection.cs

@@ -18,7 +18,7 @@ public class Connection : IDisposable
         private readonly IRequestRouter _requestRouter;
 
         public Connection(
-            TextReader input,
+            Stream input,
             IOutputHandler outputHandler,
             IReciever reciever,
             IRequestProcessIdentifier requestProcessIdentifier,

src/JsonRpc/InputHandler.cs

@@ -16,7 +16,7 @@ public class InputHandler : IInputHandler
         public static char[] HeaderKeys = { CR, LF, ':' };
         public const short MinBuffer = 21; // Minimum size of the buffer "Content-Length: X\r\n\r\n"
 
-        private readonly TextReader _input;
+        private readonly Stream _input;
         private readonly IOutputHandler _outputHandler;
         private readonly IReciever _reciever;
         private readonly IRequestProcessIdentifier _requestProcessIdentifier;
@@ -26,14 +26,15 @@ public class InputHandler : IInputHandler
         private readonly IScheduler _scheduler;
 
         public InputHandler(
-            TextReader input,
+            Stream input,
             IOutputHandler outputHandler,
             IReciever reciever,
             IRequestProcessIdentifier requestProcessIdentifier,
             IRequestRouter requestRouter,
             IResponseRouter responseRouter
             )
         {
+            if (!input.CanRead) throw new ArgumentException($"must provide a readable stream for {nameof(input)}", nameof(input));
             _input = input;
             _outputHandler = outputHandler;
             _reciever = reciever;
@@ -54,23 +55,26 @@ public void Start()
 
         private async void ProcessInputStream()
         {
+            // header is encoded in ASCII
+            // "Content-Length: 0" counts bytes for the following content
+            // content is encoded in UTF-8
             while (true)
             {
                 if (_inputThread == null) return;
 
-                var buffer = new char[300];
-                var current = await _input.ReadBlockAsync(buffer, 0, MinBuffer);
+                var buffer = new byte[300];
+                var current = await _input.ReadAsync(buffer, 0, MinBuffer);
                 if (current == 0) return; // no more _input
-
-                while (current < MinBuffer || buffer[current - 4] != CR || buffer[current - 3] != LF ||
+                while (current < MinBuffer || 
+                       buffer[current - 4] != CR || buffer[current - 3] != LF ||
                        buffer[current - 2] != CR || buffer[current - 1] != LF)
                 {
-                    var n = await _input.ReadBlockAsync(buffer, current, 1);
+                    var n = await _input.ReadAsync(buffer, current, 1);
                     if (n == 0) return; // no more _input, mitigates endless loop here.
                     current += n;
                 }
 
-                var headersContent = new string(buffer, 0, current);
+                var headersContent = System.Text.Encoding.ASCII.GetString(buffer, 0, current);
                 var headers = headersContent.Split(HeaderKeys, StringSplitOptions.RemoveEmptyEntries);
                 long length = 0;
                 for (var i = 1; i < headers.Length; i += 2)
@@ -91,15 +95,16 @@ private async void ProcessInputStream()
                 }
                 else
                 {
-                    var requestBuffer = new char[length];
+                    var requestBuffer = new byte[length];
                     var received = 0;
                     while (received < length)
                     {
-                        var n = await _input.ReadBlockAsync(requestBuffer, received, requestBuffer.Length - received);
+                        var n = await _input.ReadAsync(requestBuffer, received, requestBuffer.Length - received);
                         if (n == 0) return; // no more _input
                         received += n;
                     }
-                    var payload = new string(requestBuffer);
+                    // TODO sometimes: encoding should be based on the respective header (including the wrong "utf8" value)
+                    var payload = System.Text.Encoding.UTF8.GetString(requestBuffer); 
                     HandleRequest(payload);
                 }
             }

src/JsonRpc/OutputHandler.cs

@@ -9,13 +9,14 @@ namespace JsonRpc
 {
     public class OutputHandler : IOutputHandler
     {
-        private readonly TextWriter _output;
+        private readonly Stream _output;
         private readonly Thread _thread;
         private readonly BlockingCollection<object> _queue;
         private readonly CancellationTokenSource _cancel;
 
-        public OutputHandler(TextWriter output)
+        public OutputHandler(Stream output)
         {
+            if (!output.CanWrite) throw new ArgumentException($"must provide a writable stream for {nameof(output)}", nameof(output));
             _output = output;
             _queue = new BlockingCollection<object>();
             _cancel = new CancellationTokenSource();
@@ -42,13 +43,21 @@ private void ProcessOutputQueue()
                     if (_queue.TryTake(out var value, Timeout.Infinite, token))
                     {
                         var content = JsonConvert.SerializeObject(value);
+                        var contentBytes = System.Text.Encoding.UTF8.GetBytes(content);
 
                         // TODO: Is this lsp specific??
                         var sb = new StringBuilder();
-                        sb.Append($"Content-Length: {content.Length}\r\n");
+                        sb.Append($"Content-Length: {contentBytes.Length}\r\n");
                         sb.Append($"\r\n");
-                        sb.Append(content);
-                        _output.Write(sb.ToString());
+                        var headerBytes = System.Text.Encoding.UTF8.GetBytes(sb.ToString());
+
+                        // only one write to _output
+                        using (var ms = new MemoryStream(headerBytes.Length + contentBytes.Length))
+                        {
+                            ms.Write(headerBytes, 0, headerBytes.Length);
+                            ms.Write(contentBytes, 0, contentBytes.Length);
+                            _output.Write(ms.ToArray(), 0, (int)ms.Position);
+                        }
                     }
                 }
             }

src/Lsp/LanguageServer.cs

@@ -29,13 +29,13 @@ public class LanguageServer : ILanguageServer, IInitializeHandler, IInitializedH
         private readonly TaskCompletionSource<InitializeResult> _initializeComplete = new TaskCompletionSource<InitializeResult>();
         private readonly CompositeDisposable _disposable = new CompositeDisposable();
 
-        public LanguageServer(TextReader input, TextWriter output)
+        public LanguageServer(Stream input, Stream output)
             : this(input, new OutputHandler(output), new LspReciever(), new RequestProcessIdentifier())
         {
         }
 
         internal LanguageServer(
-            TextReader input,
+            Stream input,
             IOutputHandler output,
             LspReciever reciever,
             IRequestProcessIdentifier requestProcessIdentifier

test/JsonRpc.Tests/InputHandlerTests.cs

@@ -19,7 +19,7 @@ namespace JsonRpc.Tests
     public class InputHandlerTests
     {
         private static InputHandler NewHandler(
-            TextReader inputStream,
+            Stream inputStream,
             IOutputHandler outputHandler,
             IReciever reciever,
             IRequestProcessIdentifier requestProcessIdentifier,
@@ -53,7 +53,7 @@ public void ShouldPassInRequests()
             var reciever = Substitute.For<IReciever>();
 
             using (NewHandler(
-                new StreamReader(inputStream),
+                inputStream,
                 outputHandler,
                 reciever,
                 Substitute.For<IRequestProcessIdentifier>(),
@@ -70,6 +70,32 @@ public void ShouldPassInRequests()
             }
         }
 
+        [Fact]
+        public void ShouldPassInUtf8EncodedRequests()
+        {
+            // Note: an ä (&auml;) is encoded by two bytes, so string-length is 13 and byte-length is 14
+            var inputStream = new MemoryStream(Encoding.UTF8.GetBytes("Content-Length: 14\r\n\r\n{\"utf8\": \"ä\"}"));
+            var outputHandler = Substitute.For<IOutputHandler>();
+            var reciever = Substitute.For<IReciever>();
+
+            using (NewHandler(
+                inputStream,
+                outputHandler,
+                reciever,
+                Substitute.For<IRequestProcessIdentifier>(),
+                Substitute.For<IRequestRouter>(),
+                Substitute.For<IResponseRouter>(),
+                cts => {
+                    reciever.When(x => x.IsValid(Arg.Any<JToken>()))
+                        .Do(x => {
+                            cts.Cancel();
+                        });
+                }))
+            {
+                reciever.Received().IsValid(Arg.Is<JToken>(x => x["utf8"].ToString() == "ä"));
+            }
+        }
+
         [Fact]
         public void ShouldHandleRequest()
         {
@@ -89,7 +115,7 @@ public void ShouldHandleRequest()
                 .Returns(response);
 
             using (NewHandler(
-                new StreamReader(inputStream),
+                inputStream,
                 outputHandler,
                 reciever,
                 Substitute.For<IRequestProcessIdentifier>(),
@@ -121,7 +147,7 @@ public void ShouldHandleError()
 
 
             using (NewHandler(
-                new StreamReader(inputStream),
+                inputStream,
                 outputHandler,
                 reciever,
                 Substitute.For<IRequestProcessIdentifier>(),
@@ -152,7 +178,7 @@ public void ShouldHandleNotification()
                 .Returns(c => (new Renor[] { notification }, false));
 
             using (NewHandler(
-                new StreamReader(inputStream),
+                inputStream,
                 outputHandler,
                 reciever,
                 Substitute.For<IRequestProcessIdentifier>(),
@@ -186,7 +212,7 @@ public void ShouldHandleResponse()
             responseRouter.GetRequest(1L).Returns(tcs);
 
             using (NewHandler(
-                new StreamReader(inputStream),
+                inputStream,
                 outputHandler,
                 reciever,
                 Substitute.For<IRequestProcessIdentifier>(),

test/JsonRpc.Tests/OutputHandlerTests.cs

@@ -3,20 +3,21 @@
 using System.Threading;
 using System.Threading.Tasks;
 using NSubstitute;
+using FluentAssertions;
 using Xunit;
 
 namespace JsonRpc.Tests
 {
     public class OutputHandlerTests
     {
-        private static (OutputHandler handler, Func<Task> wait) NewHandler(TextWriter textWriter,Action<CancellationTokenSource> action)
+        private static (OutputHandler handler, Func<Task> wait) NewHandler(Stream Writer,Action<CancellationTokenSource> action)
         {
             var cts = new CancellationTokenSource();
             if (!System.Diagnostics.Debugger.IsAttached)
                 cts.CancelAfter(TimeSpan.FromSeconds(120));
             action(cts);
 
-            var handler = new OutputHandler(textWriter);
+            var handler = new OutputHandler(Writer);
             handler.Start();
             return (handler, () => {
                 cts.Wait();
@@ -27,11 +28,16 @@ private static (OutputHandler handler, Func<Task> wait) NewHandler(TextWriter te
         [Fact]
         public async Task ShouldSerializeValues()
         {
-            var tw = Substitute.For<TextWriter>();
+            var w = Substitute.For<Stream>();
+            var received = "";
+            w.CanWrite.Returns(true);
 
-            var (handler, wait) = NewHandler(tw, cts => {
-                tw.When(x => x.Write(Arg.Any<string>()))
-                    .Do(c => cts.Cancel());
+            var (handler, wait) = NewHandler(w, cts => {
+                w.When(x => x.Write(Arg.Any<byte[]>(), Arg.Any<int>(), Arg.Any<int>()))
+                    .Do(c => {
+                        received = System.Text.Encoding.UTF8.GetString(c.ArgAt<byte[]>(0), 0, c.ArgAt<int>(2));
+                        cts.Cancel();
+                    });
             });
             var value = new JsonRpc.Client.Response(1);
 
@@ -40,8 +46,10 @@ public async Task ShouldSerializeValues()
 
                 handler.Send(value);
                 await wait();
-
-                tw.Received().Write("Content-Length: 46\r\n\r\n{\"protocolVersion\":\"2.0\",\"id\":1,\"result\":null}");
+                const string send = "Content-Length: 46\r\n\r\n{\"protocolVersion\":\"2.0\",\"id\":1,\"result\":null}";
+                received.Should().Be(send);
+                var b = System.Text.Encoding.UTF8.GetBytes(send);
+                w.Received().Write(Arg.Any<byte[]>(), 0, b.Length); // can't compare b here, because it is only value-equal and this test tests reference equality
             }
         }
     }