Problems sanitizing strings on Windows

[wpferguson]

Users are having issues executing external commands on Windows.

One case is probably not being sanitized enough, https://discuss.pixls.us/t/lua-scripts-doesnt-work-with-darktable-4-8-0-for-windows/44434 and the other is from being sanitized too much, https://github.com/darktable-org/darktable/issues/17108.

[robyquin]

I used line 56 also for my windows os and It works fine for me

https://github.com/darktable-org/lua-scripts/blob/7c93868bf016969d1bf814c2df59bd77b5c237dc/lib/dtutils/system.lua#L50-L60

[wpferguson]

After thinking about this I came up with 2 solutions:

• Remove all sanitizing from the scripts and do it all in the libraries, or
• Remove all sanitizing from the libraries and depend on the scripts to pass in sanitized arguments

My choice came down to which was easier to troubleshoot:

• if sanitizing is in the libraries and there is a problem with a script, is the problem in the script or the libraries?
• if sanitizing is removed from the libraries and there is a problem with a script then the problem is most likely in the script.

My plan is this:

• leave the sanitizing functions in the libraries (i.e. sanitize(), sanitize_lua(), etc)
• remove any use of the sanitize functions in the libraries (don't sanitize command arguments)
• set up test environments on windows and linux with every sanitizing problem I can think of then test each script to figure out what needs sanitized and how.

[jsmucr]

I think this is not where one would expect that kind of sanitization:

local fname = ds.sanitize(dt.configuration.tmp_dir .. "/run_command.bat")
local file = io.open(fname, "w")

The function attempts to create (literally):

"C:\Users\user\AppData\Local\Temp/run_command.bat"

This works:

local fname = dt.configuration.tmp_dir .. "\\run_command.bat"
local file = io.open(fname, "w")

But only until it crashes again:

Z:\>"""^""E:\darktable-tools\gmic-3.4.0-cli-win64\gmic.exe""" """C:\Users\user\AppData\Local\Temp\P1327884.tif""" -deblur_richardsonlucy 0.70,10,1 -/ 256 cut 0,255 round o """E:\sharp-exports\P1327884.jpg""",100""
The filename, directory name, or volume label syntax is incorrect.
    43,4158 LUA result from windows command was 1

Those multiple escaping attempts are pretty wild. :-) I'm no LUA expert but usually when I'm dealing with this in other languages, the best approach happens to be to compose the command from individual array items. Like for example in Bash:

#!/bin/bash

ARG_FROM_THE_OUTSIDE=$1
COMMAND=('my command with spaces')
COMMAND+=("$ARG_FROM_THE_OUTSIDE")
COMMAND+=('1' '2' '3')

"${COMMAND[@]}"

It would help if windows_command() didn't compose a .bat file at all, but rather call the command directly. I understand that os.execute() requires the command to be escaped. Passing the command as an array would untie the library users' hands, and all escaping could be done on one place. Something like:

function do_something()
    run_command({'executable', 'arg1', 'arg2', ':/;^*$foo'})
end

function run_command(command)
    if dt.configuration.running_os == 'windows' then
        return os.execute(table.concat(escape_windows_command(command), ' '))
    end
    return os.execute(table.concat(escape_posix_command(command), ' '))
end

[wpferguson]

Here's the problem I was trying to fix:

• I'm on windows and my user name is user name
• I want my folder names to be descriptive, i.e. 2023 vacation to Greece
• I want to process an image that I started in darktable and want to finish in GIMP with the GIMP executable at C:\Program Files\GIMP 2\gimp.exe

To get around all the spaces in the username and the path, you have to wrap the string in double quotes, i.e. "C:\Users\user name\Pictures\2023 vacation to Greece\first day\_DSC0001.arw"

To get around the spaces in the executable string, you have to also wrap that in double quotes, i.e. "C:\Program Files\GIMP 2\gimp.exe"

And, to run it as a command you have to wrap both of the double quoted strings in double quotes, i.e. ""C:\Program Files\GIMP 2\gimp.exe" "C:\Users\user name\Pictures\2023 vacation to Greece\first day\_DSC0001.arw""

I solved this in scripts_installer and it worked well, so I figured that porting it to the libraries would be a good idea. The part I didn't consider was that I controlled all the strings in scripts_installer and knew what needed sanitized versus what didn't. In the full lua-scripts environment I don't control all the strings so I get a mixture of sanitized and not sanitized, and when I sanitize them in the libraries I sometimes end up double sanitizing or quoting improperly, hence this issue and the other related ones and discussions.

The final, and probably most difficult, is testing all of this. I need user names with and without spaces, image paths with and without spaces, and finally executable paths with and without spaces on linux and windows. I don't think I can create a hackintosh any more, so I'll just have to trust that linux is close enough. Then I have to test all the combinations for all the scripts.

I'll probably do the fix in 2 steps, the first rolling back the last round of "fixes", and the second trying to solve this problem once and for all.

It would help if windows_command() didn't compose a .bat file at all, but rather call the command directly.

We went to the batch file because darktable.control.execute couldn't handle multiple quoted items. Having just reread the reason, we might be able to enclose the whole mess in double quotes and darktable.control.execute might work. I'll test it with all the other stuff.