From d7cc11e48d395b2c7ee709aa73405e06d1d15076 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Fri, 17 Mar 2023 11:07:42 -0400 Subject: [PATCH 1/3] Use Environment.SystemDirectory from GetFolderPath(System) on Windows --- .../src/System/Environment.Win32.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Win32.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Win32.cs index 10f7815ab0af..9f699ad73633 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.Win32.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.Win32.cs @@ -220,6 +220,13 @@ private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOptio switch (folder) { + // Special-cased values to not use SHGetFolderPath when we have a more direct option available. + case SpecialFolder.System when option == SpecialFolderOption.None: + return SystemDirectory; + default: + return string.Empty; + + // Map the SpecialFolder to the appropriate Guid case SpecialFolder.ApplicationData: folderGuid = Interop.Shell32.KnownFolders.RoamingAppData; break; @@ -271,7 +278,7 @@ private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOptio case SpecialFolder.Startup: folderGuid = Interop.Shell32.KnownFolders.Startup; break; - case SpecialFolder.System: + case SpecialFolder.System when option != SpecialFolderOption.None: folderGuid = Interop.Shell32.KnownFolders.System; break; case SpecialFolder.Templates: @@ -360,15 +367,8 @@ private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOptio case SpecialFolder.Windows: folderGuid = Interop.Shell32.KnownFolders.Windows; break; - default: - return string.Empty; } - return GetKnownFolderPath(folderGuid, option); - } - - private static string GetKnownFolderPath(string folderGuid, SpecialFolderOption option) - { Guid folderId = new Guid(folderGuid); int hr = Interop.Shell32.SHGetKnownFolderPath(folderId, (uint)option, IntPtr.Zero, out string path); From 2c65dc15148f7ac64836287ede82a652591824b0 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Fri, 17 Mar 2023 12:09:09 -0400 Subject: [PATCH 2/3] Address PR feedback and fix up a few other occurrences --- .../tests/UnitTests/HostTests.cs | 5 ----- .../Composition/Hosting/AssemblyCatalogTests.cs | 2 +- .../src/System/Environment.Win32.cs | 6 ++---- .../tests/PortableExecutable/PEReaderTests.cs | 2 +- .../System.Reflection/tests/AssemblyTests.cs | 2 +- .../tests/System/EnvironmentTests.cs | 11 ----------- 6 files changed, 5 insertions(+), 23 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs index c4cb6ce45581..af826e6c4f7a 100644 --- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs +++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/HostTests.cs @@ -56,11 +56,6 @@ public void CreateDefaultBuilder_DoesNotChangeContentRootIfCurrentDirectoryIsWin using var _ = RemoteExecutor.Invoke(() => { string systemDirectory = Environment.GetFolderPath(Environment.SpecialFolder.System); - if (string.IsNullOrEmpty(systemDirectory)) - { - // Skip the environments (like Nano Server) where Environment.SpecialFolder.System returns empty - https://github.com/dotnet/runtime/issues/21430 - return; - } // Test that the path gets normalized before comparison. Use C:\WINDOWS\SYSTEM32\ instead of C:\Windows\system32. systemDirectory = systemDirectory.ToUpper() + "\\"; diff --git a/src/libraries/System.ComponentModel.Composition/tests/System/ComponentModel/Composition/Hosting/AssemblyCatalogTests.cs b/src/libraries/System.ComponentModel.Composition/tests/System/ComponentModel/Composition/Hosting/AssemblyCatalogTests.cs index 8876db46066b..da0f9cbfe84d 100644 --- a/src/libraries/System.ComponentModel.Composition/tests/System/ComponentModel/Composition/Hosting/AssemblyCatalogTests.cs +++ b/src/libraries/System.ComponentModel.Composition/tests/System/ComponentModel/Composition/Hosting/AssemblyCatalogTests.cs @@ -115,7 +115,7 @@ internal static void Constructor_InvalidFileNameAsCodeBaseArgument_ShouldThrowIO internal static void Constructor_DirectoryAsCodeBaseArgument_ShouldThrowFileLoad(Func catalogCreator) { - string directory = Environment.GetFolderPath(Environment.SpecialFolder.System); + string directory = Environment.SystemDirectory; Assert.True(Directory.Exists(directory)); Assert.Throws(() => catalogCreator(directory)); diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Win32.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Win32.cs index 9f699ad73633..01b155d718ae 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.Win32.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.Win32.cs @@ -221,7 +221,8 @@ private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOptio switch (folder) { // Special-cased values to not use SHGetFolderPath when we have a more direct option available. - case SpecialFolder.System when option == SpecialFolderOption.None: + case SpecialFolder.System: + // This assumes the system directory always exists and thus we don't need to do anything special for any SpecialFolderOption. return SystemDirectory; default: return string.Empty; @@ -278,9 +279,6 @@ private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOptio case SpecialFolder.Startup: folderGuid = Interop.Shell32.KnownFolders.Startup; break; - case SpecialFolder.System when option != SpecialFolderOption.None: - folderGuid = Interop.Shell32.KnownFolders.System; - break; case SpecialFolder.Templates: folderGuid = Interop.Shell32.KnownFolders.Templates; break; diff --git a/src/libraries/System.Reflection.Metadata/tests/PortableExecutable/PEReaderTests.cs b/src/libraries/System.Reflection.Metadata/tests/PortableExecutable/PEReaderTests.cs index f5358cab5a5f..20477b5a44b9 100644 --- a/src/libraries/System.Reflection.Metadata/tests/PortableExecutable/PEReaderTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/PortableExecutable/PEReaderTests.cs @@ -120,7 +120,7 @@ public void SubStream() [Fact] public void OpenNativeImage() { - using (var reader = new PEReader(File.OpenRead(Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.System), "kernel32.dll")))) + using (var reader = new PEReader(File.OpenRead(Path.Combine(Environment.SystemDirectory, "kernel32.dll")))) { Assert.False(reader.HasMetadata); Assert.True(reader.PEHeaders.IsDll); diff --git a/src/libraries/System.Reflection/tests/AssemblyTests.cs b/src/libraries/System.Reflection/tests/AssemblyTests.cs index 53410a35ba73..0df53d070e3f 100644 --- a/src/libraries/System.Reflection/tests/AssemblyTests.cs +++ b/src/libraries/System.Reflection/tests/AssemblyTests.cs @@ -402,7 +402,7 @@ public void LoadFile_PartiallyQualifiedPath_ThrowsArgumentException() [PlatformSpecific(TestPlatforms.Windows)] public void LoadFile_ValidPEBadIL_ThrowsBadImageFormatExceptionWithPath() { - string path = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.System), "kernelbase.dll"); + string path = Path.Combine(Environment.SystemDirectory, "kernelbase.dll"); if (!File.Exists(path)) return; diff --git a/src/libraries/System.Runtime.Extensions/tests/System/EnvironmentTests.cs b/src/libraries/System.Runtime.Extensions/tests/System/EnvironmentTests.cs index 55c666ba0b8b..f9ed70bd6770 100644 --- a/src/libraries/System.Runtime.Extensions/tests/System/EnvironmentTests.cs +++ b/src/libraries/System.Runtime.Extensions/tests/System/EnvironmentTests.cs @@ -381,17 +381,6 @@ public void GetFolderPath_Unix_SpecialFolderDoesNotExist_CreatesSuccessfully(Env [Fact] public void GetSystemDirectory() { - if (PlatformDetection.IsWindowsNanoServer) - { - // https://github.com/dotnet/runtime/issues/21430 - // On Windows Nano, ShGetKnownFolderPath currently doesn't give - // the correct result for SystemDirectory. - // Assert that it's wrong, so that if it's fixed, we don't forget to - // enable this test for Nano. - Assert.NotEqual(Environment.GetFolderPath(Environment.SpecialFolder.System), Environment.SystemDirectory); - return; - } - Assert.Equal(Environment.GetFolderPath(Environment.SpecialFolder.System), Environment.SystemDirectory); } From 67456021dc6a59a2366cd47a4704df966d8501ad Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Fri, 17 Mar 2023 17:44:58 -0400 Subject: [PATCH 3/3] Remove KnownFolders.System --- .../Interop/Windows/Shell32/Interop.SHGetKnownFolderPath.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Shell32/Interop.SHGetKnownFolderPath.cs b/src/libraries/Common/src/Interop/Windows/Shell32/Interop.SHGetKnownFolderPath.cs index dd8226a6fc9f..136d93f8e43e 100644 --- a/src/libraries/Common/src/Interop/Windows/Shell32/Interop.SHGetKnownFolderPath.cs +++ b/src/libraries/Common/src/Interop/Windows/Shell32/Interop.SHGetKnownFolderPath.cs @@ -286,12 +286,6 @@ internal static class KnownFolders /// internal const string Startup = "{B97D20BB-F46A-4C97-BA10-5E3608430854}"; - /// - /// (CSIDL_SYSTEM) System32 folder - /// "%windir%\system32" - /// - internal const string System = "{1AC14E77-02E7-4E5D-B744-2EB1AE5198B7}"; - /// /// (CSIDL_SYSTEMX86) X86 System32 folder /// "%windir%\system32" or "%windir%\syswow64"