Windows Terminal crashes when a profile's icon URL or background is unparseable/invalid #2329

AdamDanischewski · 2019-08-07T13:53:05Z

I got the latest from choco and set up an Ubuntu terminal entry. I found that if I put a garbage URL in the profiles.json icon setting entry it crashes Windows Terminal and WT won't start thereafter.

That probably shouldn't happen, though technically low priority its probably making WT look fragile.

To repeat this behavior simply remove "ms-appx:" from any icon path.

DHowett-MSFT · 2019-08-08T00:24:37Z

Yes, this is a good followup to #1468

AdamDanischewski · 2019-08-09T23:15:36Z

#622 (There should be an error dialog for incorrect settings instead of just crashing) references #886 (Cannot run deployed application (winrt::hresult_error when debugging)) and vice versa - both are closed yet both underlying issues still linger.

Sidenote: Tell MS to make Visual Studio free.

DHowett-MSFT · 2019-08-09T23:40:20Z

@AdamDanischewski

make Visual Studio free

Like Community edition?

AdamDanischewski · 2019-08-10T02:19:01Z

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.

Fixes #1333. Fixes #2329.

ghost · 2019-08-27T16:08:50Z

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

Handy links:

AdamDanischewski · 2019-09-02T18:33:41Z

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.

DHowett-MSFT · 2019-09-03T02:39:21Z

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

mdtauk · 2019-09-03T02:41:36Z

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

zadjii-msft · 2019-09-03T14:01:31Z

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?

DHowett-MSFT · 2019-09-04T01:27:36Z

"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.'.

thomthom · 2019-10-01T17:47:46Z

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.

mkitzan · 2019-12-16T20:27:27Z

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.

Reelix · 2019-12-16T21:17:20Z

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.

mkitzan · 2019-12-16T22:09:37Z

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

thomthom · 2019-12-17T09:30:02Z

I'm not able to reproduce this either now.

DHowett-MSFT · 2019-12-18T03:59:35Z

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

zadjii-msft · 2019-12-19T20:45:52Z

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.

mkitzan · 2020-01-09T21:29:26Z

~~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.

WT crashes when an unparseable/invalid `backgroundImage` or `icon` resource path is provided in `profiles.json`. This PR averts the crash by the validating and correcting resource paths as a part of the `_ValidateSettings()` function in `CascadiaSettings`. `_ValidateSettings()` is run on start up and any time `profiles.json` is changed, so a user can not change a file path and avoid the validation step. When a bad `backgroundImage` or `icon` resource path is detected, a warning screen will be presented. References #4002, which identified a consistent repro for the crash. To validate the resource, a `Windows::Foundation::Uri` object is constructed with the path. The ctor will throw if the resource path is invalid. Whether or not this validation method is robust enough is a subject worth review. The correction method for when a bad resource path is detected is to reset the `std::optional<winrt::hstring>` holding the file path. The text in the warning display was cribbed from the text used when an invalid `colorScheme` is used. Whether or not the case of a bad background image file path warrants a warning display is a subject worth review. Ensured the repro steps in #4002 did not trigger a crash. Additionally, some potential backdoor paths to a crash were tested: - Deleting the file of a validated background image file path - Changing the actual file name of a validated background image file path - Replacing the file of a validated background image file path with a non-image file (of the same name) - Using a non-image file as a background image In all the above cases WT does not crash, and instead defaults to the background color specified in the profile's `colorScheme`. This PR does not implement this recovery behavior (existing error catching code does). Closes #2329

WT crashes when an unparseable/invalid `backgroundImage` or `icon` resource path is provided in `profiles.json`. This PR averts the crash by the validating and correcting resource paths as a part of the `_ValidateSettings()` function in `CascadiaSettings`. `_ValidateSettings()` is run on start up and any time `profiles.json` is changed, so a user can not change a file path and avoid the validation step. When a bad `backgroundImage` or `icon` resource path is detected, a warning screen will be presented. References #4002, which identified a consistent repro for the crash. To validate the resource, a `Windows::Foundation::Uri` object is constructed with the path. The ctor will throw if the resource path is invalid. Whether or not this validation method is robust enough is a subject worth review. The correction method for when a bad resource path is detected is to reset the `std::optional<winrt::hstring>` holding the file path. The text in the warning display was cribbed from the text used when an invalid `colorScheme` is used. Whether or not the case of a bad background image file path warrants a warning display is a subject worth review. Ensured the repro steps in #4002 did not trigger a crash. Additionally, some potential backdoor paths to a crash were tested: - Deleting the file of a validated background image file path - Changing the actual file name of a validated background image file path - Replacing the file of a validated background image file path with a non-image file (of the same name) - Using a non-image file as a background image In all the above cases WT does not crash, and instead defaults to the background color specified in the profile's `colorScheme`. This PR does not implement this recovery behavior (existing error catching code does). Closes #2329 (cherry picked from commit 77dd51a)

ghost · 2020-01-27T18:49:16Z

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

Handy links:

ghost · 2020-02-13T18:01:41Z

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

