★ wanayoo — archive 1999 https://github.com/livecode/livecode/pull/7454Nouvelle recherche | Portail wanayoo
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[[ Bug 22942 ]] Ensure the shellcommand defaults to COMSPEC on Windows #7454

Merged
merged 1 commit into from Oct 19, 2020

Conversation

@livecodepanos
Copy link
Contributor

@livecodepanos livecodepanos commented Oct 15, 2020

Goes together with livecode/livecode-ide#2145

@livecodepanos livecodepanos added the bug label Oct 15, 2020
@livecodepanos livecodepanos added this to the 9.6.2-rc-1 milestone Oct 15, 2020
Copy link
Contributor

@runrevmark runrevmark left a comment

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!).

@@ -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)

This comment has been minimized.

@runrevmark

runrevmark Oct 16, 2020
Contributor

/* Default the shellCommand to the value of the COMSPEC environment
 * variable. */
// 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);

This comment has been minimized.

@runrevmark

runrevmark Oct 16, 2020
Contributor

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

This comment has been minimized.

@runrevmark

runrevmark Oct 16, 2020
Contributor

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?

@@ -0,0 +1 @@
# Ensure the shellcommand defaults to COMSPEC on Windows

This comment has been minimized.

@runrevmark

runrevmark Oct 16, 2020
Contributor

shellCommand

@livecodepanos livecodepanos force-pushed the livecodepanos:bugfix-22942 branch from b4d40db to d8cb11f Oct 16, 2020
@livecodepanos livecodepanos requested a review from runrevmark Oct 16, 2020
@runrevmark
Copy link
Contributor

@runrevmark runrevmark commented Oct 19, 2020

@livecode-vulcan
Copy link
Contributor

@livecode-vulcan livecode-vulcan commented Oct 19, 2020

💙 review by @runrevmark ok d8cb11f

livecode-vulcan added a commit that referenced this pull request Oct 19, 2020
[[ Bug 22942 ]] Ensure the shellcommand defaults to COMSPEC on Windows

Goes together with livecode/livecode-ide#2145
@livecode-vulcan
Copy link
Contributor

@livecode-vulcan livecode-vulcan commented Oct 19, 2020

😎 test success d8cb11f

  • try-community-armv6-android-sdk26_ndk16r15: success
  • try-community-armv7-android-ndk16r15: success
  • try-community-arm64-android-ndk16r15: success
  • try-community-x86-android-ndk16r15: success
  • try-community-x86_64-android-ndk16r15: success
  • try-community-js-emscripten-sdk1.35: success
  • try-community-universal-ios-iphoneos14.0: success
  • try-community-universal-ios-iphoneos13.5: success
  • try-community-universal-ios-iphonesimulator14.0: success
  • try-community-universal-ios-iphonesimulator13.5: success
  • try-community-universal-mac-macosx10.9: success
  • try-community-x86-linux-debian8: success
  • try-community-x86_64-linux-debian8: success
  • try-community-x86-win32: success
  • try-community-x86_64-win32: success
@livecodepanos livecodepanos merged commit 9bd23b2 into livecode:develop-9.6 Oct 19, 2020
4 checks passed
4 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/vulcan/cla Contributor Agreement signed by @livecodepanos
continuous-integration/vulcan/pr The Vulcan build succeeded on 15 builders
continuous-integration/vulcan/review Approved by reviewer @runrevmark
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.