Merge pull request #17 from fischerling/parser

Improve message parsing

5 files changed, 233 insertions, 52 deletions

diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml
index fb5ddf2..d33d6af 100644
--- a/.github/workflows/check.yml
+++ b/.github/workflows/check.yml
@@ -15,12 +15,8 @@ on:
 
 # A workflow run is made up of one or more jobs that can run sequentially or in parallel
 jobs:
-  # This workflow contains a single job called "build"
   check:
-    # The type of runner that the job will run on
     runs-on: ubuntu-latest
-
-    # Steps represent a sequence of tasks that will be executed as part of the job
     steps:
       # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it
       - uses: actions/checkout@v2
@@ -37,3 +33,15 @@ jobs:
         
       - name: static analysis
         run: make check-luacheck
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+
+      - name: install dependencies
+        run: |
+          sudo apt-get -y install luarocks
+          sudo luarocks install lunatest
+
+      - name: run test
+        run: make test
diff --git a/Makefile b/Makefile
index 116c2a0..cdf9647 100644
--- a/Makefile
+++ b/Makefile
@@ -2,6 +2,8 @@
 
 LUA_FILES := $(shell find -name "*.lua" -not -path "./json.lua")
 
+TEST_FILES := $(shell find -name "*_test.lua")
+
 # bash's process substitution is used for check-format
 SHELL := /bin/bash
 
@@ -15,3 +17,6 @@ check-format:
 
 format:
 	lua-format -i $(LUA_FILES)
+
+test:
+	for tf in $(TEST_FILES); do $$tf; done
diff --git a/init.lua b/init.lua
index 51993ed..b611276 100644
--- a/init.lua
+++ b/init.lua
@@ -1,6 +1,10 @@
 -- Copyright (c) 2021 Florian Fischer. All rights reserved.
 -- Use of this source code is governed by a MIT license found in the LICENSE file.
 -- We require vis compiled with the communicate patch
+local source_str = debug.getinfo(1, 'S').source:sub(2)
+local source_path = source_str:match('(.*/)')
+
+local parser = dofile(source_path .. 'parser.lua')
 
 -- forward declaration of our client table
 local lspc
@@ -23,10 +27,7 @@ if not vis.communicate then
 end
 
 local function load_fallback_json()
-  local source_str = debug.getinfo(1, 'S').source:sub(2)
-  local script_path = source_str:match('(.*/)')
-
-  return dofile(script_path .. 'json.lua')
+  return dofile(source_path .. 'json.lua')
 end
 
 -- find a suitable json implementation
@@ -868,51 +869,20 @@ end
 -- In the worst case a data chunk contains two partial messages on at the beginning
 -- and one at the end
 local function ls_recv_data(ls, data)
-  -- new message received
-  if ls.partial_response.len == 0 then
-    lspc.log('LSPC: parse new message')
-    local header = data:match('^Content%-Length: %d+')
-    if not header then
-      lspc_err('received unexpected message: ' .. data)
-      return
-    end
-    ls.partial_response.exp_len = tonumber(header:match('%d+'))
-    local _, content_start = data:find('\r\n\r\n')
-
-    ls.partial_response.msg = data:sub(content_start + 1)
-    ls.partial_response.len = string.len(ls.partial_response.msg)
-
-  else -- try to complete partial received message
-    lspc.log('LSPC: parse message continuation')
-    ls.partial_response.msg = ls.partial_response.msg .. data
-    ls.partial_response.len = ls.partial_response.len + string.len(data)
+  local err = ls.parser:add(data)
+  if err then
+    lspc_err(err)
+    return
   end
 
-  -- received message not complete yet
-  if ls.partial_response.len < ls.partial_response.exp_len then
+  local msgs = ls.parser:get_msgs()
+  if not msgs then
     return
   end
 
