From 0106d47933b7f7436a3eae002fbbe247d48ca035 Mon Sep 17 00:00:00 2001 From: "Dustin L. Howett" Date: Mon, 16 Jun 2025 15:29:59 -0500 Subject: [PATCH 1/2] Gently rework icon and background image validation 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 --- .../CascadiaSettings.cpp | 102 ++++++++++++------ 1 file changed, 71 insertions(+), 31 deletions(-) diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp b/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp index a2526951084..daa2c3027a7 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp @@ -488,6 +488,40 @@ void CascadiaSettings::_validateAllSchemesExist() } } +static bool _validateSingleMediaResource(std::wstring_view resource) +{ + // URI + try + { + winrt::Windows::Foundation::Uri resourceUri{ resource }; + if (!resourceUri) + { + return false; + } + + const auto scheme{ resourceUri.SchemeName() }; + std::wstring_view schemeSv{ scheme }; + // Only file: URIs and ms-* URIs are permissible. http, https, ftp, gopher, etc. are not. + return til::equals_insensitive_ascii(scheme, L"file") || (schemeSv.size() > 3 && til::equals_insensitive_ascii(schemeSv.substr(0, 3), L"ms-")); + } + catch (...) + { + // fall through + } + + // Not a URI? Try a path. + try + { + std::filesystem::path resourcePath{ resource }; + return std::filesystem::exists(resourcePath); + } + catch (...) + { + // fall through + } + return false; +} + // Method Description: // - Ensures that all specified images resources (icons and background images) are valid URIs. // This does not verify that the icon or background image files are encoded as an image. @@ -501,24 +535,26 @@ void CascadiaSettings::_validateAllSchemesExist() // we find any invalid icon images. void CascadiaSettings::_validateMediaResources() { - auto invalidBackground{ false }; - auto invalidIcon{ false }; + auto warnInvalidBackground{ false }; + auto warnInvalidIcon{ false }; for (auto profile : _allProfiles) { if (const auto path = profile.DefaultAppearance().ExpandedBackgroundImagePath(); !path.empty()) { - // Attempt to convert the path to a URI, the ctor will throw if it's invalid/unparseable. - // This covers file paths on the machine, app data, URLs, and other resource paths. - try + if (!_validateSingleMediaResource(path)) { - winrt::Windows::Foundation::Uri imagePath{ path }; - } - catch (...) - { - // reset background image path - profile.DefaultAppearance().ClearBackgroundImagePath(); - invalidBackground = true; + if (profile.DefaultAppearance().HasBackgroundImagePath()) + { + // Only warn and delete if the user set this at the top level (do not warn for fragments, just clear it) + warnInvalidBackground = true; + profile.DefaultAppearance().ClearBackgroundImagePath(); + } + else + { + // reset background image path (set it to blank as an override for any fragment value) + profile.DefaultAppearance().BackgroundImagePath({}); + } } } @@ -526,17 +562,18 @@ void CascadiaSettings::_validateMediaResources() { if (const auto path = profile.UnfocusedAppearance().ExpandedBackgroundImagePath(); !path.empty()) { - // Attempt to convert the path to a URI, the ctor will throw if it's invalid/unparseable. - // This covers file paths on the machine, app data, URLs, and other resource paths. - try + if (!_validateSingleMediaResource(path)) { - winrt::Windows::Foundation::Uri imagePath{ path }; - } - catch (...) - { - // reset background image path - profile.UnfocusedAppearance().ClearBackgroundImagePath(); - invalidBackground = true; + if (profile.UnfocusedAppearance().HasBackgroundImagePath()) + { + warnInvalidBackground = true; + profile.UnfocusedAppearance().ClearBackgroundImagePath(); + } + else + { + // reset background image path (set it to blank as an override for any fragment value) + profile.UnfocusedAppearance().BackgroundImagePath({}); + } } } } @@ -552,24 +589,27 @@ void CascadiaSettings::_validateMediaResources() if (const auto icon = profile.Icon(); icon.size() > 2 && icon != HideIconValue) { const auto iconPath{ wil::ExpandEnvironmentStringsW(icon.c_str()) }; - try - { - winrt::Windows::Foundation::Uri imagePath{ iconPath }; - } - catch (...) + if (!_validateSingleMediaResource(iconPath)) { - profile.ClearIcon(); - invalidIcon = true; + if (profile.HasIcon()) + { + warnInvalidIcon = true; + profile.ClearIcon(); + } + else + { + profile.Icon({}); + } } } } - if (invalidBackground) + if (warnInvalidBackground) { _warnings.Append(SettingsLoadWarnings::InvalidBackgroundImage); } - if (invalidIcon) + if (warnInvalidIcon) { _warnings.Append(SettingsLoadWarnings::InvalidIcon); } From 6c852438afdc7b1ef81f055773725ec624f762a4 Mon Sep 17 00:00:00 2001 From: "Dustin L. Howett" Date: Mon, 16 Jun 2025 17:26:09 -0500 Subject: [PATCH 2/2] click-click. that was easy. --- src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp b/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp index daa2c3027a7..9dc2c156f2e 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp @@ -500,9 +500,8 @@ static bool _validateSingleMediaResource(std::wstring_view resource) } const auto scheme{ resourceUri.SchemeName() }; - std::wstring_view schemeSv{ scheme }; // Only file: URIs and ms-* URIs are permissible. http, https, ftp, gopher, etc. are not. - return til::equals_insensitive_ascii(scheme, L"file") || (schemeSv.size() > 3 && til::equals_insensitive_ascii(schemeSv.substr(0, 3), L"ms-")); + return til::equals_insensitive_ascii(scheme, L"file") || til::starts_with_insensitive_ascii(scheme, L"ms-"); } catch (...) {