Skip to content

Conversation

@salva
Copy link

@salva salva commented Jul 3, 2017

On Windows, in the case where several arguments were given, the one
corresponding to the executable name was handled in a special way,
resolving and then converting it to its short version using
GetShortPathName.

Unfortunatelly, GetShortPathName is implemented at the file system
level and doesn't guarantee a file name that doesn't require quoting.
Specifically, when using VirtualBox shares, GetShortPathName can
return names with spaces inside.

In that case, the executable name with spaces was inserted unquoted
into the command line causing the invoked program to fail to parse its
arguments correctly.

For instance, running ["y:/foo bar/my.exe", "/d"] would result in the
invokation of "y:/foo bar/my.exe" with the command line "y:/foo
bar/my.exe /d" which would be break as ["y:/foo", "bar/my.exe", "/d"].

This patch, still resolves and normalizes the executable name before
passing it to Win32::Process::Create, but then when composing the
command line, it uses the former name as given by the user after
quoting it properly.

On Windows, in the case where several arguments were given, the one
corresponding to the executable name was handled in a special way,
resolving and then converting it to its short version using
GetShortPathName.

Unfortunatelly, GetShortPathName is implemented at the file system
level and doesn't guarantee a file name that doesn't require quoting.
Specifically, when using VirtualBox shares, GetShortPathName can
return names with spaces inside.

In that case, the executable name with spaces was inserted unquoted
into the command line causing the invoked program to fail to parse its
arguments correctly.

For instance, running ["y:/foo bar/my.exe", "/d"] would result in the
invokation of "y:/foo bar/my.exe" with the command line "y:/foo
bar/my.exe /d" which would be break as ["y:/foo", "bar/my.exe", "/d"].

This patch, still resolves and normalizes the executable name before
passing it to Win32::Process::Create, but then when composing the
command line, it uses the former name as given by the user after
quoting it properly.
@salva
Copy link
Author

salva commented Sep 20, 2017

@blair, just in case you are too busy with other things I would like to volunteer for making a new release of Proc::Background with this fix.

@salva
Copy link
Author

salva commented Oct 24, 2017

ping!

@blair
Copy link
Owner

blair commented Oct 27, 2017

Hi, I've handed over maintenance to another developer. I've asked him for his GitHub username and will cc him here when I get it.

@nrdvana
Copy link

nrdvana commented Nov 2, 2017

Hi. This looks good with my planned changes. (I'm planning to add support for detecting an appropriate interpreter in the style of the Linux kernel, for files which are not executables)

@salva
Copy link
Author

salva commented Feb 27, 2018

@nrdvana, ping!

As I had already told the previous maintainer, If you are too busy on other things, I volunteer for making a new release with this fix (or even taking maintenance of the module completely).

@nrdvana
Copy link

nrdvana commented Feb 28, 2018

Wow, sorry for letting this go so long. @blair did you want us to continue with this repository (I don't have write access yet) or start one on my or @salva 's profile?

I do still have an unfinished patch set I wanted to add, and it's invasive enough that I figured I should manage any bugs that result from it, but I don't care whether I'm the maintainer or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants