New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows の job でダブルコーテーションを付けるとコマンドを正常実行できない場合がある #1136

Open
hokorobi opened this Issue Dec 29, 2017 · 4 comments

Comments

Projects
None yet
3 participants
@hokorobi

hokorobi commented Dec 29, 2017

質問・報告の内容

8.0.0921 において job で正常実行できたコマンドが 8.0.0938 では実行できなくなりました。
パスを括っているダブルコーテーションがないと実行できます。
バグか、もしくは、 job_start の書き方を改めた方が良いでしょうか?

コード例:

function! Disp(ch, msg)
    echom iconv(a:msg, 'char', &encoding)
endfunction

let shellslash = &shellslash
set noshellslash
call job_start([&shell, &shellcmdflag, 'rd /S /Q "C:\\path\\to\\hoge"'],  { "callback" : "Disp"})
let &shellslash = shellslash

実行結果は、「ファイル名、ディレクトリ名、またはボリューム ラベルの構文が間違っています。」となります。

Vimのバージョン

8.0.0938

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

  • Windows 10 Home 64bit (1709 16299.125)

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

(関係していそうなプラグインなどがあればココに書いてください)

その他

Vim のビルド環境がなく https://github.com/vim/vim-win32-installer を使っているため、8.0.0921 と 8.0.0938 の比較となりました。
8.0.0928: MS-Windows: passing arglist to job has escaping problems で変化があったのではないかと思います。

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Dec 29, 2017

Member

結論から言うと

job_start の書き方を改めた方が良いでしょうか?

「はい」です。上記のソースは実際には以下のコマンドが実行されます。

C:\WINDOWS\system32\cmd.exe /c "rd /S /Q \"C:\tmp\bar\""

Windows は UNIX と違い、引数のパースをコマンド自身が行います。そしてその実装が各々で異なる場合があります。実行モジュールの場合、おおよそ今実装している物で通るはずです。しかし cmd.exe は特殊な事を行っていて、実行モジュールの引数パース方法と異なります。

試しに以下のソースをコンパイルしてみて下さい。

#include <stdio.h>

int
main(int argc, char* argv[]) {
  int i;
  for (i = 0; i < argc; i++) puts(argv[i]);
  return 0;
}

これを上記のコマンドの cmd.exe の代わりに実行すると以下の様に表示されるはずです。

a.exe
/c
rd /S /Q "C:\tmp\bar"

これは期待する結果です。コマンドプロンプトの場合は "rd /S /Q \"C:\tmp\bar\"" この引数が展開されずそのまま渡されてしまいます。

これは Windows では引数のパースが個々のコマンドで異なるという事実の為、致し方ないのです。

修正の提案ですが、Windows で cmd.exe を使う場合に限っては [&shell, &shellcmdflag, arg] ではなく printf("%s %s %s", &shell, &shellcmdflag, arg) という実装にしてもらうのが良いと思います。

Member

mattn commented Dec 29, 2017

結論から言うと

job_start の書き方を改めた方が良いでしょうか?

「はい」です。上記のソースは実際には以下のコマンドが実行されます。

C:\WINDOWS\system32\cmd.exe /c "rd /S /Q \"C:\tmp\bar\""

Windows は UNIX と違い、引数のパースをコマンド自身が行います。そしてその実装が各々で異なる場合があります。実行モジュールの場合、おおよそ今実装している物で通るはずです。しかし cmd.exe は特殊な事を行っていて、実行モジュールの引数パース方法と異なります。

試しに以下のソースをコンパイルしてみて下さい。

#include <stdio.h>

int
main(int argc, char* argv[]) {
  int i;
  for (i = 0; i < argc; i++) puts(argv[i]);
  return 0;
}

これを上記のコマンドの cmd.exe の代わりに実行すると以下の様に表示されるはずです。

a.exe
/c
rd /S /Q "C:\tmp\bar"

これは期待する結果です。コマンドプロンプトの場合は "rd /S /Q \"C:\tmp\bar\"" この引数が展開されずそのまま渡されてしまいます。

これは Windows では引数のパースが個々のコマンドで異なるという事実の為、致し方ないのです。

