Skip to content

Fix {Bash,Cmd,Ps}Writer.IfCmd to escape its arguments

Nick Thomas requested to merge (removed):1820-fix-ifcmd-should-escape-its-args into master

What does this MR do?

Refactors the shell writers to use the same escaping logic in their Command() and IfCmd() methods

Why was this MR needed?

!359 (merged) started using IfCmd with URLs as arguments. URLs include various characters that need escaping, especially & in bash.

Are there points in the code the reviewer needs to double check?

The powershell and cmd changes look sane, but I haven't managed to run the executors/shell tests. Although I don't think they exercise the cache path anyway.

Does this MR meet the acceptance criteria?

  • Documentation created/updated
  • Tests
    • Added for this feature/bug
    • All builds are passing
  • Branch has no merge conflicts with master (if you do - rebase it please)

What are the relevant issue numbers?

Closes #1820 (closed)

/cc @ayufan @tmaczukin @stanhu

Merge request reports