Windows Terminal crashes when a profile&#39;s icon URL or background is unparseable/invalid #3265

@AdamDanischewski commented on GitHub (Aug 10, 2019):

Lol, yea, I meant Professional - I haven't used VS before but I see Community VS is full featured. I'm surprised these critical bugs aren't getting fixed quicker.

I've been out of the loop for a few years touristing, interesting to see how far MS has embraced Open Source. Once I get a dev env set up I'll probably contribute.

@AdamDanischewski commented on GitHub (Aug 10, 2019): Lol, yea, I meant Professional - I haven't used VS before but I see Community VS is full featured. I'm surprised these critical bugs aren't getting fixed quicker. I've been out of the loop for a few years touristing, interesting to see how far MS has embraced Open Source. Once I get a dev env set up I'll probably contribute.

claunia commented

@ghost commented on GitHub (Aug 27, 2019):

:tada:This issue was addressed in #2376, which has now been successfully released as Windows Terminal Preview v0.4.2382.0.🎉

Handy links:

@ghost commented on GitHub (Aug 27, 2019): :tada:This issue was addressed in #2376, which has now been successfully released as `Windows Terminal Preview v0.4.2382.0`.:tada: Handy links: * [Release Notes](https://github.com/microsoft/terminal/releases/tag/v0.4.2382.0) * [Store Download](https://www.microsoft.com/store/apps/9n0dx20hk701?cid=storebadge&ocid=badge)

claunia commented

@AdamDanischewski commented on GitHub (Sep 2, 2019):

This issue is still not fixed as of v0.4.2382.0 - please re-open and de-merge this issue from #2376 since #2376 is fixed and this issue is not.

@AdamDanischewski commented on GitHub (Sep 2, 2019): This issue is still not fixed as of v0.4.2382.0 - please re-open and de-merge this issue from #2376 since #2376 is fixed and this issue is not.

claunia commented

@DHowett-MSFT commented on GitHub (Sep 3, 2019):

Interesting. I had to try about six different variants before I was able to find one that would crash.

@DHowett-MSFT commented on GitHub (Sep 3, 2019): Interesting. I had to try about six different variants before I was able to find one that would crash.

claunia commented

@mdtauk commented on GitHub (Sep 3, 2019):

There should be a default icon that a profile will revert to, when the specified icon is not found

@mdtauk commented on GitHub (Sep 3, 2019): There should be a default icon that a profile will revert to, when the specified icon is not found

claunia commented

@zadjii-msft commented on GitHub (Sep 3, 2019):

Interesting. I had to try about six different variants before I was able to find one that would crash.

@DHowett-MSFT Mind sharing what URL variant caused the crash? Or filing a new issue with that variant?

@zadjii-msft commented on GitHub (Sep 3, 2019): > > > Interesting. I had to try about six different variants before I was able to find one that would crash. @DHowett-MSFT _Mind sharing what URL variant caused the crash?_ Or filing a new issue with that variant?

claunia commented

@DHowett-MSFT commented on GitHub (Sep 4, 2019):

"icon": "/beeftown.png",

It can't construct a Uri.

Exception thrown at 0x00007FFCF5715369 (KernelBase.dll) in WindowsTerminal.exe:
WinRT originate error - 0x80070057 : '/beeftown.png is not a valid absolute URI.'.

@DHowett-MSFT commented on GitHub (Sep 4, 2019): ```json "icon": "/beeftown.png", ``` It can't construct a `Uri`. ``` Exception thrown at 0x00007FFCF5715369 (KernelBase.dll) in WindowsTerminal.exe: WinRT originate error - 0x80070057 : '/beeftown.png is not a valid absolute URI.'. ```

claunia commented

@thomthom commented on GitHub (Oct 1, 2019):

I just ran into this when I tried to use environmental variables in the icon path.

This will crash in 0.5.2681.0

"icon": "%HOMEPATH%\\Dropbox\\terminal\\VSWinIcon_100x.png",

This will not.

"icon": "C:\\Users\\Thomas\\Dropbox\\terminal\\VSWinIcon_100x.png",

I realise that env variables might not be supported for icons. But just wanted to throw in my crash scenario.

@thomthom commented on GitHub (Oct 1, 2019): I just ran into this when I tried to use environmental variables in the icon path. This will crash in 0.5.2681.0 ``` "icon": "%HOMEPATH%\\Dropbox\\terminal\\VSWinIcon_100x.png", ``` This will not. ``` "icon": "C:\\Users\\Thomas\\Dropbox\\terminal\\VSWinIcon_100x.png", ``` I realise that env variables might not be supported for icons. But just wanted to throw in my crash scenario.

claunia commented

