Cleanup registry usage within CoreLib (#20050)
authorJan Kotas <jkotas@microsoft.com>
Wed, 19 Sep 2018 21:12:40 +0000 (14:12 -0700)
committerGitHub <noreply@github.com>
Wed, 19 Sep 2018 21:12:40 +0000 (14:12 -0700)
- Ensure that the registry keys are always disposed
- Use smaller subset of registry APIs
- Reduce diffs with CoreCLR/CoreFX
- Contributes to #11009 and #17899

src/System.Private.CoreLib/shared/Interop/Windows/Interop.Errors.cs
src/System.Private.CoreLib/shared/Microsoft/Win32/Registry.cs
src/System.Private.CoreLib/shared/System/Diagnostics/Tracing/EventProvider.cs
src/System.Private.CoreLib/shared/System/Globalization/HijriCalendar.Win32.cs
src/System.Private.CoreLib/shared/System/Globalization/JapaneseCalendar.Win32.cs
src/System.Private.CoreLib/shared/System/Globalization/JapaneseCalendar.WinRT.cs
src/System.Private.CoreLib/shared/System/TimeZoneInfo.Win32.cs
src/System.Private.CoreLib/src/Microsoft/Win32/RegistryKey.cs
src/System.Private.CoreLib/src/System/ThrowHelper.cs

index 7f907fb..1a95db3 100644 (file)
@@ -39,8 +39,7 @@ internal partial class Interop
         internal const int ERROR_NO_UNICODE_TRANSLATION = 0x459;
         internal const int ERROR_NOT_FOUND = 0x490;
         internal const int ERROR_BAD_IMPERSONATION_LEVEL = 0x542;
-        internal const int ERROR_NO_SYSTEM_RESOURCES = 0x5AA;        
-        internal const int E_FILENOTFOUND = unchecked((int)0x80070002);
+        internal const int ERROR_NO_SYSTEM_RESOURCES = 0x5AA;
         internal const int ERROR_TIMEOUT = 0x000005B4;
     }
 }
index bc4ee08..d9b1d9d 100644 (file)
@@ -20,7 +20,8 @@ namespace Microsoft.Win32
 
         /// <summary>Local Machine key. This key should be used as the root for all machine specific settings.</summary>
         public static readonly RegistryKey LocalMachine = RegistryKey.OpenBaseKey(RegistryHive.LocalMachine, RegistryView.Default);
-        
+
+#if REGISTRY_ASSEMBLY
         /// <summary>Classes Root Key. This is the root key of class information.</summary>
         public static readonly RegistryKey ClassesRoot = RegistryKey.OpenBaseKey(RegistryHive.ClassesRoot, RegistryView.Default);
 
@@ -100,5 +101,6 @@ namespace Microsoft.Win32
                 key.SetValue(valueName, value, valueKind);
             }
         }
+#endif
     }
 }
index 36b363f..9e52ba2 100644 (file)
@@ -507,37 +507,39 @@ namespace System.Diagnostics.Tracing
 
             // Determine our session from what is in the registry.  
             string regKey = @"\Microsoft\Windows\CurrentVersion\Winevt\Publishers\{" + m_providerName + "}";
-            if (System.Runtime.InteropServices.Marshal.SizeOf(typeof(IntPtr)) == 8)
+            if (IntPtr.Size == 8)
                 regKey = @"Software" + @"\Wow6432Node" + regKey;
             else
                 regKey = @"Software" + regKey;
 
