[[ Bug 22942 ]] Ensure the shellcommand defaults to COMSPEC on Windows #7454
[[ Bug 22942 ]] Ensure the shellcommand defaults to COMSPEC on Windows #7454
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this patch there is the possibility that the shellCommand could be empty. There is no way to correctly guess what to use if COMSPEC is empty so we should make sure that shell() does not work if it is (indeed, this would be sensible on all platforms!)
In MCFilesEvalShell after the secureMode check do:
if (MCStringIsEmpty(MCshellcmd))
{
MCeerror->add(EE_SHELL_BADCOMMAND, 0, 0, "no shell");
ctxt . Throw();
return;
}
This will cause an error to be thrown if the shellCommand is empty - rather than it trying to run launch a process with a malformed command line (which will probably fail anyway, but this stops the attempt!).
engine/src/dskw32.cpp
Outdated
| @@ -1534,8 +1534,8 @@ struct MCWindowsDesktop: public MCSystemInterface, public MCWindowsSystemService | |||
| } | |||
| } | |||
|
|
|||
| // On NT systems 'cmd.exe' is the command processor | |||
| MCValueAssign(MCshellcmd, MCSTR("cmd.exe")); | |||
| // On NT systems use the env var COMSPEC (= C:\Windows\System32\cmd.exe) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/* Default the shellCommand to the value of the COMSPEC environment
* variable. */
engine/src/dskw32.cpp
Outdated
| // On NT systems 'cmd.exe' is the command processor | ||
| MCValueAssign(MCshellcmd, MCSTR("cmd.exe")); | ||
| // On NT systems use the env var COMSPEC (= C:\Windows\System32\cmd.exe) | ||
| MCS_getenv(MCSTR("COMSPEC"), MCshellcmd); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we should use MCValueAssign here like elsewhere:
MCAutoStringRef t_comspec;
if (MCS_getenv(MCSTR("COMSPEC"), &t_comspec))
{
MCValueAssign(MCshellcmd, *t_comspec);
}
else
{
MCValueAssign(MCshellcmd, kMCEmptyString);
}
| @@ -22,8 +22,8 @@ set the shellCommand to "/bin/sh/ksh" | |||
| Value: | |||
| The <shellCommand> is a string. | |||
| By default, the <shellCommand> <property> is set to "/bin/sh" (the | |||
| Bourne shell) on <Unix|Unix systems>, and "command.com" on Windows | |||
| systems. | |||
| Bourne shell) on <Unix|Unix systems>, and to the environment variable "COMSPEC" on Windows | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and to the value of the COMSPEC environment variable on Windows systems.
(I don't think there's a need to say anymore than that).
Should Windows be referred to like <Windows|Windows systems> as it is for Unix?
docs/notes/bugfix-22942.md
Outdated
| @@ -0,0 +1 @@ | |||
| # Ensure the shellcommand defaults to COMSPEC on Windows | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shellCommand
b4d40db
to
d8cb11f
Compare
|
@livecode-vulcan review ok d8cb11f |
|
|
[[ Bug 22942 ]] Ensure the shellcommand defaults to COMSPEC on Windows Goes together with livecode/livecode-ide#2145
|
Goes together with livecode/livecode-ide#2145