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.

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

Jump to bottom

Terminal crashes on wt . #12535

Closed
hello-smile6 opened this issue Feb 19, 2022 · 8 comments · Fixed by #12538
Closed

Terminal crashes on wt . #12535

hello-smile6 opened this issue Feb 19, 2022 · 8 comments · Fixed by #12538
Labels
Area-Commandline wt.exe's commandline arguments Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal v1.14

Comments

@hello-smile6
Copy link

Windows Terminal version

1.12.10393.0

Windows build number

19043.1469

Other Software

  • Command Prompt
  • Windows Explorer
    screenshot

Steps to reproduce

  1. Open Windows Terminal.
  2. Open Command Prompt using a non-Windows Terminal context.
  3. Execute wt . in Command Prompt.
    (May also happen with Git Bash)

Expected Behavior

I was expecting to have Windows Terminal open in the current directory.

Actual Behavior

Windows Terminal crashed and I lost about 30 minutes of work.
@hello-smile6 hello-smile6 added the Issue-Bug It either shouldn't be doing this or needs an investigation. label Feb 19, 2022
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Feb 19, 2022
@hello-smile6
Copy link
Author

Just got another error.

[error 2147942402 (0x80070002) when launching `%cd%']

@237dmitry
Copy link

I was expecting to have Windows Terminal open in the current directory.

wt --startingDirectory .

@DHowett
Copy link
Member

DHowett commented Feb 19, 2022

Yes, the argument after wt is a command to run (as in, wt powershell.)

You need to use wt -d xxx to set the starting directory.

@DHowett DHowett added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Feb 19, 2022
@hello-smile6
Copy link
Author

Yes, the argument after wt is a command to run (as in, wt powershell.)

You need to use wt -d xxx to set the starting directory.

If that is the case, then what about programs that use Windows Terminal without making sure the target binary exists?

@ghost ghost added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Feb 19, 2022
@DHowett
Copy link
Member

DHowett commented Feb 20, 2022

It should always display an error (as you noted in your second comment) in a new terminal tab/window/pane. If it crashes, that is a bug we should fix.

@ianjoneill ianjoneill mentioned this issue Feb 20, 2022
Merged
6 tasks
@ghost ghost added the In-PR This issue has a related PR label Feb 20, 2022
@hello-smile6 hello-smile6 mentioned this issue Feb 21, 2022
Closed
@zadjii-msft zadjii-msft added Area-Commandline wt.exe's commandline arguments Priority-1 A description (P1) Product-Terminal The new Windows Terminal. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Attention The core contributors need to come back around and look at this ASAP. labels Feb 22, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Feb 22, 2022
@zadjii-msft zadjii-msft added this to the Terminal v1.14 milestone Feb 22, 2022
@ghost ghost closed this as completed in #12538 Feb 22, 2022
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Feb 22, 2022
ghost pushed a commit that referenced this issue Feb 22, 2022
@ianjoneill
Don't crash trying to parse a command line that's a directory (#12538)
722aafa 
## Summary of the Pull Request
Prevents a crash that could occur when invoking `wt C:\`

## PR Checklist
* [x] Closes #12535
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

## Detailed Description of the Pull Request / Additional comments
Updates `CascadiaSettings::NormalizeCommandLine()` to check that there are a suitable number of command line arguments to be concatenated together, to prevent accessing an array index in `argv` that doesn't exist.

Also prevents a test flake that could occur in `TerminalSettingsTests::CommandLineToArgvW()`, due to generating an empty command line argument.

## Validation Steps Performed
Added a test, and checked that invoking each of the command lines below behaved as expected:
```
wtd C:\ # Window pops up with [error 2147942405 (0x80070005) when launching `C:\']
wtd C:\Program Files # Window pops up with [error 2147942402 (0x80070002) when launching `C:\Program Files']
wtd cmd # cmd profile pops up
wtd C:\Program Files\Powershell\7\pwsh -WorkingDirectory C:\ # PowerShell profile pops up in C:\
wtd "C:\Program Files\Powershell\7\pwsh" -WorkingDirectory C:\ # PowerShell profile pops up in C:\
wtd . # Window pops up with [error 2147942405 (0x80070005) when launching `.']
```
miniksa pushed a commit that referenced this issue Mar 3, 2022
@ianjoneill @miniksa
Don't crash trying to parse a command line that's a directory (#12538)
b005ffc 
## Summary of the Pull Request
Prevents a crash that could occur when invoking `wt C:\`

## PR Checklist
* [x] Closes #12535
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

## Detailed Description of the Pull Request / Additional comments
Updates `CascadiaSettings::NormalizeCommandLine()` to check that there are a suitable number of command line arguments to be concatenated together, to prevent accessing an array index in `argv` that doesn't exist.

Also prevents a test flake that could occur in `TerminalSettingsTests::CommandLineToArgvW()`, due to generating an empty command line argument.

## Validation Steps Performed
Added a test, and checked that invoking each of the command lines below behaved as expected:
```
wtd C:\ # Window pops up with [error 2147942405 (0x80070005) when launching `C:\']
wtd C:\Program Files # Window pops up with [error 2147942402 (0x80070002) when launching `C:\Program Files']
wtd cmd # cmd profile pops up
wtd C:\Program Files\Powershell\7\pwsh -WorkingDirectory C:\ # PowerShell profile pops up in C:\
wtd "C:\Program Files\Powershell\7\pwsh" -WorkingDirectory C:\ # PowerShell profile pops up in C:\
wtd . # Window pops up with [error 2147942405 (0x80070005) when launching `.']
```
DHowett pushed a commit that referenced this issue Mar 10, 2022
@ianjoneill @DHowett
Don't crash trying to parse a command line that's a directory (#12538)
0a8cb7d 
## Summary of the Pull Request
Prevents a crash that could occur when invoking `wt C:\`

## PR Checklist
* [x] Closes #12535
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

## Detailed Description of the Pull Request / Additional comments
Updates `CascadiaSettings::NormalizeCommandLine()` to check that there are a suitable number of command line arguments to be concatenated together, to prevent accessing an array index in `argv` that doesn't exist.

Also prevents a test flake that could occur in `TerminalSettingsTests::CommandLineToArgvW()`, due to generating an empty command line argument.

## Validation Steps Performed
Added a test, and checked that invoking each of the command lines below behaved as expected:
```
wtd C:\ # Window pops up with [error 2147942405 (0x80070005) when launching `C:\']
wtd C:\Program Files # Window pops up with [error 2147942402 (0x80070002) when launching `C:\Program Files']
wtd cmd # cmd profile pops up
wtd C:\Program Files\Powershell\7\pwsh -WorkingDirectory C:\ # PowerShell profile pops up in C:\
wtd "C:\Program Files\Powershell\7\pwsh" -WorkingDirectory C:\ # PowerShell profile pops up in C:\
wtd . # Window pops up with [error 2147942405 (0x80070005) when launching `.']
```

(cherry picked from commit 722aafa)
@ghost
Copy link

ghost commented Mar 25, 2022

🎉This issue was addressed in #12538, which has now been successfully released as Windows Terminal v1.12.1073.:tada:

Handy links:

@ghost
Copy link

ghost commented Mar 25, 2022

🎉This issue was addressed in #12538, which has now been successfully released as Windows Terminal Preview v1.13.1073.:tada:

Handy links:

@AdrX003
Copy link

AdrX003 commented Jul 25, 2023

Yes, the argument after wt is a command to run (as in, wt powershell.)

You need to use wt -d xxx to set the starting directory.

ive made it work by using %~dp0 , been trying different things for this for hours yesterday! 🥳

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-Commandline wt.exe's commandline arguments Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal v1.14
Development

Successfully merging a pull request may close this issue.

Don't crash trying to parse a command line that's a directory ianjoneill/terminal
5 participants
@DHowett @zadjii-msft @AdrX003 @hello-smile6 @237dmitry