Fix Windows command escape for \"

[Al2Klimov]

fixes #4849

[htriem]

@Al2Klimov Please add a test protocol for before and after.

[Al2Klimov]

Roger.

[Al2Klimov]

Test case

Config

object CheckCommand "args2idsl" {
	command = [ PluginDir + "/args2idsl.exe", "$args2idsl$" ]
}

object Host "lolcat" {
	check_command = "args2idsl"
	check_interval = 1s
	vars.args2idsl = {{{ !"#$$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\]^_`abcdefghijklmnopqrstuvwxyz{|}~
" \" \\" \\\" \\\\"}}}
}

Plugin

https://github.com/Al2Klimov/args2idsl

Test protocol

Before

After

[julianbrost]

Enough Windows Escaping for today, what a dumpster fire. I'll revisit this next year but one thing I already know that will be part of the review: test cases for this would be nice, ideally some that also pass the result through CommandLineToArgvW.

Notes for my future self that might be helpful for the review:

• https://docs.python.org/3.9/library/subprocess.html#converting-an-argument-sequence-to-a-string-on-windows
• https://github.com/python/cpython/blob/v3.9.1/Lib/subprocess.py#L529-L596

[julianbrost]

I wrote some test for this, see https://github.com/Icinga/icinga2/compare/jbrost/windows-escape-args (that is your commit rebased to the current master + a commit adding tests).

Without your fix:

   70/110 Test  #70: base-base_utility/EscapeCreateProcessArg ................................***Failed    0.58 sec
  Running 1 test case...
C:/Users/Julian Brost/source/repos/icinga2/test/base-utility.cpp(107): error : in "base_utility/EscapeCreateProcessArg": CommandLineToArgvW() should return original value for foo"bar (got: foobar) [C:\Users\Julian Brost\source\repos\icinga2\build\RUN_TESTS.vcxproj]
C:/Users/Julian Brost/source/repos/icinga2/test/base-utility.cpp(103): error : in "base_utility/EscapeCreateProcessArg": CommandLineToArgvW() should find 2 arguments for "foo bar" [C:\Users\Julian Brost\source\repos\icinga2\build\RUN_TESTS.vcxproj]
C:/Users/Julian Brost/source/repos/icinga2/test/base-utility.cpp(107): error : in "base_utility/EscapeCreateProcessArg": CommandLineToArgvW() should return original value for "foo bar" (got: foo) [C:\Users\Julian Brost\source\repos\icinga2\build\RUN_TESTS.vcxproj]
C:/Users/Julian Brost/source/repos/icinga2/test/base-utility.cpp(103): error : in "base_utility/EscapeCreateProcessArg": CommandLineToArgvW() should find 2 arguments for " \" \\" \\\" \\\\" [C:\Users\Julian Brost\source\repos\icinga2\build\RUN_TESTS.vcxproj]
C:/Users/Julian Brost/source/repos/icinga2/test/base-utility.cpp(107): error : in "base_utility/EscapeCreateProcessArg": CommandLineToArgvW() should return original value for " \" \\" \\\" \\\\" (got: ) [C:\Users\Julian Brost\source\repos\icinga2\build\RUN_TESTS.vcxproj
]
C:/Users/Julian Brost/source/repos/icinga2/test/base-utility.cpp(103): error : in "base_utility/EscapeCreateProcessArg": CommandLineToArgvW() should find 2 arguments for  !"#$$%&'()*+,-./09:;<=>?@AZ[\]^_`az{|}~ " \" \\" \\\" \\\\" [C:\Users\Julian Brost\source\repos\ici
nga2\build\RUN_TESTS.vcxproj]
C:/Users/Julian Brost/source/repos/icinga2/test/base-utility.cpp(107): error : in "base_utility/EscapeCreateProcessArg": CommandLineToArgvW() should return original value for  !"#$$%&'()*+,-./09:;<=>?@AZ[\]^_`az{|}~ " \" \\" \\\" \\\\" (got:  !#$$%&'()*+,-./09:;<=>?@AZ[
\]^_`az{|}~) [C:\Users\Julian Brost\source\repos\icinga2\build\RUN_TESTS.vcxproj]

  *** 7 failures are detected in the test module "icinga2"

With your fix:

          Start  70: base-base_utility/EscapeCreateProcessArg
   70/110 Test  #70: base-base_utility/EscapeCreateProcessArg ................................   Passed    0.75 sec

There are still two things that could be improved about the tests:

• Currently just the function body is in #ifdef _WIN32, maybe this can be done better and the whole test can be excluded on non-Windows.
• On my machine, when running the test, it complains about leaked memory, but I don't see where this should leak anything. (Failed) tests additionally output the following:

  Detected memory leaks!
  Dumping objects ->
  {29934} normal block at 0x000001F773C3F890, 88 bytes long.
   Data: <   s       s    > D0 EF C3 73 F7 01 00 00 10 E7 C3 73 F7 01 00 00
  {29933} normal block at 0x000001F773C3E710, 88 bytes long.
   Data: <p  s       s    > 70 F0 C3 73 F7 01 00 00 D0 EF C3 73 F7 01 00 00
  {29932} normal block at 0x000001F773C3F070, 88 bytes long.
   Data: <   s       s    > D0 EF C3 73 F7 01 00 00 10 E7 C3 73 F7 01 00 00
  {29931} normal block at 0x000001F773BD1D90, 16 bytes long.
   Data: <X  s            > 58 1A 90 73 F7 01 00 00 00 00 00 00 00 00 00 00
  {29930} normal block at 0x000001F773C3EFD0, 88 bytes long.
   Data: <p  s       s    > 70 F0 C3 73 F7 01 00 00 10 E7 C3 73 F7 01 00 00
  

[Al2Klimov]

Tests LGTM. No idea where a memory leak could come from. Feel free to push the tests commit on this branch.