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.

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

RS1035: The symbol 'Environment' is banned for use by analyzers #4891

Open
abdes opened this issue Nov 15, 2024 · 2 comments
Open

RS1035: The symbol 'Environment' is banned for use by analyzers #4891

abdes opened this issue Nov 15, 2024 · 2 comments
Assignees

Comments

@abdes
Copy link

abdes commented Nov 15, 2024

Describe the bug

Using .NET 9, latest Windows App SDK:

nuget\packages\microsoft.windowsappsdk\1.6.241106002\include\UndockedRegFreeWinRT-AutoInitializer.cs(27,13,27,119): error RS1035: The symbol 'Environment' is banned for use by analyzers: Analyzers should not read their settings direct

Steps to reproduce the bug

Using .NET 9

<EnforceExtendedAnalyzerRules>true</EnforceExtendedAnalyzerRules>

Expected behavior

No response

Screenshots

No response

NuGet package version

Windows App SDK 1.6.2: 1.6.241106002

Packaging type

Packaged (MSIX)

Windows version

Windows 11 version 22H2 (22621, 2022 Update)

IDE

Visual Studio 2022

Additional context

No response

@abdes abdes changed the title Bug title RS1035: The symbol 'Environment' is banned for use by analyzers Nov 16, 2024
@DrusTheAxe DrusTheAxe self-assigned this Nov 21, 2024
@DrusTheAxe
Copy link
Member

@Scottj1s I'm not seeing a good way for dev\UndockedRegFreeWinRT\UndockedRegFreeWinRT-AutoInitializer.cs to declare something (an attribute?) to suppress the warning because yes, this is OK.

The problematic code passes a string from the caller's assembly to be later used by UndockedRegFreeWinRT (URFW) and MRT. Environment variables are used as a form of 'global variable' within WinAppSDK

            // Set base directory env var for PublishSingleFile support (referenced by SxS redirection)
            Environment.SetEnvironmentVariable("MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY", AppContext.BaseDirectory);

            // No error handling needed as the target function does nothing (just {return S_OK}).
            // It's the act of calling the function causing the DllImport to load the DLL that
            // matters. This provides the moral equivalent of a native DLL's Import Address
            // Table (IAT) have an entry that's resolved when this module is loaded.
            NativeMethods.WindowsAppRuntime_EnsureIsLoaded();

I'm inclined to change this to explicitly handle this e.g. change dev\UndockedRegFreeWinRT\UndockedRegFreeWinRT-AutoInitializer.cs

...
        [DllImport("Microsoft.WindowsAppRuntime.dll", CharSet = CharSet.Unicode, ExactSpelling = true)]
        internal static extern int WindowsAppRuntime_SetBaseDirectory(string baseDirectory);
...
    class AutoInitialize
    {
        [global::System.Runtime.CompilerServices.ModuleInitializer]
        internal static void AccessWindowsAppSDK()
        {
            NativeMethods.WindowsAppRuntime_EnsureIsLoaded();

            int hr = NativeMethods.WindowsAppRuntime_SetBaseDirectory(AppContext.BaseDirectory);
            if (hr < 0)
            {
                Marshal.ThrowExceptionForHR(hr);
            }
        }

and in dev\WindowsAppRuntime_DLL\WindowsAppRuntime_EnsureIsLoaded.cpp

wil::unique_cotaskmem_string g_windowsAppRuntime_baseDirectory;

STDAPI WindowsAppRuntime_EnsureIsLoaded()
{
    return S_OK;
}

STDAPI WindowsAppRuntime_EnsureIsLoaded2(PCWSTR baseDirectory)
{
    if (!baseDirectory || (baseDirectory[0] == L'\0'))
    {
        g_windowsAppRuntime_baseDirectory.reset();
    }
    else
    {
        g_windowsAppRuntime_baseDirectory{ wil::make_cotaskmem_string_nothrow(baseDirectory) };
        RETURN_IF_NULL_ALLOC_MSG(g_windowsAppRuntime_baseDirectory, "BaseDirectory: %ls", baseDirectory);
    }
    return S_OK;
}

PCWSTR WindowsAppRuntime_GetBaseDirectory()
{
    return g_windowsAppRuntime_baseDirectory.get();
}

and change URFW+MRT from GetEnvironmentVariable() to WindowsAppRuntime_GetBaseDirectory()

This avoids using envvars as globals e.g. if an app doesn't use the C# auto-initializer this avoids malicious actors from setting the envvar to affect behavior.

You have any qualms with this change?

P.S. Interestingly this EnvVar game only occurs in the auto-initializer for C# but not for C++. This is intentional?

@DrusTheAxe
Copy link
Member

I'm not seeing a good way for dev\UndockedRegFreeWinRT\UndockedRegFreeWinRT-AutoInitializer.cs to declare something (an attribute?) to suppress the warning because yes, this is OK.

@Scottj1s Another option is Application.Properties() instead of environment variables

CoreApplication.Properties Property (Windows.ApplicationModel.Core) - Windows apps | Microsoft Learn
CoreApplication Class (Windows.ApplicationModel.Core) - Windows apps | Microsoft Learn

For example to set the option apps would

Windows.ApplicationModel.Core.Application.Properties()["Microsoft.WindowsAppRuntime.BaseDirectory"] = AppContext.BaseDirectory;

and in WinAppSDK check/retrieve via the C++ equivalent of

Windows.Foundation.PropertyValue value;
HRESULT hr = Windows.ApplicationModel.Core.Application.Properties().Lookup("Microsoft.WindowsAppRuntime.BaseDirectory", &value)
string baseDirectory = (SUCCEEDED(hr) ? value.GetString() : nullptr);

In C++ that looks something like

auto properties{ winrt::Windows::ApplicationModel::Core::Application::Properties() };
winrt::hstring baseDirectory;
wil::com_ptr<abi::Windows::ApplicationModel::Core::Application::IPropertySet> propertiesAsABI{ properties.as<abi::Windows::ApplicationModel::Core::Application::IPropertySet>() };
winrt::hstring key{ "Microsoft.WindowsAppRuntime.BaseDirectory" };
winrt::Windows::Foundation::PropertyValue value;
const HRESULT hr{ propertiesAsABI->Lookup(key, wil::out_param(value)) };
THROW_HR_IF(hr, hr != E_BOUNDS);
if (SUCCEEDED(hr))
{
    baseDirectory = std::move(value);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants