Skip to content

shebang without .ps1 extension ends up in recursive loop calling powershell #3963

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.

Sign up for GitHub

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

Closed
SteveL-MSFT wants to merge 1 commit into from
Closed

shebang without .ps1 extension ends up in recursive loop calling powershell #3963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion src/System.Management.Automation/engine/CommandSearcher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,41 @@ private CommandInfo GetInfoFromPath(string path)
break;
}

if (String.Equals(extension, StringLiterals.PowerShellScriptFileExtension, StringComparison.OrdinalIgnoreCase))
// handle case where we are called by the OS to handle a shebang script using us as the interpreter
bool isShebangScript = false;
try
{
using(FileStream fileStream = new FileStream(path, FileMode.Open, FileAccess.Read))
Copy link
Collaborator

@iSazonov iSazonov Jun 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a lot of code! Can we just read the line (with length < Path.Max) and check Shebang with RegEx.IsMatch()? And it seems we're going to have less trouble with BOM.

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT Jun 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to limit how much gets read (first two bytes) since every file will get analyzed

Copy link
Collaborator

@iSazonov iSazonov Jun 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In either case, the entire file will be read by OS for native processing or PowerShell processing. The question is only in this local buffer. I guess 64K is not a problem.

{
byte[] bytes = new byte[2];
int bytesRead = fileStream.Read(bytes, 0, 2); // read the first two bytes to determine if shebang
if (bytesRead == 2)
{
if (bytes[0] == '#' && bytes[1] == '!')
{
// see if we are supposed to be the interpreter
using(StreamReader file = new StreamReader(fileStream))
{
string interpreter = file.ReadLine();
System.Reflection.Assembly assembly = System.Reflection.Assembly.GetEntryAssembly();
Copy link
Collaborator

@iSazonov iSazonov Jun 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should have an internal flag isInterprter?

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT Jun 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should certainly cache this since this if using shebang with PowerShell becomes popular. I have to explicitly check if the interpreter is exactly the same as the running PowerShell otherwise I let the OS handle it as we would want to support scripts that target specific versions of PowerShell instead of just the system default one. Actually this reminds me that the current code won't work with

#!/usr/bin/env powershell

Copy link
Collaborator

@iSazonov iSazonov Jun 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I open PR #3969 to cache the reflection - we can use Utils.GetApplicationBaseDefaultPowerShell()

Copy link
Collaborator

@iSazonov iSazonov Jun 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting tests

  1. interpreter can not be supported on IOS
  2. First line effective length is 512 on IOS and 127 on Linux.

// this returns path to powershell.dll
string powershellPath = assembly.Location.Replace(".dll","");
// need to handle both powershell and powershell.exe
if (interpreter.Split(' ',2)[0].Replace(".exe","") == powershellPath)
{
isShebangScript = true;
}
}
}
}
}
}
catch(Exception)
{
// If we can't read the file, safe to assume it's not a script
}

if (isShebangScript || String.Equals(extension, StringLiterals.PowerShellScriptFileExtension, StringComparison.OrdinalIgnoreCase) || isShebangScript)
{
if ((_commandTypes & CommandTypes.ExternalScript) != 0)
{
Expand Down
12 changes: 12 additions & 0 deletions test/powershell/Host/ConsoleHost.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,18 @@ Describe "ConsoleHost unit tests" -tags "Feature" {
# no extraneous output
$observed | should be $currentVersion
}

$testCases =
@{interpreter = "#!${pshome}/powershell"},
@{interpreter = "#!${pshome}/powershell.exe"}

It "Should properly execute shebang script that doesn't have .ps1 extension" -TestCases $testCases {
param($Interpreter)
$scriptPath = "~/shebangtest"
Set-Content -Path $scriptPath -Value $Interpreter
Add-Content -Path $scriptPath -Value "'hello'"
& $powershell $scriptPath | Should BeExactly 'hello'
}
}

Context "Pipe to/from powershell" {
Expand Down