Skip to content

Commit f28bb42

Browse files
authored
Gently rework icon and background image validation (#19044)
Right now, image validation accepts web-sourced icons (boo) and rejects images whose paths begin with `\\?\`. In addition, it will warn the user for things out of their control like images set by fragments. This pull request adds a filesystem path validator (which accepts images with fully-qualified paths and UNC paths), makes the URI validator reject any web-origin URIs (only `file` and `ms-*` are allowable), and suppresses warnings for any images that were not _directly_ set by the user. Since we want to avoid using fragment images that fail validation, we no longer `Clear` each image property but rather set it to the blank or fallback value. This does **not** actually add support for images at absolute paths beginning with `\\?\`. Such images are still rejected by `Image` and the other XAML fixtures we use for images. It's better than a warning, though. Closes #18703 Closes #14143 Refs #18710 Refs #5204 Related to #18922 (http-origin icons will be blank everywhere and not just the jump list ;))
1 parent 098da6c commit f28bb42

File tree

1 file changed

+70
-31
lines changed

1 file changed

+70
-31
lines changed

src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp

Lines changed: 70 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,39 @@ void CascadiaSettings::_validateAllSchemesExist()
488488
}
489489
}
490490

491+
static bool _validateSingleMediaResource(std::wstring_view resource)
492+
{
493+
// URI
494+
try
495+
{
496+
winrt::Windows::Foundation::Uri resourceUri{ resource };
497+
if (!resourceUri)
498+
{
499+
return false;
500+
}
501+
502+
const auto scheme{ resourceUri.SchemeName() };
503+
// Only file: URIs and ms-* URIs are permissible. http, https, ftp, gopher, etc. are not.
504+
return til::equals_insensitive_ascii(scheme, L"file") || til::starts_with_insensitive_ascii(scheme, L"ms-");
505+
}
506+
catch (...)
507+
{
508+
// fall through
509+
}
510+
511+
// Not a URI? Try a path.
512+
try
513+
{
514+
std::filesystem::path resourcePath{ resource };
515+
return std::filesystem::exists(resourcePath);
516+
}
517+
catch (...)
518+
{
519+
// fall through
520+
}
521+
return false;
522+
}
523+
491524
// Method Description:
492525
// - Ensures that all specified images resources (icons and background images) are valid URIs.
493526
// This does not verify that the icon or background image files are encoded as an image.
@@ -501,42 +534,45 @@ void CascadiaSettings::_validateAllSchemesExist()
501534
// we find any invalid icon images.
502535
void CascadiaSettings::_validateMediaResources()
503536
{
504-
auto invalidBackground{ false };
505-
auto invalidIcon{ false };
537+
auto warnInvalidBackground{ false };
538+
auto warnInvalidIcon{ false };
506539

507540
for (auto profile : _allProfiles)
508541
{
509542
if (const auto path = profile.DefaultAppearance().ExpandedBackgroundImagePath(); !path.empty())
510543
{
511-
// Attempt to convert the path to a URI, the ctor will throw if it's invalid/unparseable.
512-
// This covers file paths on the machine, app data, URLs, and other resource paths.
513-
try
544+
if (!_validateSingleMediaResource(path))
514545
{
515-
winrt::Windows::Foundation::Uri imagePath{ path };
516-
}
517-
catch (...)
518-
{
519-
// reset background image path
520-
profile.DefaultAppearance().ClearBackgroundImagePath();
521-
invalidBackground = true;
546+
if (profile.DefaultAppearance().HasBackgroundImagePath())
547+
{
548+
// Only warn and delete if the user set this at the top level (do not warn for fragments, just clear it)
549+
warnInvalidBackground = true;
550+
profile.DefaultAppearance().ClearBackgroundImagePath();
551+
}
552+
else
553+
{
554+
// reset background image path (set it to blank as an override for any fragment value)
555+
profile.DefaultAppearance().BackgroundImagePath({});
556+
}
522557
}
523558
}
524559

525560
if (profile.UnfocusedAppearance())
526561
{
527562
if (const auto path = profile.UnfocusedAppearance().ExpandedBackgroundImagePath(); !path.empty())
528563
{
529-
// Attempt to convert the path to a URI, the ctor will throw if it's invalid/unparseable.
530-
// This covers file paths on the machine, app data, URLs, and other resource paths.
531-
try
564+
if (!_validateSingleMediaResource(path))
532565
{
533-
winrt::Windows::Foundation::Uri imagePath{ path };
534-
}
535-
catch (...)
536-
{
537-
// reset background image path
538-
profile.UnfocusedAppearance().ClearBackgroundImagePath();
539-
invalidBackground = true;
566+
if (profile.UnfocusedAppearance().HasBackgroundImagePath())
567+
{
568+
warnInvalidBackground = true;
569+
profile.UnfocusedAppearance().ClearBackgroundImagePath();
570+
}
571+
else
572+
{
573+
// reset background image path (set it to blank as an override for any fragment value)
574+
profile.UnfocusedAppearance().BackgroundImagePath({});
575+
}
540576
}
541577
}
542578
}
@@ -552,24 +588,27 @@ void CascadiaSettings::_validateMediaResources()
552588
if (const auto icon = profile.Icon(); icon.size() > 2 && icon != HideIconValue)
553589
{
554590
const auto iconPath{ wil::ExpandEnvironmentStringsW<std::wstring>(icon.c_str()) };
555-
try
556-
{
557-
winrt::Windows::Foundation::Uri imagePath{ iconPath };
558-
}
559-
catch (...)
591+
if (!_validateSingleMediaResource(iconPath))
560592
{
561-
profile.ClearIcon();
562-
invalidIcon = true;
593+
if (profile.HasIcon())
594+
{
595+
warnInvalidIcon = true;
596+
profile.ClearIcon();
597+
}
598+
else
599+
{
600+
profile.Icon({});
601+
}
563602
}
564603
}
565604
}
566605

567-
if (invalidBackground)
606+
if (warnInvalidBackground)
568607
{
569608
_warnings.Append(SettingsLoadWarnings::InvalidBackgroundImage);
570609
}
571610

572-
if (invalidIcon)
611+
if (warnInvalidIcon)
573612
{
574613
_warnings.Append(SettingsLoadWarnings::InvalidIcon);
575614
}

0 commit comments

Comments
 (0)