From 483ed22f686010cb9fe6f8ec4512263dd8d95706 Mon Sep 17 00:00:00 2001
From: Florian Fischer <florian.fischer@muhq.space>
Date: Mon, 16 Aug 2021 14:01:15 +0200
Subject: move and fix parser code

Move LSP message parser code into parser.lua.
Fix parsing when message headers are split between to data junks.
Simplify parser code using one string buffer and consuming already
parsed message parts.
---
 init.lua   | 64 ++++++++++++++---------------------------------------
 parser.lua | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 48 deletions(-)
 create mode 100644 parser.lua

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}
-- 
cgit v1.2.3


From 29b50c08a7e8524797549354a2e5063974c0e1b2 Mon Sep 17 00:00:00 2001
From: Florian Fischer <florian.fischer@muhq.space>
Date: Mon, 16 Aug 2021 14:04:39 +0200
Subject: add tests for the LSP message parser

---
 .github/workflows/check.yml |  16 ++++--
 Makefile                    |   5 ++
 parser_test.lua             | 125 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 142 insertions(+), 4 deletions(-)
 create mode 100755 parser_test.lua

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/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()
-- 
cgit v1.2.3