vim-jp / issues

有志で既知のバグや要望を検討・管理し、オフィシャルへの還元をしていきます。
https://vim-jp.org/
341 stars 11 forks source link

:call remote_startserver("") で、client-serverが通信不能 #1411

Closed svardew closed 1 year ago

svardew commented 1 year ago

不具合の内容

-autoservername +clientserver の vim で :call remote_startserver("") すると、通信不能なサーバーができてしまう。 v:servername は変更不可、起動したVimサーバーはkillできない、ので、vimを終了する事態になる。

現象・ログ

再現手順

[uxterm] $ vim --serverlist => 0 servers

(termA) $ vim --clean :call remote_startserver("") :echo "aa/" .. v:servername .. "/zz" => aa//zz

[uxterm] $ vim --serverlist => 1 servers [""]

(termB) $ vim --clean :call remote_startserver("") :echo "aa/" .. v:servername .. "/zz" => aa/1/zz

[uxterm] $ vim --serverlist => 2 servers ["", "1"] [uxterm] $ vim --servername "" --remote-send "yy3pgg" => 未知のオプション引数です: "--remote-send"

期待動作

call remote_startserver("") は、serverlist()が空の場合、エラーを投げる(Invalid Argument ?)

Vimのバージョン

VIM - Vi IMproved 9.0 (2022 Jun 28, compiled Mar 13 2023 20:35:57) 適用済パッチ: 1-1402 Huge 版 with GTK3 GUI. 機能の一覧 有効(+)/無効(-) -sodium

OSの種類/ディストリ/バージョン

ubuntu 16.04.7 LTS (4.15.0-142-generic i686 GNU/Linux)

使用している or 関係していそうなプラグイン

neoterm

その他

server = tv_get_string_chk() は varp->vval.v_string か (char_u *)"" のどちらかが返ってくるので、 if (server == NULL) 判定部を空文字のサーバー名がすり抜けるのではないでしょうか if (in_vim9script() && ... と同じようなチェックを追加すると、とりあえず回避できました。 if (server == NULL || check_for_nonempty_string_arg(argvars,0) == FAIL)

h-east commented 1 year ago

書かれている通り check_for_nonempty_string_arg() でチェックをおこなうのが良いと思います。 いろいろ整理するとこのようなpatchになりました。

あと、helpの加筆とtestの追加すればPull Request出せます。 (@svardew さんがやっていただくのありです)

diff --git a/src/clientserver.c b/src/clientserver.c
index dfbb8a8b0..b94bcbc61 100644
--- a/src/clientserver.c
+++ b/src/clientserver.c
@@ -968,18 +968,15 @@ f_remote_send(typval_T *argvars UNUSED, typval_T *rettv)
 f_remote_startserver(typval_T *argvars UNUSED, typval_T *rettv UNUSED)
 {
 #ifdef FEAT_CLIENTSERVER
-    char_u *server;
-
-    if (in_vim9script() && check_for_string_arg(argvars, 0) == FAIL)
+    if (check_for_nonempty_string_arg(argvars, 0) == FAIL)
    return;

-    server = tv_get_string_chk(&argvars[0]);
-    if (server == NULL)
-   return;     // type error; errmsg already given
     if (serverName != NULL)
    emsg(_(e_already_started_server));
     else
     {
+   char_u *server = tv_get_string_chk(&argvars[0]);
+
 # ifdef FEAT_X11
    if (check_connection() == OK)
        serverRegisterName(X_DISPLAY, server);
h-east commented 1 year ago

patch更新。

diff --git a/runtime/doc/builtin.txt b/runtime/doc/builtin.txt
index ec680bec4..063c25d30 100644
--- a/runtime/doc/builtin.txt
+++ b/runtime/doc/builtin.txt
@@ -7285,6 +7285,7 @@ remote_send({server}, {string} [, {idvar}])
 remote_startserver({name})
        Become the server {name}.  This fails if already running as a
        server, when |v:servername| is not empty.
+       When {name} is empty string, an error is given.

        Can also be used as a |method|: >
            ServerName()->remote_startserver()
diff --git a/src/clientserver.c b/src/clientserver.c
index dfbb8a8b0..b94bcbc61 100644
--- a/src/clientserver.c
+++ b/src/clientserver.c
@@ -968,18 +968,15 @@ f_remote_send(typval_T *argvars UNUSED, typval_T *rettv)
 f_remote_startserver(typval_T *argvars UNUSED, typval_T *rettv UNUSED)
 {
 #ifdef FEAT_CLIENTSERVER
-    char_u *server;
-
-    if (in_vim9script() && check_for_string_arg(argvars, 0) == FAIL)
+    if (check_for_nonempty_string_arg(argvars, 0) == FAIL)
    return;

-    server = tv_get_string_chk(&argvars[0]);
-    if (server == NULL)
-   return;     // type error; errmsg already given
     if (serverName != NULL)
    emsg(_(e_already_started_server));
     else
     {
+   char_u *server = tv_get_string_chk(&argvars[0]);
+
 # ifdef FEAT_X11
    if (check_connection() == OK)
        serverRegisterName(X_DISPLAY, server);
diff --git a/src/testdir/test_clientserver.vim b/src/testdir/test_clientserver.vim
index 64c9ab8e8..53947f410 100644
--- a/src/testdir/test_clientserver.vim
+++ b/src/testdir/test_clientserver.vim
@@ -182,7 +182,8 @@ func Test_client_server()
     endif
   endtry

-  call assert_fails('call remote_startserver([])', 'E730:')
+  call assert_fails('call remote_startserver("")', 'E1175:')
+  call assert_fails('call remote_startserver([])', 'E1174:')
   call assert_fails("let x = remote_peek([])", 'E730:')
   call assert_fails("let x = remote_read('vim10')",
         \ has('unix') ? ['E573:.*vim10'] : 'E277:')

defcompileでエラーになっていなかった。。 これでいいのかどうかちょっと考えます。

vim9script
def Aaa()
    remote_startserver("")
enddef
defcompile
h-east commented 1 year ago

PRed. https://github.com/vim/vim/pull/12327

h-east commented 1 year ago

Included. patch 9.0.1504

h-east commented 1 year ago

@svardew Thanks for the reporting👍

svardew commented 1 year ago

@h-east Thank you for your work! パッチ作成ありがとうございます。 v9.0.1506 をビルドして修正されてるのを確認しました。