Loading ...
Sorry, an error occurred while loading the content.

Re: bad default shellxquote in Widows

Expand Messages
  • Bram Moolenaar
    ... I had some trouble reproducing the problem. When both shellquote and ... The double quote at the end is essential. It appears cmd.exe strips the first
    Message 1 of 12 , Nov 4, 2008
    • 0 Attachment
      Benjamin Fritz wrote:

      > I've been playing around with a plugin
      > (http://www.vim.org/scripts/script.php?script_id=2378), trying to
      > specify a path to the _command_ that is passed to system(). Since this
      > command may have spaces in it, it needs to be wrapped in quotes.
      >
      > However, if it is wrapped in quotes, the default settings of
      > shellxquote on win32 do not work. This command will not do what is
      > intended, for example:
      >
      > cmd /c "C:\some path\some program.exe" "an argument"
      >
      > If you set shellxquote to \" it does work. The command passed looks
      > funny, but is actually desired:
      >
      > cmd /c ""C:\some path\some program.exe" "an argument""
      >
      > I think that shellxquote should default to \" on Windows always. I
      > know of no case where it fails.
      >
      > The reason for this behavior:
      >
      > >From the help for cmd in Windows:
      >
      > > If /C or /K is specified, then the remainder of the command line after
      > > the switch is processed as a command line, where the following logic is
      > > used to process quote (") characters:
      > >
      > > 1. If all of the following conditions are met, then quote characters
      > > on the command line are preserved:
      > >
      > > - no /S switch
      > > - exactly two quote characters
      > > - no special characters between the two quote characters,
      > > where special is one of: &<>()@^|
      > > - there are one or more whitespace characters between the
      > > the two quote characters
      > > - the string between the two quote characters is the name
      > > of an executable file.
      > >
      > > 2. Otherwise, old behavior is to see if the first character is
      > > a quote character and if so, strip the leading character and
      > > remove the last quote character on the command line, preserving
      > > any text after the last quote character."
      >
      > Both values of shellxquote, in this case, will fall into category 2.
      >
      > Thus, the current default actually executes the command:
      >
      > C:\some path\some program.exe" "an argument
      >
      > Whereas, setting shellxquote to \" will execute:
      >
      > "C:\some path\some program.exe" "an argument"
      >
      > This is the desired command.
      >
      > Normal commands would still work, for example, the command:
      >
      > echo "abc def"
      >
      > becomes:
      >
      > cmd /c "echo "abc def""
      >
      > which executes:
      >
      > echo "abc def"
      >
      > Again, just as desired.
      >
      > This might not always work, because I think some versions of cmd.exe
      > will strip the _closing_ quote instead of the _last_ quote. For the
      > best possible compatibility, it is probably a good idea to also
      > default shellcmdflag to "/s\ /c" instead of just "/c" in the same
      > situation that "/c" currently applies. This will force the 2nd quote
      > behavior given in the quoted cmd help.

      I had some trouble reproducing the problem. When both 'shellquote' and
      'shellxquote' are emtpy and 'shell' is cmd.exe, this works fine:

      :echo system('"e:/mksnt/echo.exe" foo bar')

      I finally figured out a way to break it:

      :echo system('"e:/mksnt/echo.exe" "foo bar"')

      The double quote at the end is essential.
      It appears cmd.exe strips the first and last double quote and tries to
      execute e:/mksnt/echo.exe" "foo . Setting 'shellxquote' to a double
      quote indeed helps to fix this.

      Now the question is what will break if we change the default for
      'shellxquote'. I'm not sure about that.

      --
      If Microsoft would build a car...
      ... The airbag system would ask "are you SURE?" before deploying.

      /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
      /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
      \\\ download, build and distribute -- http://www.A-A-P.org ///
      \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

      --~--~---------~--~----~------------~-------~--~----~
      You received this message from the "vim_dev" maillist.
      For more information, visit http://www.vim.org/maillist.php
      -~----------~----~----~----~------~----~------~--~---
    • Ben Fritz
      ... The case you found where it breaks is probably more common than would appear at first glance, though I did not realize it was so specific. There is a line
      Message 2 of 12 , Nov 6, 2008
      • 0 Attachment
        On Nov 4, 3:08 pm, Bram Moolenaar <B...@...> wrote:
        >
        > I had some trouble reproducing the problem. When both 'shellquote' and
        > 'shellxquote' are emtpy and 'shell' is cmd.exe, this works fine:
        >
        > :echo system('"e:/mksnt/echo.exe" foo bar')
        >
        > I finally figured out a way to break it:
        >
        > :echo system('"e:/mksnt/echo.exe" "foo bar"')
        >
        > The double quote at the end is essential.

        And at the beginning, it would appear, because this also works:

        :echo system('echo "a b c"')

        > It appears cmd.exe strips the first and last double quote and tries to
        > execute e:/mksnt/echo.exe" "foo . Setting 'shellxquote' to a double
        > quote indeed helps to fix this.
        >

        The case you found where it breaks is probably more common than would
        appear at first glance, though I did not realize it was so specific.

        There is a line in the script I mentioned working on (http://
        www.vim.org/scripts/script.php?script_id=2378):

        let s:lint = system(shellescape(g:mlint_path_to_mlint) . " " .
        shellescape(b:mlintTempDir . s:filename))

        In my testing, I had set g:mlint_path_to_mlint to something in my
        "Program Files" directory (hence the need for a double quote) and
        s:filename contained "Copy of startup.m" (also, a need for the double
        quote).

        There may be a much better way to accomplish this (which I'd love to
        hear), but nevertheless using system() to invoke an external program
        in Windows with a potentially space-infused path, and then passing
        this program an argument similarly infused with spaces (such as a file
        path), is potentially very useful.

        > Now the question is what will break if we change the default for
        > 'shellxquote'. I'm not sure about that.
        >

        I doubt that changing the default would break anything built into Vim,
        but I suppose that remains to be seen. Changing the default would
        certainly add a whole class of use cases that work "out of the box".
        Any corner cases that for some reason do not work properly could
        easily be worked around as done in the plugin mentioned, by setting
        and restoring shellxquote (and possibly shellcmdflag). I haven't found
        a way to break system() with shellxquote=\", but there is certainly a
        way to break it with the current default. I have admittedly not done
        much with shellxquote for uses other than system(). I see potential
        for a problem here, especially when using redirection, but I _think_
        it will work.

        Plugins are another question though. For testing, I placed my proposed
        values of shellxquote and shellcmdflag in my vimrc, and immediately
        broke the TagList plugin.

        This seems to be caused by a bug in TagList, actually. I don't think
        the following code is needed, and it is the root of the bug. But, I'm
        not actually sure what the intent of it is:

        if &shellxquote == '"'
        " Double-quotes within double-quotes will not work in the
        " command-line.If the 'shellxquote' option is set to double-
        quotes,
        " then escape the double-quotes in the ctags command-line.
        let ctags_cmd = escape(ctags_cmd, '"')
        endif

        Later (in Windows), Taglist writes ctags_cmd to a temporary batch file
        and executes the batch file with system().

        I wonder how many other plugins do something similar.
        --~--~---------~--~----~------------~-------~--~----~
        You received this message from the "vim_dev" maillist.
        For more information, visit http://www.vim.org/maillist.php
        -~----------~----~----~----~------~----~------~--~---
      • David Fishburn
        ... I am the one who supplied the code to Yegappan to create the ctags_cmd file. This is not just affecting Vim, it seems cmd.exe will not execute commands
        Message 3 of 12 , Nov 6, 2008
        • 0 Attachment
          On Thu, Nov 6, 2008 at 12:58 PM, Ben Fritz <fritzophrenic@...> wrote:
          >
          > On Nov 4, 3:08 pm, Bram Moolenaar <B...@...> wrote:
          >>
          ...
          > Plugins are another question though. For testing, I placed my proposed
          > values of shellxquote and shellcmdflag in my vimrc, and immediately
          > broke the TagList plugin.
          >
          > This seems to be caused by a bug in TagList, actually. I don't think
          > the following code is needed, and it is the root of the bug. But, I'm
          > not actually sure what the intent of it is:
          >
          > if &shellxquote == '"'
          > " Double-quotes within double-quotes will not work in the
          > " command-line.If the 'shellxquote' option is set to double-
          > quotes,
          > " then escape the double-quotes in the ctags command-line.
          > let ctags_cmd = escape(ctags_cmd, '"')
          > endif
          >
          > Later (in Windows), Taglist writes ctags_cmd to a temporary batch file
          > and executes the batch file with system().

          I am the one who supplied the code to Yegappan to create the ctags_cmd file.

          This is not just affecting Vim, it seems cmd.exe will not execute
          commands with more than 1 set of double quotes. I guess you are
          saying above, you can by simply doubling up the first and the last.

          Gee, I wish I had figured that our earlier. I have had to do the old
          cmd file for several different projects (unrelated to Vim).

          Dave

          --~--~---------~--~----~------------~-------~--~----~
          You received this message from the "vim_dev" maillist.
          For more information, visit http://www.vim.org/maillist.php
          -~----------~----~----~----~------~----~------~--~---
        • Bram Moolenaar
          Ben Fritz wrote: [...] ... The problem here appears to be that the plugin assumes a sh like shell . Escaping double quotes with a backslash doesn t work for
          Message 4 of 12 , Nov 6, 2008
          • 0 Attachment
            Ben Fritz wrote:

            [...]

            > > Now the question is what will break if we change the default for
            > > 'shellxquote'. I'm not sure about that.
            >
            > I doubt that changing the default would break anything built into Vim,
            > but I suppose that remains to be seen. Changing the default would
            > certainly add a whole class of use cases that work "out of the box".
            > Any corner cases that for some reason do not work properly could
            > easily be worked around as done in the plugin mentioned, by setting
            > and restoring shellxquote (and possibly shellcmdflag). I haven't found
            > a way to break system() with shellxquote=\", but there is certainly a
            > way to break it with the current default. I have admittedly not done
            > much with shellxquote for uses other than system(). I see potential
            > for a problem here, especially when using redirection, but I _think_
            > it will work.
            >
            > Plugins are another question though. For testing, I placed my proposed
            > values of shellxquote and shellcmdflag in my vimrc, and immediately
            > broke the TagList plugin.
            >
            > This seems to be caused by a bug in TagList, actually. I don't think
            > the following code is needed, and it is the root of the bug. But, I'm
            > not actually sure what the intent of it is:
            >
            > if &shellxquote == '"'
            > " Double-quotes within double-quotes will not work in the
            > " command-line.If the 'shellxquote' option is set to double-
            > quotes,
            > " then escape the double-quotes in the ctags command-line.
            > let ctags_cmd = escape(ctags_cmd, '"')
            > endif
            >
            > Later (in Windows), Taglist writes ctags_cmd to a temporary batch file
            > and executes the batch file with system().
            >
            > I wonder how many other plugins do something similar.

            The problem here appears to be that the plugin assumes a sh like
            'shell'. Escaping double quotes with a backslash doesn't work for
            cmd.exe, it sees backslashes as path separators..


            --
            Some of the well know MS-Windows errors:
            ESLEEP Operator fell asleep
            ENOERR No error yet
            EDOLLAR OS too expensive
            EWINDOWS MS-Windows loaded, system in danger

            /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
            /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
            \\\ download, build and distribute -- http://www.A-A-P.org ///
            \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

            --~--~---------~--~----~------------~-------~--~----~
            You received this message from the "vim_dev" maillist.
            For more information, visit http://www.vim.org/maillist.php
            -~----------~----~----~----~------~----~------~--~---
          • Tony Mechelynck
            On 06/11/08 18:58, Ben Fritz wrote: [...] ... [...] To avoid passing a path with spaces in it, use the short form of the path, such as %:p:8 (this works only
            Message 5 of 12 , Nov 6, 2008
            • 0 Attachment
              On 06/11/08 18:58, Ben Fritz wrote:
              [...]
              > In my testing, I had set g:mlint_path_to_mlint to something in my
              > "Program Files" directory (hence the need for a double quote) and
              > s:filename contained "Copy of startup.m" (also, a need for the double
              > quote).
              >
              > There may be a much better way to accomplish this (which I'd love to
              > hear), but nevertheless using system() to invoke an external program
              > in Windows with a potentially space-infused path, and then passing
              > this program an argument similarly infused with spaces (such as a file
              > path), is potentially very useful.
              [...]

              To avoid passing a path with spaces in it, use the "short form" of the
              path, such as %:p:8 (this works only on Windows -- on Unix you can just
              backslash-escape the spaces).

              This will convert the path so that "Program Files" becomes PROGRA~1, "My
              Documents" MYDOCU~1 and, I think, "Copy of startup.m" COPYOF~1.M -- the
              same directories and files will be accessed, but without the need for
              quoting the path.


              Best regards,
              Tony.
              --
              Information Center, n.:
              A room staffed by professional computer people whose job it is
              to tell you why you cannot have the information you require.

              --~--~---------~--~----~------------~-------~--~----~
              You received this message from the "vim_dev" maillist.
              For more information, visit http://www.vim.org/maillist.php
              -~----------~----~----~----~------~----~------~--~---
            • Ben Fritz
              On Nov 6, 11:20 pm, Tony Mechelynck ... Ah yes, this would be a reasonable solution in the general case. I have two issues with
              Message 6 of 12 , Nov 7, 2008
              • 0 Attachment
                On Nov 6, 11:20 pm, Tony Mechelynck <antoine.mechely...@...>
                wrote:
                > To avoid passing a path with spaces in it, use the "short form" of the
                > path, such as %:p:8 (this works only on Windows -- on Unix you can just
                > backslash-escape the spaces).
                >
                > This will convert the path so that "Program Files" becomes PROGRA~1, "My
                > Documents" MYDOCU~1 and, I think, "Copy of startup.m" COPYOF~1.M -- the
                > same directories and files will be accessed, but without the need for
                > quoting the path.
                >
                > Best regards,
                > Tony.

                Ah yes, this would be a reasonable solution in the general case. I
                have two issues with it, however:

                1. It would require separate logic for Unix vs. Windows, whereas
                simply fixing the default value of shellxquote and shellcmdflag for
                Windows would allow the same logic to be used.
                2. In the particular plugin I was working on, which highlights errors
                in MATLAB scripts using the mlint utility, system() is used to pass a
                file name to mlint. One of the checks that mlint does is to ensure
                that the file name is "formed from a legal MATLAB identifier." Using
                the short form of Windows path names, mlint will always return this
                error if the file name is longer than 8 characters (though
                incidentally, the file name is also invalid if it contains spaces, but
                theoretically the temporary directory it is stored in may have spaces
                in the path with no problems).

                So I think the better solution would be to fix the values of
                shellxquote and shellcmdflag, either by adding extra logic to the
                plugin, or by actually changing the defaults.

                For convienience, the suggested default values are:

                " for windows, always:
                set shellxquote=\"
                " for windows, when 'shell' does not contain "sh" somewhere:
                set shellcmdflag=/s\ /c

                This will place extra quotes around the entire command, and force the
                cmd.exe behavior that removes the first and last quote from the line
                if the first character is a quote.

                I personally think that the current defaults break in enough cases
                that we should change them as suggested in this thread, but I can see
                the argument for keeping the current values to prevent plugins like
                taglist that escape quotes with backslashes depending on the value of
                shellxquote. Plugin developers may have assumed (as done in the
                taglist plugin) that if the cmd.exe shell is used, shellxquote will
                _not_ be equal to a " character. Thus, setting shellxquote to \"
                causes code to execute that is not intended to run at all for the
                cmd.exe shell.
                --~--~---------~--~----~------------~-------~--~----~
                You received this message from the "vim_dev" maillist.
                For more information, visit http://www.vim.org/maillist.php
                -~----------~----~----~----~------~----~------~--~---
              • Bram Moolenaar
                ... I think this should only be done for cmd.exe, not for command.com. So the check should not be shell not matching sh but shell matching cmd . ...
                Message 7 of 12 , Nov 8, 2008
                • 0 Attachment
                  Ben Fritz wrote:

                  > On Nov 6, 11:20 pm, Tony Mechelynck <antoine.mechely...@...>
                  > wrote:
                  > > To avoid passing a path with spaces in it, use the "short form" of the
                  > > path, such as %:p:8 (this works only on Windows -- on Unix you can just
                  > > backslash-escape the spaces).
                  > >
                  > > This will convert the path so that "Program Files" becomes PROGRA~1, "My
                  > > Documents" MYDOCU~1 and, I think, "Copy of startup.m" COPYOF~1.M -- the
                  > > same directories and files will be accessed, but without the need for
                  > > quoting the path.
                  > >
                  > > Best regards,
                  > > Tony.
                  >
                  > Ah yes, this would be a reasonable solution in the general case. I
                  > have two issues with it, however:
                  >
                  > 1. It would require separate logic for Unix vs. Windows, whereas
                  > simply fixing the default value of shellxquote and shellcmdflag for
                  > Windows would allow the same logic to be used.
                  > 2. In the particular plugin I was working on, which highlights errors
                  > in MATLAB scripts using the mlint utility, system() is used to pass a
                  > file name to mlint. One of the checks that mlint does is to ensure
                  > that the file name is "formed from a legal MATLAB identifier." Using
                  > the short form of Windows path names, mlint will always return this
                  > error if the file name is longer than 8 characters (though
                  > incidentally, the file name is also invalid if it contains spaces, but
                  > theoretically the temporary directory it is stored in may have spaces
                  > in the path with no problems).
                  >
                  > So I think the better solution would be to fix the values of
                  > shellxquote and shellcmdflag, either by adding extra logic to the
                  > plugin, or by actually changing the defaults.
                  >
                  > For convienience, the suggested default values are:
                  >
                  > " for windows, always:
                  > set shellxquote=\"
                  > " for windows, when 'shell' does not contain "sh" somewhere:
                  > set shellcmdflag=/s\ /c
                  >
                  > This will place extra quotes around the entire command, and force the
                  > cmd.exe behavior that removes the first and last quote from the line
                  > if the first character is a quote.

                  I think this should only be done for cmd.exe, not for command.com. So
                  the check should not be "'shell' not matching sh" but "'shell' matching
                  cmd".

                  > I personally think that the current defaults break in enough cases
                  > that we should change them as suggested in this thread, but I can see
                  > the argument for keeping the current values to prevent plugins like
                  > taglist that escape quotes with backslashes depending on the value of
                  > shellxquote. Plugin developers may have assumed (as done in the
                  > taglist plugin) that if the cmd.exe shell is used, shellxquote will
                  > _not_ be equal to a " character. Thus, setting shellxquote to \"
                  > causes code to execute that is not intended to run at all for the
                  > cmd.exe shell.

                  Yes there is a risk. It's hard to tell what might break.

                  --
                  hundred-and-one symptoms of being an internet addict:
                  205. You're constantly yelling at your spouse, family, roommate, whatever,
                  for using the phone for stupid things...like talking.

                  /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
                  /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
                  \\\ download, build and distribute -- http://www.A-A-P.org ///
                  \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

                  --~--~---------~--~----~------------~-------~--~----~
                  You received this message from the "vim_dev" maillist.
                  For more information, visit http://www.vim.org/maillist.php
                  -~----------~----~----~----~------~----~------~--~---
                • Ben Fritz
                  ... That s probably a good idea, for now at least. I have no idea how command.com works. Someone who uses it should probably look this up. The help for cmd.exe
                  Message 8 of 12 , Nov 10, 2008
                  • 0 Attachment
                    On Nov 8, 7:24 am, Bram Moolenaar <B...@...> wrote:
                    >
                    > I think this should only be done for cmd.exe, not for command.com.  So
                    > the check should not be "'shell' not matching sh" but "'shell' matching
                    > cmd".

                    That's probably a good idea, for now at least. I have no idea how
                    command.com works. Someone who uses it should probably look this up.
                    The help for cmd.exe describes the behavior we are trying to force
                    with the new values of shellxquote and shellcmdflag as "old behavior,"
                    so perhaps command.com already uses it by default.
                    --~--~---------~--~----~------------~-------~--~----~
                    You received this message from the "vim_dev" maillist.
                    For more information, visit http://www.vim.org/maillist.php
                    -~----------~----~----~----~------~----~------~--~---
                  • Craig Barkhouse
                    ... This may work in a lot of cases, but it is not a good general solution. Firstly, not all filesystems generate short filenames. FAT32 and NTFS are capable
                    Message 9 of 12 , Nov 11, 2008
                    • 0 Attachment
                      Tony wrote:
                      > To avoid passing a path with spaces in it, use the "short form" of the
                      > path, such as %:p:8 (this works only on Windows -- on Unix you can just
                      > backslash-escape the spaces).
                      >
                      > This will convert the path so that "Program Files" becomes PROGRA~1, "My
                      > Documents" MYDOCU~1 and, I think, "Copy of startup.m" COPYOF~1.M -- the
                      > same directories and files will be accessed, but without the need for
                      > quoting the path.

                      This may work in a lot of cases, but it is not a good general solution. Firstly, not all filesystems generate short filenames. FAT32 and NTFS are capable of generating short filenames. Secondly, on NTFS at least, you can disable short filenames. I always disable it on my machines (there is some perf gain from doing so). Thirdly, NTFS probably won't support short filenames forever. Short filenames are already considered deprecated. In a future release of Windows, possibly the one after Windows 7, expect short filename support to disappear completely.

                      Craig


                      --~--~---------~--~----~------------~-------~--~----~
                      You received this message from the "vim_dev" maillist.
                      For more information, visit http://www.vim.org/maillist.php
                      -~----------~----~----~----~------~----~------~--~---
                    • Ben Fritz
                      I found another case where my proposed new default values break things. I had an implementation of a MyDiff function to use with diffexpr, that added the first
                      Message 10 of 12 , Nov 11, 2008
                      • 0 Attachment
                        I found another case where my proposed new default values break
                        things.

                        I had an implementation of a MyDiff function to use with diffexpr,
                        that added the first and last quote as part of its internal logic.
                        I.e., it was doing the job of my suggested shellxquote internally.
                        Naturally, adding a _third_ opening quote and another closing quote
                        screwed things up a little bit.

                        I'm not sure where I got the MyDiff function I was using...I checked
                        the most recent vimrc_example.vim and it is rather different. But I
                        think I got it from there at some point.

                        Nevertheless, failing to fix a problem because it will break work-
                        arounds for the same problem is not a good excuse in my opinion.
                        --~--~---------~--~----~------------~-------~--~----~
                        You received this message from the "vim_dev" maillist.
                        For more information, visit http://www.vim.org/maillist.php
                        -~----------~----~----~----~------~----~------~--~---
                      Your message has been successfully submitted and would be delivered to recipients shortly.