-  local complete_msg = ls.partial_response.msg:sub(1,
-                                                   ls.partial_response.exp_len)
-
-  lspc.log('LSPC: handling complete message: ' .. complete_msg)
-  local resp = json.decode(complete_msg)
-  ls_handle_msg(ls, resp)
-
-  local leftover = ls.partial_response.len - ls.partial_response.exp_len
-
-  if leftover > 0 then
-    local leftover_data = ls.partial_response.msg:sub(ls.partial_response
-                                                          .exp_len + 1)
-    lspc.log('LSPC: parse leftover: "' .. leftover_data .. '"')
-
-    ls.partial_response.exp_len = 0
-    ls.partial_response.len = 0
-    ls_recv_data(ls, leftover_data)
-  else
-    ls.partial_response.exp_len = 0
-    ls.partial_response.len = 0
+  for _, msg in ipairs(msgs) do
+    local resp = json.decode(msg)
+    ls_handle_msg(ls, resp)
   end
 end
 
@@ -1030,7 +1000,7 @@ local function ls_start_server(syntax)
     id = 0,
     inflight = {},
     open_files = {},
-    partial_response = {exp_len = 0, len = 0, response = nil},
+    parser = parser.Parser(),
   }
 
   ls.fd = vis:communicate(ls_conf.name, ls_conf.cmd)
@@ -1360,7 +1330,5 @@ vis:option_register('lspc-highlight-diagnostics', 'bool', function(value)
   return true
 end, 'Should lspc highlight diagnostics if available')
 
-local source_str = debug.getinfo(1, 'S').source:sub(2)
-local script_path = source_str:match('(.*/)')
-dofile(script_path .. 'bindings.lua')
+dofile(source_path .. 'bindings.lua')
 return lspc