修正の提案ですが、Windows で cmd.exe を使う場合に限っては [&shell, &shellcmdflag, arg] ではなく printf("%s %s %s", &shell, &shellcmdflag, arg) という実装にしてもらうのが良いと思います。

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Dec 29, 2017

Member

本体をもし修正する場合、リストで渡された第一引数が %COMSPEC% の場合だけ残りの引数のエスケープをやめるという案も無くはないです。かなりワークアラウンドですが。
このワークアラウンドへの可否については 👍👎 でどうぞ。

Member

mattn commented Dec 29, 2017

本体をもし修正する場合、リストで渡された第一引数が %COMSPEC% の場合だけ残りの引数のエスケープをやめるという案も無くはないです。かなりワークアラウンドですが。
このワークアラウンドへの可否については 👍👎 でどうぞ。

@hokorobi

This comment has been minimized.

Show comment
Hide comment
@hokorobi

hokorobi Jan 2, 2018

下位互換性の観点からはワークアラウンドに賛成します。
それ以外の観点を含めた判断は、私にはできそうにないのでお任せします。

hokorobi commented Jan 2, 2018

下位互換性の観点からはワークアラウンドに賛成します。
それ以外の観点を含めた判断は、私にはできそうにないのでお任せします。

@mattn

This comment has been minimized.

Show comment
Hide comment
@mattn

mattn Jan 3, 2018

Member
diff --git a/src/channel.c b/src/channel.c
index f7eded21e..4609a4a39 100644
--- a/src/channel.c
+++ b/src/channel.c
@@ -5102,13 +5102,21 @@ win32_build_cmd(list_T *l, garray_T *gap)
 {
     listitem_T  *li;
     char_u	*s;
+    char_u	*cmd_shell = mch_getenv("COMSPEC");
+    int		is_cmd = FALSE;
 
     for (li = l->lv_first; li != NULL; li = li->li_next)
     {
 	s = get_tv_string_chk(&li->li_tv);
 	if (s == NULL)
 	    return FAIL;
-	s = win32_escape_arg(s);
+	if (cmd_shell != NULL &&
+		li == l->lv_first && STRICMP(s, cmd_shell) == 0) is_cmd = TRUE;
+	if (is_cmd)
+	    s = vim_strsave_escaped_ext(s,
+			(char_u *)"|", '^', FALSE);
+	else
+	    s = win32_escape_arg(s);
 	if (s == NULL)
 	    return FAIL;
 	ga_concat(gap, s);

$COMSPEC との比較しかしていないので cmd とか cmd.exe で指定された場合は元の動作となります。ワークアラウンドなので Bram から ok が出ない可能性が高いです。SearchPath で前もってパスを調べる案もありますが、遅くなる可能性があるので、Windows を使っている皆さんから意見を募集したいです。

Member

mattn commented Jan 3, 2018

diff --git a/src/channel.c b/src/channel.c
index f7eded21e..4609a4a39 100644
--- a/src/channel.c
+++ b/src/channel.c
@@ -5102,13 +5102,21 @@ win32_build_cmd(list_T *l, garray_T *gap)
 {
     listitem_T  *li;
     char_u	*s;
+    char_u	*cmd_shell = mch_getenv("COMSPEC");
+    int		is_cmd = FALSE;
 
     for (li = l->lv_first; li != NULL; li = li->li_next)
     {
 	s = get_tv_string_chk(&li->li_tv);
 	if (s == NULL)
 	    return FAIL;
-	s = win32_escape_arg(s);
+	if (cmd_shell != NULL &&
+		li == l->lv_first && STRICMP(s, cmd_shell) == 0) is_cmd = TRUE;
+	if (is_cmd)
+	    s = vim_strsave_escaped_ext(s,
+			(char_u *)"|", '^', FALSE);
+	else
+	    s = win32_escape_arg(s);
 	if (s == NULL)
 	    return FAIL;
 	ga_concat(gap, s);

$COMSPEC との比較しかしていないので cmd とか cmd.exe で指定された場合は元の動作となります。ワークアラウンドなので Bram から ok が出ない可能性が高いです。SearchPath で前もってパスを調べる案もありますが、遅くなる可能性があるので、Windows を使っている皆さんから意見を募集したいです。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment