-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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.
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
Conversation
|
I suggest taking a step back, in the context of our discussion to align PowerShell's CLI with that of POSIX-like shells (#3743): POSIX-like shells interpret an operand (in POSIX speak: an argument other than an option / option-argument) as a script file to execute rather than an arbitrary shell command. Translated into PowerShell terms that means:
If this change were made, no extra accommodations would have to made for shebang line - except that |
|
@mklement0 that would certainly be a breaking change, although it would align semantics with POSIX (but break many of our tests that expect the positional parameter to be a command). We'll discuss the implications at the next committee meeting (next week). |
…ithout a ps1 extension, powershell treats it as a native command so this ends up in a recursive loop. fix is to inspect the command to see if it is a shebang script and one we should handle. if so, just treat it like a ps1 script.
iSazonov
left a comment
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.
Leave a comment
| bool isShebangScript = false; | ||
| try | ||
| { | ||
| using(FileStream fileStream = new FileStream(path, FileMode.Open, FileAccess.Read)) |
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.
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.
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.
I wanted to limit how much gets read (first two bytes) since every file will get analyzed
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.
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.
| using(StreamReader file = new StreamReader(fileStream)) | ||
| { | ||
| string interpreter = file.ReadLine(); | ||
| System.Reflection.Assembly assembly = System.Reflection.Assembly.GetEntryAssembly(); |
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.
Maybe we should have an internal flag isInterprter?
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.
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
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.
I open PR #3969 to cache the reflection - we can use Utils.GetApplicationBaseDefaultPowerShell()
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.
- interpreter can not be supported on IOS
- First line effective length is 512 on IOS and 127 on Linux.
|
the more I think about this, I agree with @mklement0. there's really only two approaches:
|
|
Can we use a shebang like: |
|
I'm not sure this is the right fix. First, the difference between Second, I don't think we want shebang PowerShell scripts to run in process, which this change would allow. I believe the correct fix will involve detecting that we're invoked via shebang, which might mean looking at |
|
@iSazonov |
|
Fundamentally: Is aligning with the CLI of POSIX-like shells a goal in principle? If so:
POSIX-like shells have no analog to the
(Conversely, to pass an arbitrary shell command line, you must use the The shebang mechanism - built into the kernel of Unix platforms - relies on that very behavior: The path to the script file (exactly as specified when the script was directly invoked) is passed as an (operand) argument to the interpreter executable specified by full name in the shebang line, [edit] followed by the arguments given to the script on invocation. In terms of the command line that is invoked by the kernel, and the
A simple example: Let's assume script #!/bin/sh
echo "\$0: [$0]"
echo "Originating command line: [$(ps -o args= $$)]"Let's invoke that script directly, letting the kernel interpret the shebang line: Now let's see how we can change Note when a POSIX-like shell is explicitly invoked with a script filename operand - e.g.,
It would run in-process in the |
|
@mklement0 - if you run With the proposed change, running This should be a intentional design choice, not an accidental side effect of fixing the recursive hang. My inclination is that starting a new process is desirable (so perhaps more POSIX-like), but I'm open to a discussion, especially given PowerShell's slow startup cost. As for And as for |
Again, if we're talking about aligning with POSIX-like shells: I get that this is not how it currently works with invoking
What interpreter ultimately processes a given executable file that happens to have a shebang line should really be considered an implementation detail, and there's no reason to reflect that interpreter in the filename extension (even though, sadly, there's a widespread bad habit in the Unix world of naming shell scripts |
|
What is the behavior for other interpreters (bash, perl, python, etc.)? Run a native script in-process? |
|
perl follows the POSIX convention of having the positional parameter point to a script and using an explicit We should strongly consider adopting the POSIX conventions for PowerShell (which means closing this PR):
|
|
My question was how they behave internally: if we type in Bash "bash-script-file-name" - do Bash run it "in-process"? |
|
@SteveL-MSFT: An (interfaith) amen to that.
Interpreting all remaining arguments after the script filename as the arguments to pass through to the script is also part of "shebang-line compatibility", so all POSIX-like shells and Perl support it too; Ruby are Node.js are other examples. The ability to use Combining The first argument after the command string binds to $ sh -c 'printf "%s " "$@"' ARG0 one two
one two # !! Note how ARG0 is not listed.PowerShell does not support this, currently: # !! BROKEN
PS> powershell -noprofile -command '$args' one two
Unexpected token 'one' in expression or statement.
# !! BROKEN
PS> powershell -noprofile -command '"`$args: $args"' one two
$args : The term '$args' is not recognized as the name of a cmdlet, function, script file, or operable programIt looks like PowerShell simply string-appends the additional arguments directly to the command string, which, of course, only works in limited cases, whereas The only way to get this to work currently appears to be: $ powershell -noprofile -command '& { "`$args: $args" }' one two
$args: one twoFrom within Powershell:
# !! BROKEN when invoked from PowerShell
PS> powershell -noprofile -command '& { "`$args: $args" }' one two
$args: : The term '$args:' is not recognized as the name of a cmdlet, function, script file, or operable program.
# !! DOES NOT WORK - arguments not supported.
PS> powershell -noprofile -command { "`$args: $args" } one two
(prints CLI help text)
# Only works *without arguments*:
PS> powershell -noprofile -command { "`$args: [$args]" }
$args: []Other problematic aspects of |
No: The only way to make a POSIX-like shell run a script in-process in an existing session is to use Conversely, anything that is neither a shell builtin (command) nor a shell-language statement is considered an external utility to be invoked via a system call, which makes it inevitably run in a child process, and it is irrelevant (a) whether the script invoked is indeed a script or a binary and (b) if the former, what shell/interpreter will process it. Note, however, that this only works with executable shell scripts (whereas invocation with An otherwise well-formed script with shebang line that isn't executable [by the current user] will simply result in a If it is executable, a system call is made to invoke it, at which point the kernel's shebang-line processing will kick in. As a courtesy, if a script is executable but lacks a (valid) shebang line, POSIX-like shells will fall back to processing the script themselves, but - again - in a separate instance in a child process. On a side note: A UTF-8 BOM would break a shebang line. |
|
To more explicitly contrast the invoke-a-script-written-in-the-shell's-own-language scenarios:
Personally, I think the implicit and invariably global location-changing behavior is problematic. |
|
Another area is static members on classes. Maybe we should have syntactic sugar for invoking scripts easily out-of-proc. .\a.ps1
. .\a.ps1
! .\a.ps1 # -> powershell.exe -noprofile -file .\a.ps1 |
|
Or change the defaults before PowerShell v6 ships, so that you explicitly have to opt in to run in-proc. |
Good point, and definitely worth documenting, though probably less of a problem in practice.
Introducing another symbol-based operator may not be worth the effort (as an aside: I assume
As long as it doesn't come with surprises, I think that running in-process by default is a great strength:
|
|
@PowerShell/powershell-committee reviewed this and agree that the right change is to have -File be the positional parameter instead of -Command. Also allow scripts used with -File (or implicit) to not require a .ps1 extension. A new PR will be submitted. |
Fix #1103
On Linux, when a shebang is encountered (first two bytes are #!), it looks for the interpreter to pass the pass to. In this case, if it is powershell it calls powershell with a path to the script. If the script doesn't
have a .ps1 extension, powershell treats it like a native command and executes it as such. The OS
sees the shebang and again calls powershell with the script. This causes a hang (and will eventually consume all the OS resources as it keeps spawning new powershells).
Fix is to inspect the file to see if it contains the shebang magic number. If so, we check if the running
powershell is the interpreter. If so, we treat it the same as a .ps1 script. Otherwise, we execute it as
a native command and expect the OS to find the correct interpreter (could be different version of powershell, for example).