diff --git a/parser.lua b/parser.lua
new file mode 100644
index 0000000..f1b09f5
--- /dev/null
+++ b/parser.lua
@@ -0,0 +1,75 @@
+-- Copyright (c) 2021 Florian Fischer. All rights reserved.
+-- Use of this source code is governed by a MIT license found in the LICENSE file.
+
+-- Parse the data send by a language server
+-- Note the chunks received may not end with the end of a message.
+-- In the worst case a data chunk contains two partial messages on at the beginning
+-- and one at the end
+
+local function add(p, data)
+  p.data = p.data .. data
+
+  -- we have not seen a complete header in data yet
+  if not p.exp_len then
+    -- search for the end of a header
+    -- LSP message format: 'header\r\n\r\nbody'
+    local header_end, content_start = p.data:find('\r\n\r\n')
+
+    -- header is not complete yet -> save data and wait for more
+    if not header_end then
+      return
+    end
+
+    -- got a complete header
+    local header = p.data:sub(1, header_end)
+
+    -- extract content length from the header
+    p.exp_len = tonumber(header:match('%d+'))
+    if not p.exp_len then
+      return 'invalid header in data: ' .. p.data
+    end
+
+    -- consume header from data
+    p.data = p.data:sub(content_start + 1)
+  end
+
+  local data_avail = string.len(p.data)
+  -- we have no complete message yet -> await more data
+  if p.exp_len > data_avail then
+    return
+  end
+
+  local complete_msg = p.data:sub(1, p.exp_len)
+  table.insert(p.msgs, complete_msg)
+
+  -- consume complete_msg from data
+  p.data = p.data:sub(p.exp_len + 1)
+
+  local leftover = data_avail - p.exp_len
+
+  -- reset exp_len to search for a new header
+  p.exp_len = nil
+
+  if leftover > 0 then
+    return p:add('')
+  end
+end
+
+local function get_msgs(p)
+  local msgs = p.msgs
+  p.msgs = {}
+  return msgs
+end
+
+local function Parser()
+  local parser = {
+    add = add,
+    get_msgs = get_msgs,
+    exp_len = nil,
+    data = '',
+    msgs = {},
+  }
+  return parser
+end
+
+return {Parser = Parser}
diff --git a/parser_test.lua b/parser_test.lua
new file mode 100755
index 0000000..e607611
--- /dev/null
+++ b/parser_test.lua
@@ -0,0 +1,125 @@
+#!/usr/bin/env lua
+
+local parser = require('parser')
+
+local function build_msg(body)
+  return 'Content-Length: ' .. tostring(string.len(body)) .. '\r\n\r\n' .. body
+end
+
+local lunatest = require('lunatest')
+
+function test_complete_msg() -- luacheck: ignore 111
+  local msg = build_msg('foo')
+  local p = parser.Parser()
+  local err = p:add(msg)
+  lunatest.assert_nil(err)
+
+  local msgs = p:get_msgs()
+  lunatest.assert_table(msgs)
+  lunatest.assert_len(1, msgs)
+  lunatest.assert_equal(msgs[1], 'foo')
+end
+
+function test_two_complete_msgs() -- luacheck: ignore 111
+  local p = parser.Parser()
+  local data = build_msg('foo')
+  local err = p:add(data)
+  lunatest.assert_nil(err)
+
+  local msgs = p:get_msgs()
+  lunatest.assert_table(msgs)
+  lunatest.assert_len(1, msgs)
+  lunatest.assert_equal(msgs[1], 'foo')
+
+  data = build_msg('bar')
+  err = p:add(data)
+  lunatest.assert_nil(err)
+  msgs = p:get_msgs()
+  lunatest.assert_table(msgs)
+  lunatest.assert_len(1, msgs)
+  lunatest.assert_equal(msgs[1], 'bar')
+end
+
+function test_two_complete_msgs_at_once() -- luacheck: ignore 111
+  local data = build_msg('foo') .. build_msg('bar')
+  local p = parser.Parser()
+  local err = p:add(data)
+  lunatest.assert_nil(err)
+
+  local msgs = p:get_msgs()
+  lunatest.assert_table(msgs)
+  lunatest.assert_len(2, msgs)
+  lunatest.assert_equal(msgs[1], 'foo')
+  lunatest.assert_equal(msgs[2], 'bar')
+end
+
+function test_split_msg() -- luacheck: ignore 111
+  local msg = build_msg('foo')
+  local part1 = msg:sub(1, -3)
+  local part2 = msg:sub(-2)
+  local p = parser.Parser()
+  local err = p:add(part1)
+  lunatest.assert_nil(err)
+
+  err = p:add(part2)
+  lunatest.assert_nil(err)
+
+  local msgs = p:get_msgs()
+  lunatest.assert_table(msgs)
+  lunatest.assert_len(1, msgs)
+  lunatest.assert_equal(msgs[1], 'foo')
+end
+
+function test_complete_and_split_msg() -- luacheck: ignore 111
+  local msg = build_msg('foo') .. build_msg('bar')
+  local part1 = msg:sub(1, -3)
+  local part2 = msg:sub(-2)
+  local p = parser.Parser()
+  local err = p:add(part1)
+  lunatest.assert_nil(err)
+
+  err = p:add(part2)
+  lunatest.assert_nil(err)
+
+  local msgs = p:get_msgs()
+  lunatest.assert_table(msgs)
+  lunatest.assert_len(2, msgs)
+  lunatest.assert_equal(msgs[1], 'foo')
+  lunatest.assert_equal(msgs[2], 'bar')
+end
+
+function test_split_hdr() -- luacheck: ignore 111
+  local msg = build_msg('foo')
+  local part1 = msg:sub(1, 3)
+  local part2 = msg:sub(4)
+  local p = parser.Parser()
+  local err = p:add(part1)
+  lunatest.assert_nil(err)
+
+  err = p:add(part2)
+  lunatest.assert_nil(err)
+
+  local msgs = p:get_msgs()
+  lunatest.assert_table(msgs)
+  lunatest.assert_len(1, msgs)
+  lunatest.assert_equal(msgs[1], 'foo')
+end
+
+function test_split_hdr_body_sep() -- luacheck: ignore 111
+  local msg = build_msg('foo')
+  local part1 = msg:sub(1, 19)
+  local part2 = msg:sub(20)
+  local p = parser.Parser()
+  local err = p:add(part1)
+  lunatest.assert_nil(err)
+
+  err = p:add(part2)
+  lunatest.assert_nil(err)
+
+  local msgs = p:get_msgs()
+  lunatest.assert_table(msgs)
+  lunatest.assert_len(1, msgs)
+  lunatest.assert_equal(msgs[1], 'foo')
+end
+
+lunatest.run()