-            var key = Microsoft.Win32.Registry.LocalMachine.OpenSubKey(regKey);
-            if (key != null)
+            using (var key = Microsoft.Win32.Registry.LocalMachine.OpenSubKey(regKey))
             {
-                foreach (string valueName in key.GetValueNames())
+                if (key != null)
                 {
-                    if (valueName.StartsWith("ControllerData_Session_", StringComparison.Ordinal))
+                    foreach (string valueName in key.GetValueNames())
                     {
-                        string strId = valueName.Substring(23);      // strip of the ControllerData_Session_
-                        int etwSessionId;
-                        if (int.TryParse(strId, out etwSessionId))
+                        if (valueName.StartsWith("ControllerData_Session_", StringComparison.Ordinal))
                         {
-                            // we need to assert this permission for partial trust scenarios
-                            (new RegistryPermission(RegistryPermissionAccess.Read, regKey)).Assert();
-                            var data = key.GetValue(valueName) as byte[];
-                            if (data != null)
+                            string strId = valueName.Substring(23);      // strip of the ControllerData_Session_
+                            int etwSessionId;
+                            if (int.TryParse(strId, out etwSessionId))
                             {
-                                var dataAsString = System.Text.Encoding.UTF8.GetString(data);
-                                int keywordIdx = dataAsString.IndexOf("EtwSessionKeyword", StringComparison.Ordinal);
-                                if (0 <= keywordIdx)
+                                // we need to assert this permission for partial trust scenarios
+                                (new RegistryPermission(RegistryPermissionAccess.Read, regKey)).Assert();
+                                var data = key.GetValue(valueName) as byte[];
+                                if (data != null)
                                 {
-                                    int startIdx = keywordIdx + 18;
-                                    int endIdx = dataAsString.IndexOf('\0', startIdx);
-                                    string keywordBitString = dataAsString.Substring(startIdx, endIdx-startIdx);
-                                    int keywordBit;
-                                    if (0 < endIdx && int.TryParse(keywordBitString, out keywordBit))
-                                        action(etwSessionId, 1L << keywordBit, ref sessionList);
+                                    var dataAsString = System.Text.Encoding.UTF8.GetString(data);
+                                    int keywordIdx = dataAsString.IndexOf("EtwSessionKeyword", StringComparison.Ordinal);
+                                    if (0 <= keywordIdx)
+                                    {
+                                        int startIdx = keywordIdx + 18;
+                                        int endIdx = dataAsString.IndexOf('\0', startIdx);
+                                        string keywordBitString = dataAsString.Substring(startIdx, endIdx-startIdx);
+                                        int keywordBit;
+                                        if (0 < endIdx && int.TryParse(keywordBitString, out keywordBit))
+                                            action(etwSessionId, 1L << keywordBit, ref sessionList);
+                                    }
                                 }
                             }
                         }
@@ -582,10 +584,10 @@ namespace System.Diagnostics.Tracing
             {
 #if (!ES_BUILD_PCL && !ES_BUILD_PN && PLATFORM_WINDOWS)
                 string regKey = @"\Microsoft\Windows\CurrentVersion\Winevt\Publishers\{" + m_providerId + "}";
-                if (System.Runtime.InteropServices.Marshal.SizeOf(typeof(IntPtr)) == 8)
-                    regKey = @"HKEY_LOCAL_MACHINE\Software" + @"\Wow6432Node" + regKey;
+                if (IntPtr.Size == 8)
+                    regKey = @"Software" + @"\Wow6432Node" + regKey;
                 else
-                    regKey = @"HKEY_LOCAL_MACHINE\Software" + regKey;
+                    regKey = @"Software" + regKey;
 
                 string valueName = "ControllerData_Session_" + etwSessionId.ToString(CultureInfo.InvariantCulture);
 
@@ -593,12 +595,15 @@ namespace System.Diagnostics.Tracing
 #if !CORECLR
                 (new RegistryPermission(RegistryPermissionAccess.Read, regKey)).Assert();
 #endif
-                data = Microsoft.Win32.Registry.GetValue(regKey, valueName, null) as byte[];
-                if (data != null)
+                using (var key = Microsoft.Win32.Registry.LocalMachine.OpenSubKey(regKey))
                 {
-                    // We only used the persisted data from the registry for updates.   
-                    command = ControllerCommand.Update;
-                    return true;
+                    data =  key?.GetValue(valueName, null) as byte[];
+                    if (data != null)
+                    {
+                        // We only used the persisted data from the registry for updates.   
+                        command = ControllerCommand.Update;
+                        return true;
+                    }
                 }
 #endif
             }
index 7333f87..8c23e46 100644 (file)
@@ -39,56 +39,45 @@ namespace System.Globalization
         ============================================================================*/
         private static int GetAdvanceHijriDate()
         {
-            int hijriAdvance = 0;
-            Microsoft.Win32.RegistryKey key = null;
-
-            try
+            using (RegistryKey key = Registry.CurrentUser.OpenSubKey(InternationalRegKey))
             {
-                // Open in read-only mode.
-                key = RegistryKey.OpenBaseKey(RegistryHive.CurrentUser).OpenSubKey(InternationalRegKey, false);
-            }
-            //If this fails for any reason, we'll just return 0.
-            catch (ObjectDisposedException) { return 0; }
-            catch (ArgumentException) { return 0; }
+                // Abort if we didn't find anything
+                if (key == null)
+                {
+                    return 0;
+                }
 
-            if (key != null)
-            {
-                try
+                object value = key.GetValue(HijriAdvanceRegKeyEntry, null, RegistryValueOptions.DoNotExpandEnvironmentNames);
+                if (value == null)
                 {
-                    object value = key.InternalGetValue(HijriAdvanceRegKeyEntry, null, false, false);
-                    if (value == null)
-                    {
-                        return (0);
-                    }
-                    string str = value.ToString();
-                    if (string.Compare(str, 0, HijriAdvanceRegKeyEntry, 0, HijriAdvanceRegKeyEntry.Length, StringComparison.OrdinalIgnoreCase) == 0)
+                    return 0;
+                }
+
+                int hijriAdvance = 0;
+                string str = value.ToString();
+                if (string.Compare(str, 0, HijriAdvanceRegKeyEntry, 0, HijriAdvanceRegKeyEntry.Length, StringComparison.OrdinalIgnoreCase) == 0)
+                {
+                    if (str.Length == HijriAdvanceRegKeyEntry.Length)
+                        hijriAdvance = -1;
+                    else
                     {
-                        if (str.Length == HijriAdvanceRegKeyEntry.Length)
-                            hijriAdvance = -1;
-                        else
+                        try
                         {
-                            try
+                            int advance = int.Parse(str.AsSpan(HijriAdvanceRegKeyEntry.Length), provider:CultureInfo.InvariantCulture);
+                            if ((advance >= MinAdvancedHijri) && (advance <= MaxAdvancedHijri))
                             {
-                                int advance = int.Parse(str.AsSpan(HijriAdvanceRegKeyEntry.Length), provider:CultureInfo.InvariantCulture);
-                                if ((advance >= MinAdvancedHijri) && (advance <= MaxAdvancedHijri))
-                                {
-                                    hijriAdvance = advance;
-                                }
+                                hijriAdvance = advance;
                             }
-                            // If we got garbage from registry just ignore it.
-                            // hijriAdvance = 0 because of declaraction assignment up above.
-                            catch (ArgumentException) { }
-                            catch (FormatException) { }
-                            catch (OverflowException) { }
                         }
+                        // If we got garbage from registry just ignore it.
+                        // hijriAdvance = 0 because of declaraction assignment up above.
+                        catch (ArgumentException) { }
+                        catch (FormatException) { }
+                        catch (OverflowException) { }
                     }
                 }
-                finally
-                {
-                    key.Close();
-                }
+                return hijriAdvance;
             }
-            return (hijriAdvance);
         }
     }
 }
index 2356afe..8657d31 100644 (file)
@@ -10,8 +10,7 @@ namespace System.Globalization
 {
     public partial class JapaneseCalendar : Calendar
     {
-        private const string c_japaneseErasHive = @"System\CurrentControlSet\Control\Nls\Calendars\Japanese\Eras";
-        private const string c_japaneseErasHivePermissionList = @"HKEY_LOCAL_MACHINE\" + c_japaneseErasHive;
+        private const string JapaneseErasHive = @"System\CurrentControlSet\Control\Nls\Calendars\Japanese\Eras";
 
         // We know about 4 built-in eras, however users may add additional era(s) from the
         // registry, by adding values to HKLM\SYSTEM\CurrentControlSet\Control\Nls\Calendars\Japanese\Eras
@@ -37,29 +36,30 @@ namespace System.Globalization
             try
             {
                 // Need to access registry
-                RegistryKey key = RegistryKey.OpenBaseKey(RegistryHive.LocalMachine).OpenSubKey(c_japaneseErasHive, false);
-
-                // Abort if we didn't find anything
-                if (key == null) return null;
-
-                // Look up the values in our reg key
-                string[] valueNames = key.GetValueNames();
-                if (valueNames != null && valueNames.Length > 0)
+                using (RegistryKey key = Registry.LocalMachine.OpenSubKey(JapaneseErasHive))
                 {
-                    registryEraRanges = new EraInfo[valueNames.Length];
+                    // Abort if we didn't find anything
+                    if (key == null) return null;
 
-                    // Loop through the registry and read in all the values
-                    for (int i = 0; i < valueNames.Length; i++)
+                    // Look up the values in our reg key
+                    string[] valueNames = key.GetValueNames();
+                    if (valueNames != null && valueNames.Length > 0)
                     {
-                        // See if the era is a valid date
-                        EraInfo era = GetEraFromValue(valueNames[i], key.GetValue(valueNames[i]).ToString());
+                        registryEraRanges = new EraInfo[valueNames.Length];
+
+                        // Loop through the registry and read in all the values
+                        for (int i = 0; i < valueNames.Length; i++)
+                        {
+                            // See if the era is a valid date
+                            EraInfo era = GetEraFromValue(valueNames[i], key.GetValue(valueNames[i]).ToString());
 
-                        // continue if not valid
-                        if (era == null) continue;
+                            // continue if not valid
+                            if (era == null) continue;
 
-                        // Remember we found one.
-                        registryEraRanges[iFoundEras] = era;
-                        iFoundEras++;
+                            // Remember we found one.
+                            registryEraRanges[iFoundEras] = era;
+                            iFoundEras++;
+                        }
                     }
                 }
             }
@@ -199,7 +199,7 @@ namespace System.Globalization
 
         // PAL Layer ends here
 
-        private static string[] s_japaneseErasEnglishNames = new string[] { "M", "T", "S", "H" };
+        private static readonly string[] s_japaneseErasEnglishNames = new string[] { "M", "T", "S", "H" };
 
         private static string GetJapaneseEnglishEraName(int era)
         {
index 818cb5b..633baa4 100644 (file)
@@ -46,12 +46,12 @@ namespace System.Globalization
 
         // PAL Layer ends here
 
-        private static string[] JapaneseErasEnglishNames = new string[] { "M", "T", "S", "H" };
+        private static readonly string[] s_JapaneseErasEnglishNames = new string[] { "M", "T", "S", "H" };
 
         private static string GetJapaneseEnglishEraName(int era)
         {
             Debug.Assert(era > 0);
-            return era <= JapaneseErasEnglishNames.Length ? JapaneseErasEnglishNames[era - 1] : " ";
+            return era <= s_JapaneseErasEnglishNames.Length ? s_JapaneseErasEnglishNames[era - 1] : " ";
         }
 
         private static bool GetJapaneseEraInfo(int era, out DateTimeOffset dateOffset, out string eraName, out string abbreviatedEraName)
index 443b8d4..03e7926 100644 (file)
@@ -574,8 +574,8 @@ namespace System
                     // read LastEntry   {(yearN, 1, 1) - MaxValue       }
 
                     // read the FirstEntry and LastEntry key values (ex: "1980", "2038")
-                    int first = (int)dynamicKey.GetValue(FirstEntryValue, -1, RegistryValueOptions.None);
-                    int last = (int)dynamicKey.GetValue(LastEntryValue, -1, RegistryValueOptions.None);
+                    int first = (int)dynamicKey.GetValue(FirstEntryValue, -1);
+                    int last = (int)dynamicKey.GetValue(LastEntryValue, -1);
 
                     if (first == -1 || last == -1 || first > last)
                     {
@@ -678,7 +678,7 @@ namespace System
 
         private static unsafe bool TryGetTimeZoneEntryFromRegistry(RegistryKey key, string name, out REG_TZI_FORMAT dtzi)
         {
-            byte[] regValue = key.GetValue(name, null, RegistryValueOptions.None) as byte[];
+            byte[] regValue = key.GetValue(name, null) as byte[];
             if (regValue == null || regValue.Length != sizeof(REG_TZI_FORMAT))
             {
                 dtzi = default;
@@ -744,7 +744,7 @@ namespace System
                 //
                 if (result)
                 {
-                    string registryStandardName = key.GetValue(StandardValue, string.Empty, RegistryValueOptions.None) as string;
+                    string registryStandardName = key.GetValue(StandardValue, string.Empty) as string;
                     result = string.Equals(registryStandardName, timeZone.GetStandardName(), StringComparison.Ordinal);
                 }
                 return result;
@@ -873,9 +873,9 @@ namespace System
             daylightName = string.Empty;
 
             // read the MUI_ registry keys
-            string displayNameMuiResource = key.GetValue(MuiDisplayValue, string.Empty, RegistryValueOptions.None) as string;
-            string standardNameMuiResource = key.GetValue(MuiStandardValue, string.Empty, RegistryValueOptions.None) as string;
-            string daylightNameMuiResource = key.GetValue(MuiDaylightValue, string.Empty, RegistryValueOptions.None) as string;
+            string displayNameMuiResource = key.GetValue(MuiDisplayValue, string.Empty) as string;
+            string standardNameMuiResource = key.GetValue(MuiStandardValue, string.Empty) as string;
+            string daylightNameMuiResource = key.GetValue(MuiDaylightValue, string.Empty) as string;
 
             // try to load the strings from the native resource DLL(s)
             if (!string.IsNullOrEmpty(displayNameMuiResource))
@@ -896,15 +896,15 @@ namespace System
             // fallback to using the standard registry keys
             if (string.IsNullOrEmpty(displayName))
             {
-                displayName = key.GetValue(DisplayValue, string.Empty, RegistryValueOptions.None) as string;
+                displayName = key.GetValue(DisplayValue, string.Empty) as string;
             }
             if (string.IsNullOrEmpty(standardName))
             {
-                standardName = key.GetValue(StandardValue, string.Empty, RegistryValueOptions.None) as string;
+                standardName = key.GetValue(StandardValue, string.Empty) as string;
             }
             if (string.IsNullOrEmpty(daylightName))
             {
-                daylightName = key.GetValue(DaylightValue, string.Empty, RegistryValueOptions.None) as string;
+                daylightName = key.GetValue(DaylightValue, string.Empty) as string;
             }
         }
 
index 48f3fb4..3176ada 100644 (file)
@@ -52,6 +52,7 @@ using System.Buffers;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
+using System.Security;
 using System.Text;
 
 namespace Microsoft.Win32
@@ -144,13 +145,6 @@ namespace Microsoft.Win32
             }
         }
 
-        // This dummy method is added to have the same implemenatation of Registry class.
-        // Its not being used anywhere.
-        public RegistryKey CreateSubKey(string subkey)
-        {
-            return null;
-        }
-
         private static void FixupPath(StringBuilder path)
         {
             Debug.Assert(path != null);
@@ -287,7 +281,7 @@ namespace Microsoft.Win32
             {
                 if (throwOnMissingValue)
                 {
-                    ThrowHelper.ThrowArgumentException(ExceptionResource.Arg_RegSubKeyValueAbsent);
+                    throw new ArgumentException(SR.Arg_RegSubKeyValueAbsent);
                 }
                 // Otherwise, just return giving no indication to the user.
                 // (For compatibility)
@@ -370,7 +364,7 @@ namespace Microsoft.Win32
             {
                 // We need to throw SecurityException here for compatibility reasons,
                 // although UnauthorizedAccessException will make more sense.
-                ThrowHelper.ThrowSecurityException(ExceptionResource.Security_RegistryPermission);
+                throw new SecurityException(SR.Security_RegistryPermission);
             }
 
             return null;
@@ -873,7 +867,7 @@ namespace Microsoft.Win32
                             {
                                 if (dataStrings[i] == null)
                                 {
-                                    ThrowHelper.ThrowArgumentException(ExceptionResource.Arg_RegSetStrArrNull);
+                                    throw new ArgumentException(SR.Arg_RegSetStrArrNull);
                                 }
                                 sizeInBytes = checked(sizeInBytes + (dataStrings[i].Length + 1) * 2);
                             }
@@ -950,19 +944,19 @@ namespace Microsoft.Win32
             }
             catch (OverflowException)
             {
-                ThrowHelper.ThrowArgumentException(ExceptionResource.Arg_RegSetMismatchedKind);
+                throw new ArgumentException(SR.Arg_RegSetMismatchedKind);
             }
             catch (InvalidOperationException)
             {
-                ThrowHelper.ThrowArgumentException(ExceptionResource.Arg_RegSetMismatchedKind);
+                throw new ArgumentException(SR.Arg_RegSetMismatchedKind);
             }
             catch (FormatException)
             {
-                ThrowHelper.ThrowArgumentException(ExceptionResource.Arg_RegSetMismatchedKind);
+                throw new ArgumentException(SR.Arg_RegSetMismatchedKind);
             }
             catch (InvalidCastException)
             {
-                ThrowHelper.ThrowArgumentException(ExceptionResource.Arg_RegSetMismatchedKind);
+                throw new ArgumentException(SR.Arg_RegSetMismatchedKind);
             }
 
             if (ret == 0)
@@ -1056,7 +1050,7 @@ namespace Microsoft.Win32
         {
             if (_hkey == null)
             {
-                ThrowHelper.ThrowObjectDisposedException(_keyName, ExceptionResource.ObjectDisposed_RegKeyClosed);
+                throw new ObjectDisposedException(_keyName, SR.ObjectDisposed_RegKeyClosed);
             }
         }
 
@@ -1065,7 +1059,7 @@ namespace Microsoft.Win32
             EnsureNotDisposed();
             if (!IsWritable())
             {
-                ThrowHelper.ThrowUnauthorizedAccessException(ExceptionResource.UnauthorizedAccess_RegistryNoWrite);
+                throw new UnauthorizedAccessException(SR.UnauthorizedAccess_RegistryNoWrite);
             }
         }
 
@@ -1113,28 +1107,23 @@ namespace Microsoft.Win32
             while (nextSlash != -1)
             {
                 if ((nextSlash - current) > MaxKeyLength)
-                    ThrowHelper.ThrowArgumentException(ExceptionResource.Arg_RegKeyStrLenBug);
+                    throw new ArgumentException(SR.Arg_RegKeyStrLenBug);
 
                 current = nextSlash + 1;
                 nextSlash = name.IndexOf("\\", current, StringComparison.OrdinalIgnoreCase);
             }
 
             if ((name.Length - current) > MaxKeyLength)
-                ThrowHelper.ThrowArgumentException(ExceptionResource.Arg_RegKeyStrLenBug);
+                throw new ArgumentException(SR.Arg_RegKeyStrLenBug);
         }
 
         private static void ValidateKeyView(RegistryView view)
         {
             if (view != RegistryView.Default && view != RegistryView.Registry32 && view != RegistryView.Registry64)
             {
-                ThrowHelper.ThrowArgumentException(ExceptionResource.Argument_InvalidRegistryViewCheck, ExceptionArgument.view);
+                throw new ArgumentException(SR.Argument_InvalidRegistryViewCheck, nameof(view));
             }
         }
-
-        // Win32 constants for error handling
-        private const int FORMAT_MESSAGE_IGNORE_INSERTS = 0x00000200;
-        private const int FORMAT_MESSAGE_FROM_SYSTEM = 0x00001000;
-        private const int FORMAT_MESSAGE_ARGUMENT_ARRAY = 0x00002000;
     }
 
     // the name for this API is meant to mimic FileMode, which has similar values
index dd9ee50..e18520d 100644 (file)
@@ -489,18 +489,11 @@ namespace System
         Argument_ImplementIComparable,
         Argument_InvalidType,
         Argument_InvalidArgumentForComparison,
-        Argument_InvalidRegistryKeyPermissionCheck,
         ArgumentOutOfRange_NeedNonNegNum,
 
         Arg_ArrayPlusOffTooSmall,
         Arg_NonZeroLowerBound,
         Arg_RankMultiDimNotSupported,
-        Arg_RegKeyDelHive,
-        Arg_RegKeyStrLenBug,
-        Arg_RegSetStrArrNull,
-        Arg_RegSetMismatchedKind,
-        Arg_RegSubKeyAbsent,
-        Arg_RegSubKeyValueAbsent,
 
         Argument_AddingDuplicate,
         Serialization_InvalidOnDeser,
@@ -527,13 +520,7 @@ namespace System
         InvalidOperation_EnumEnded,
         NotSupported_SortedListNestedWrite,
         InvalidOperation_NoValue,
-        InvalidOperation_RegRemoveSubKey,
-        Security_RegistryPermission,
-        UnauthorizedAccess_RegistryNoWrite,
-        ObjectDisposed_RegKeyClosed,
         NotSupported_InComparableType,
-        Argument_InvalidRegistryOptionsCheck,
-        Argument_InvalidRegistryViewCheck,
         InvalidOperation_NullArray,
         Arg_MustBeType,
         Arg_NeedAtLeast1Rank,