[vim/vim] It's difficult for channel server to parse messages from ch_evalexpr() and ch_sendexpr() (#1001)

[haya14busa]

Hi, bram.

As patch 7.4.1252 said, channel servers may receive multiple messages concatenated like this one ([1,"hello!"][2,"hello!"]).

It may also occurs in every channel servers including sample demoserver.py ($VIMRUNTIME/tools/demoserver.py)

How to Reproduce

test.vim

function! s:test() abort
  let handle = ch_open('localhost:8765')
  for _ in range(10)
    call ch_sendexpr(handle, 'hello!')
  endfor
endfunction

call s:test()

1. $ python demoserver.py ($VIMRUNTIME/tools/demoserver.py)
2. $ vim test.vim
3. :source %

We can see following server error like this.

=== socket opened ===
received: [1,"hello!"][2,"hello!"][3,"hello!"][4,"hello!"][5,"hello!"][6,"hello!"][7,"hello!"][8,"hello!"]
json decoding failed
received: [9,"hello!"][10,"hello!"]
json decoding failed
=== socket closed ===

Solution

patch 7.4.1252 fixed the test server by finding ][, but it doesn't handle messages which contains ][.
On top of that, every channel servers, regardless of programing language, have to handle this problem.
It's possible, but really troublesome to handle messages correctly.

So, I suggest to use '\n' as delimiter which is handy for channel servers, regardless of programing language, to handle messages.

How do you think?
I guess we cannot change channel specification after Vim 8.0 release, so we should include this change or include other solution.

Other

This pull request doens't include doc changes nor demoserver.py improvement.
This pull request just intends to show the problem and disscuss it. Of course, you can discard this patch.
This patch includes @k-takata's work. Thanks @k-takata.

---

You can view, comment on, or merge this pull request online at:

  https://github.com/vim/vim/pull/1001

Commit Summary

• use \n as delimiter for ch_evalexpr() and ch_sendexpr()
• fix channel_test: use \n as a delimiter

File Changes

• M src/channel.c (3)
• M src/json.c (1)
• M src/testdir/test_channel.py (8)
• M src/testdir/test_channel.vim (6)

Patch Links:

• https://github.com/vim/vim/pull/1001.patch
• https://github.com/vim/vim/pull/1001.diff

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, .

[Bram Moolenaar]

haya14busa wrote:

> As [patch 7.4.1252](https://github.com/vim/vim/commit/e7bed627c89ed80bc4b2d96f542819029adf6e76) said, channel servers may receive multiple messages concatenated like this one (`[1,"hello!"][2,"hello!"]`).

>
> It may also occurs in every channel servers including sample demoserver.py ($VIMRUNTIME/tools/demoserver.py)
>

> ### How to Reproduce
>
> #### test.vim
> ```vim

> function! s:test() abort
> let handle = ch_open('localhost:8765')
> for _ in range(10)
> call ch_sendexpr(handle, 'hello!')
> endfor
> endfunction
>
> call s:test()

> ```
>
> 1. $ python demoserver.py ($VIMRUNTIME/tools/demoserver.py)
> 2. $ vim test.vim
> 3. :source %

>
> We can see following server error like this.
>
> ```
> === socket opened ===
> received: [1,"hello!"][2,"hello!"][3,"hello!"][4,"hello!"][5,"hello!"][6,"hello!"][7,"hello!"][8,"hello!"]
> json decoding failed
> received: [9,"hello!"][10,"hello!"]
> json decoding failed
> === socket closed ===

> ```
>
> ### Solution
> [patch 7.4.1252](https://github.com/vim/vim/commit/e7bed627c89ed80bc4b2d96f542819029adf6e76) fixed the test server by finding `][`, but it doesn't handle messages which contains `][`.

> On top of that, every channel servers, regardless of programing language, have to handle this problem.
> It's possible, but really troublesome to handle messages correctly.
>
> So, I suggest to use '\n' as delimiter which is handy for channel
> servers, regardless of programing language, to handle messages.
>
> How do you think?

That makes sense. Also, many read functions read a whole line, thus
look for the NL character.

I assume JSON parsers see the NL as white space and will ignore it.

> I guess we cannot change channel specification after Vim 8.0 release,
> so we should include this change or include other solution.
>

> ### Other

> This pull request doens't include doc changes nor demoserver.py improvement.
> This pull request just intends to show the problem and disscuss it. Of course, you can discard this patch.
> This patch includes @k-takata's work. Thanks @k-takata.

I'll check out the patch.

--
hundred-and-one symptoms of being an internet addict:
69. Yahoo welcomes you with your own start page

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[Bram Moolenaar]

Closed #1001 via f1f0792.

[haya14busa]

I confirmed it works.
Thank you for the fix!