@mkitzan commented on GitHub (Dec 16, 2019):

Does this issue still repro? I can't get a crash with any of the icon paths. Let me know if you've got an icon path which can consistently crash WT, and any specific repro steps required.

@mkitzan commented on GitHub (Dec 16, 2019): Does this issue still repro? I can't get a crash with any of the icon paths. Let me know if you've got an icon path which can consistently crash WT, and any specific repro steps required.

claunia commented

@Reelix commented on GitHub (Dec 16, 2019):

I posted in #2376 which got merged into this issue. Using my original steps, I am unable to reproduce this issue in 0.7.3451.0 so it seems to have been fixed.

@Reelix commented on GitHub (Dec 16, 2019): I posted in #2376 which got merged into this issue. Using my original steps, I am unable to reproduce this issue in 0.7.3451.0 so it seems to have been fixed.

claunia commented

@mkitzan commented on GitHub (Dec 16, 2019):

Phantom fixes, nice. Would be good if others in the thread who had repros after #2376 was merged could confirm too.

@mkitzan commented on GitHub (Dec 16, 2019): Phantom fixes, nice. Would be good if others in the thread who had repros after #2376 was merged could confirm too.

claunia commented

@thomthom commented on GitHub (Dec 17, 2019):

I'm not able to reproduce this either now.

@thomthom commented on GitHub (Dec 17, 2019): I'm not able to reproduce this either now.

claunia commented

@DHowett-MSFT commented on GitHub (Dec 18, 2019):

Fascinating. I'm going to close this for now, but we'll keep our eyes peeled. Thanks for the confirmations!

@DHowett-MSFT commented on GitHub (Dec 18, 2019): Fascinating. I'm going to close this for now, but we'll keep our eyes peeled. Thanks for the confirmations!

claunia commented

@zadjii-msft commented on GitHub (Dec 19, 2019):

We're reactivating this from discussion in #4002:

Windows build number: Microsoft Windows NT 10.0.19041.0
Windows Terminal version (if applicable): 0.7.3451.0
Steps to reproduce

In profiles.json under a profile, add a background image path that references the home directory ~.
{
    ...
    "profiles": [
        {
            "name": "Windows PowerShell",
            "backgroundImage": "~/Pictures/Console/PowerShell.png",
        }
    ...
}
Expected behavior

Should not crash. Should show the expected background image.

Actual behavior

Windows Terminal crash at startup.

@zadjii-msft commented on GitHub (Dec 19, 2019): We're reactivating this from discussion in #4002: > > ``` > Windows build number: Microsoft Windows NT 10.0.19041.0 > Windows Terminal version (if applicable): 0.7.3451.0 > ``` > > # Steps to reproduce > > In `profiles.json` under a profile, add a background image path that references the home directory `~`. > > ```json > { > ... > "profiles": [ > { > "name": "Windows PowerShell", > "backgroundImage": "~/Pictures/Console/PowerShell.png", > } > ... > } > ``` > > # Expected behavior > > Should not crash. Should show the expected background image. > # Actual behavior > > Windows Terminal crash at startup.

claunia commented

@mkitzan commented on GitHub (Jan 9, 2020):

~~The particular instance of this crash from #4002 is due to SelectionBackground() in the UpdateSettings event throwing due to the file path.~~ Edit: was not able to repro this in a stack trace recently

Rather than defensively catching an exception, I wonder if it would make sense to validate the media associated with a profile somewhere during the profile creation process. Then we could intercept the issue (error page, inserting some default media...) before any event is even launched.

@mkitzan commented on GitHub (Jan 9, 2020): ~The particular instance of this crash from #4002 is due to [`SelectionBackground()`](https://github.com/microsoft/terminal/blob/master/src/cascadia/TerminalControl/TermControl.cpp#L274) in the `UpdateSettings` event throwing due to the file path.~ Edit: was not able to repro this in a stack trace recently Rather than defensively catching an exception, I wonder if it would make sense to validate the media associated with a profile somewhere during the profile creation process. Then we could intercept the issue (error page, inserting some default media...) before any event is even launched.

claunia commented

@ghost commented on GitHub (Jan 27, 2020):

:tada:This issue was addressed in #4194, which has now been successfully released as Windows Terminal Preview v0.8.10261.0.🎉

Handy links:

@ghost commented on GitHub (Jan 27, 2020): :tada:This issue was addressed in #4194, which has now been successfully released as `Windows Terminal Preview v0.8.10261.0`.:tada: Handy links: * [Release Notes](https://github.com/microsoft/terminal/releases/tag/v0.8.10261.0) * [Store Download](https://www.microsoft.com/store/apps/9n0dx20hk701?cid=storebadge&ocid=badge)

claunia commented