Handy links:

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 Aug 7, 2019

carlos-zamora added this to the Terminal 1908.1 milestone Aug 7, 2019

DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Aug 8, 2019

DHowett-MSFT changed the title ~~Bad Icon Url Crashes Windows Terminal - Then WT Fails To Start~~ Windows Terminal crashes when a profile's icon URL is unparseable/invalid Aug 8, 2019

zadjii-msft mentioned this issue Aug 9, 2019

Only update the icon of a tab it the icon actually _changed_ #2376

Merged

4 tasks

ghost added the In-PR This issue has a related PR label Aug 9, 2019

DHowett-MSFT closed this as completed in #2376 Aug 14, 2019

ghost added Needs-Tag-Fix Doesn't match tag requirements 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 Aug 14, 2019

DHowett-MSFT pushed a commit that referenced this issue Aug 14, 2019

Only update the icon of a tab it the icon actually _changed_ (#2376)

8999c66

Fixes #1333. Fixes #2329.

DHowett-MSFT reopened this Sep 3, 2019

ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Sep 3, 2019

DHowett-MSFT removed the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Sep 3, 2019

zadjii-msft modified the milestones: Terminal 1908.1, Terminal-1911 Oct 3, 2019

cinnamon-msft modified the milestones: Terminal-1911, Terminal-1912 Nov 11, 2019

DHowett-MSFT closed this as completed Dec 18, 2019

ghost added the Needs-Tag-Fix Doesn't match tag requirements label Dec 18, 2019

DHowett-MSFT mentioned this issue Dec 19, 2019

Windows Terminal crash when background image path contains tilde #4002

Closed

DHowett-MSFT reopened this Dec 19, 2019

ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Dec 19, 2019

DHowett-MSFT changed the title ~~Windows Terminal crashes when a profile's icon URL is unparseable/invalid~~ Windows Terminal crashes when a profile's icon URL or background is unparseable/invalid Dec 19, 2019

mkitzan mentioned this issue Jan 13, 2020

Fix crash related to unparseable/invalid media resource paths #4194

Merged

5 tasks

ghost added the In-PR This issue has a related PR label Jan 13, 2020

cinnamon-msft modified the milestones: Terminal v0.8, Terminal v0.9 Jan 14, 2020

DHowett-MSFT closed this as completed in #4194 Jan 17, 2020

ghost added Needs-Tag-Fix Doesn't match tag requirements 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 Jan 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Windows Terminal crashes when a profile's icon URL or background is unparseable/invalid #2329

Windows Terminal crashes when a profile's icon URL or background is unparseable/invalid #2329

AdamDanischewski commented Aug 7, 2019

DHowett-MSFT commented Aug 8, 2019

AdamDanischewski commented Aug 9, 2019 •

edited

Loading

DHowett-MSFT commented Aug 9, 2019

AdamDanischewski commented Aug 10, 2019

ghost commented Aug 27, 2019

AdamDanischewski commented Sep 2, 2019

DHowett-MSFT commented Sep 3, 2019

mdtauk commented Sep 3, 2019

zadjii-msft commented Sep 3, 2019

DHowett-MSFT commented Sep 4, 2019

thomthom commented Oct 1, 2019

mkitzan commented Dec 16, 2019

Reelix commented Dec 16, 2019

mkitzan commented Dec 16, 2019

thomthom commented Dec 17, 2019

DHowett-MSFT commented Dec 18, 2019

zadjii-msft commented Dec 19, 2019

Steps to reproduce

Expected behavior

Actual behavior

mkitzan commented Jan 9, 2020 •

edited

Loading

ghost commented Jan 27, 2020

ghost commented Feb 13, 2020

Windows Terminal crashes when a profile's icon URL or background is unparseable/invalid #2329

Windows Terminal crashes when a profile's icon URL or background is unparseable/invalid #2329

Comments

AdamDanischewski commented Aug 7, 2019

DHowett-MSFT commented Aug 8, 2019

AdamDanischewski commented Aug 9, 2019 • edited Loading

DHowett-MSFT commented Aug 9, 2019

AdamDanischewski commented Aug 10, 2019

ghost commented Aug 27, 2019

AdamDanischewski commented Sep 2, 2019

DHowett-MSFT commented Sep 3, 2019

mdtauk commented Sep 3, 2019

zadjii-msft commented Sep 3, 2019

DHowett-MSFT commented Sep 4, 2019

thomthom commented Oct 1, 2019

mkitzan commented Dec 16, 2019

Reelix commented Dec 16, 2019

mkitzan commented Dec 16, 2019

thomthom commented Dec 17, 2019

DHowett-MSFT commented Dec 18, 2019

zadjii-msft commented Dec 19, 2019

Steps to reproduce

Expected behavior

Actual behavior

mkitzan commented Jan 9, 2020 • edited Loading

ghost commented Jan 27, 2020

ghost commented Feb 13, 2020

AdamDanischewski commented Aug 9, 2019 •

edited

Loading

mkitzan commented Jan 9, 2020 •

edited

Loading