escapeshellarg improvements

[pocomane]

escapeshellarg (and so shelljoin) does not handle some cases. At my understanding these functions aim to produce strings that can be passed to os.execute bypassing all the shell pre-parsing. If this is the intent, the following example gives a wrong output:

print(require 'shelljoin' {' a\\"',' b\\', 'c $PATH'})

Indeed:

• '\' should be escaped to avoid unwanted escapes
• '$' should be escaped to avoid variable expansion

Moreover these functions work only with a posix shell. I think at least windows should be supported. Maybe you could use something like:

local function posix_quote_argument(str)
  -- if not str:match("'") then
  --   return "'" .. str .. "'"
  -- end
  str = str:gsub("[$`\"\\]", "\\%1") 
  return '"' .. str .. '"'
end

local function windows_quote_argument(str)
  -- str = str:gsub('["\\]', '\\%1')
  -- return '"'..str..'"'
  str = str:gsub('[%%&\\^<>|]', '^%1')
  str = str:gsub('"', "\\%1")
  str = str:gsub('[ \t][ \t]*', '"%1"')
  return str
end

local function detect_the_shell()
  local shell = os.getenv('SHELL')
  if shell then
    if '/' == shell:sub(1,1) and 'sh' == shell:sub(-2, -1) then
      return posix_quote_argument
    end
  end
  return windows_quote_argument
end

(actually this came from some my old code that was also tested a bit). I did not a comit+pull request since I do not know if this is the desired behaviour.

[aiq]

At my understanding these functions aim to produce strings that can be passed to os.execute bypassing all the shell pre-parsing.

Thats right.
But does shelljoin not escape '' correct?

Moreover these functions work only with a posix shell.

I use OSX and Ubuntu and had no need for Windows Support. But you are right it would improve the function.

You can extend the existing tests with the problem cases and the expected output. I can add this function or do it yourself.

[pocomane]

I can update the function by myself for sure. However I was guessing if it was better to actually involve a shell itself for the tests. Something like:

local escapeshellarg = require( "escapeshellarg" )
local t = require( "taptest" )

local function shcompare( got, expected )
  local w = require( "writefile" )
  local r = require( "readfile" )
  w( 'tmp.lua', [[
    for i=1,#arg do
      require'writefile'('tmp'..i..'.txt',arg[i])
    end
  ]] )
  local i, luacmd = 0, 'lua'
  while arg[i] do luacmd = arg[i] ; i = i -1 end
  os.execute ( luacmd .. [[ tmp.lua ]] .. got .. ' ' )
  return r( 'tmp1.txt' ) == expected
end

t( escapeshellarg( [[Hello's world]] ),     [[Hello's world]],    shcompare)
t( escapeshellarg( [[use a " to mark]] ),   [[use a " to mark]],  shcompare )
t( escapeshellarg( [[should escape \]] ),   [[should escape \]],  shcompare )

t( escapeshellarg( [[special $PATH]] ),     [[special $PATH]],    shcompare )
t( escapeshellarg( [[special %PATH%]] ),    [[special %PATH%]],   shcompare )

t( escapeshellarg( [[redirect>o.txt]] ),    [[redirect>o.txt]],   shcompare )
t( escapeshellarg( [[redirect<i.txt]] ),    [[redirect<i.txt]],   shcompare )
t( escapeshellarg( [[pipe|extcmd]] ),       [[pipe|extcmd]],      shcompare )

t()

Note: I can not test right now however I thing the current code will failing the '\' and '$' tests in the example.