Runtime: Missing methods in Microsoft.Win32.Registry

Created on 19 Jan 2017  路  23Comments  路  Source: dotnet/runtime

In a porting project I'm working on I noticed RegistryKey.Close is not in the surface area right now. It should come back. It's also useful to generally diff the Microsoft.Win32.Registry types with desktop.

area-Microsoft.Win32

All 23 comments

Would simply adding the following method to the Microsoft.Win32.RegistryKey class resolve this issue?

public void Close()
{
    Dispose();
}

It's a good start! I don't know offhand if there are more; @weshaggard has a differ handy I don't, so I didn't dig in further.

It would require adding that method to both the ref and the src. However we should also diff the full API surface and try and fill in any other gaps.

I could take a look at the API differences for the Registry types:

  • Registry
  • RegistryKey

So, based on the https://raw.githubusercontent.com/dotnet/corefx/master/src/shims/ApiCompatBaseline.netfx461.txt file, these are the differences:

Missing types

Microsoft.Win32.RegistryKeyPermissionCheck

Missing members

Microsoft.Win32.RegistryKey Microsoft.Win32.Registry.DynData
Microsoft.Win32.RegistryHive Microsoft.Win32.RegistryHive.DynData
Microsoft.Win32.RegistryKey.Close()
Microsoft.Win32.RegistryKey.CreateSubKey(System.String, Microsoft.Win32.RegistryKeyPermissionCheck)
Microsoft.Win32.RegistryKey.CreateSubKey(System.String, Microsoft.Win32.RegistryKeyPermissionCheck, Microsoft.Win32.RegistryOptions)
Microsoft.Win32.RegistryKey.CreateSubKey(System.String, Microsoft.Win32.RegistryKeyPermissionCheck, Microsoft.Win32.RegistryOptions, System.Security.AccessControl.RegistrySecurity)
Microsoft.Win32.RegistryKey.CreateSubKey(System.String, Microsoft.Win32.RegistryKeyPermissionCheck, System.Security.AccessControl.RegistrySecurity)
Microsoft.Win32.RegistryKey.GetAccessControl()
Microsoft.Win32.RegistryKey.GetAccessControl(System.Security.AccessControl.AccessControlSections)
Microsoft.Win32.RegistryKey.OpenRemoteBaseKey(Microsoft.Win32.RegistryHive, System.String)
Microsoft.Win32.RegistryKey.OpenRemoteBaseKey(Microsoft.Win32.RegistryHive, System.String, Microsoft.Win32.RegistryView)
Microsoft.Win32.RegistryKey.OpenSubKey(System.String, Microsoft.Win32.RegistryKeyPermissionCheck)
Microsoft.Win32.RegistryKey.OpenSubKey(System.String, Microsoft.Win32.RegistryKeyPermissionCheck, System.Security.AccessControl.RegistryRights)
Microsoft.Win32.RegistryKey.SetAccessControl(System.Security.AccessControl.RegistrySecurity)

Different inheritance

'Microsoft.Win32.RegistryKey' does not inherit from base type 'System.MarshalByRefObject'

Half of the missing methods require the RegistryKeyPermissionCheck parameter. We should firstly decide, whether we exclude this type, or not.

Note this is not relevant to .NET Standard, which excludes Registry.

@weshaggard I guess we should add all these? Per the rule that if we share a type with Desktop, we implement all the members (or at least they're present)

Yes we should try and add all these. The ACL APIs are the only ones that are kind of interesting because we currently have https://github.com/dotnet/corefx/tree/master/src/Microsoft.Win32.Registry.AccessControl which we should reconcile with but otherwise I think we should add all the others.

@sepidehMS something to do when you don't have a higher priority task.

As RegistryKeyPermissionCheck type does not exist in Core, based on what explained in https://github.com/dotnet/corefx/issues/15255 I believe the APIs requiring it shouldn't be added. Is that right?

Also, the following is Obsolete on Desktop. Do we add it?
Microsoft.Win32.RegistryKey Microsoft.Win32.Registry.DynData

We should should add RegistryKeyPermissionCheck directly in the registry library unless there is some reason we cannot.

I'm good with excluding DynData.

@sepidehMS could you work with @darxis with implementing and picking the correct set of APIs? @darxis assuming you are still interesting in contributing to this issue.

Agree with @weshaggard. ACL could be problematic, but given that registry is inherently Windows-only I don't see why not. Keeping RegistryKeyPermissionCheck to the registry would make the most sense.

Don't see a reason to include DynData. It's Win9x only which is wayyyyy out of support and not even supported by .NET Core.

I believe we have a stub implementation for non-Windows platforms. PNSE the ACL apis there?


From: Immo Landwerth notifications@github.com
Sent: Monday, January 23, 2017 11:14:16 PM
To: dotnet/corefx
Cc: Bill Wert; Author
Subject: Re: [dotnet/corefx] Missing methods in Microsoft.Win32.Registry (#15332)

Agree with @weshaggardhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fweshaggard&data=02%7C01%7Cbillwert%40microsoft.com%7C29e2622450204445f9eb08d444289d3c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636208388607353394&sdata=CBBFFGmUAGzER2Khaq4qO8nbDW74YTrV833df85kXTU%3D&reserved=0. ACL could be problematic, but given that registry is inherently Windows-only I don't see why not.

-
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdotnet%2Fcorefx%2Fissues%2F15332%23issuecomment-274727177&data=02%7C01%7Cbillwert%40microsoft.com%7C29e2622450204445f9eb08d444289d3c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636208388607353394&sdata=t90LXqGHl0gmh83sAUUWCFPwNiT0rixUa22jddYazyc%3D&reserved=0, or mute the threadhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAEUEcKV9RLboMvczWsvu2SbIy-XGNzj5ks5rVaTIgaJpZM4Lojrh&data=02%7C01%7Cbillwert%40microsoft.com%7C29e2622450204445f9eb08d444289d3c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636208388607353394&sdata=bMPTKVcPjtMH1yMAuwB3roelRtk8Q4LxzHLsUg9C7vw%3D&reserved=0.

@weshaggard Yes, I'm still interested in contributing to this issue.

@darxis Thanks for your interest in this. Here is the list of APIs that need to be added for this work:

Microsoft.Win32.RegistryKey.Close()
Microsoft.Win32.RegistryKey.CreateSubKey(System.String, Microsoft.Win32.RegistryKeyPermissionCheck)
Microsoft.Win32.RegistryKey.CreateSubKey(System.String, Microsoft.Win32.RegistryKeyPermissionCheck, Microsoft.Win32.RegistryOptions)
Microsoft.Win32.RegistryKey.CreateSubKey(System.String, Microsoft.Win32.RegistryKeyPermissionCheck, Microsoft.Win32.RegistryOptions, System.Security.AccessControl.RegistrySecurity)
Microsoft.Win32.RegistryKey.CreateSubKey(System.String, Microsoft.Win32.RegistryKeyPermissionCheck, System.Security.AccessControl.RegistrySecurity)
Microsoft.Win32.RegistryKey.GetAccessControl()
Microsoft.Win32.RegistryKey.GetAccessControl(System.Security.AccessControl.AccessControlSections)
Microsoft.Win32.RegistryKey.OpenRemoteBaseKey(Microsoft.Win32.RegistryHive, System.String)
Microsoft.Win32.RegistryKey.OpenRemoteBaseKey(Microsoft.Win32.RegistryHive, System.String, Microsoft.Win32.RegistryView)
Microsoft.Win32.RegistryKey.OpenSubKey(System.String, Microsoft.Win32.RegistryKeyPermissionCheck)
Microsoft.Win32.RegistryKey.OpenSubKey(System.String, Microsoft.Win32.RegistryKeyPermissionCheck, System.Security.AccessControl.RegistryRights)
Microsoft.Win32.RegistryKey.SetAccessControl(System.Security.AccessControl.RegistrySecurity)

Also, RegistryKeyPermissionCheck type should be added to Microsoft.Win32.Registry and Microsoft.Win32.RegistryKey should inherit from System.MarshalByRefObject.

The implementation for these should go in here and all these changes need to be added to the ref as well.

Feel free to work on this :)

@sepidehMS Ok, I'm already working on it. It will take me up to 2 weeks to implement all of these.

@weshaggard I have a question.

To implement the methods that takes a parameter of type System.Security.AccessControl.RegistryRights I have to add a reference to the Microsoft.Win32.Registry.AccessControl assembly. I can't do that, because, the Microsoft.Win32.Registry.AccessControl asembly is already dependent on Microsoft.Win32.Registry (it is using this reference just to implement the RegistryAclExtensions class) as this would cause a circular dependency. The RegistryAclExtensions implements three methods, that I had to implement, as part of this task:

Microsoft.Win32.RegistryKey.GetAccessControl()
Microsoft.Win32.RegistryKey.GetAccessControl(System.Security.AccessControl.AccessControlSections)
Microsoft.Win32.RegistryKey.SetAccessControl(System.Security.AccessControl.RegistrySecurity)

So these methods are already implemented, but as extension methods in the Microsoft.Win32.Registry.AccessControl assembly.

Should we remove the whole RegistryAclExtensions class from the Microsoft.Win32.Registry.AccessControl assembly (then remove the dependency on Microsoft.Win32.Registry, and implement these three methods, directly in the RegistryKey class?
Or should we implement all the methods that are using types from the Microsoft.Win32.Registry.AccessControl assembly as extension methods into Microsoft.Win32.Registry.AccessControl assembly itself?

@darxis Thanks for working on getting this in. I believe we would prefer the second option. @weshaggard Any preferences?

(Sorry I know I typed a response to this the other day I must not have clicked comment or github dropped the ball).

We should move all the types that are needed down into Microsoft.Win32.Registry and leave a type forward for them in Microsoft.Win32.Registry.AccessControl. For compat we cannot completely remove Microsoft.Win32.Registry.AccessControl or the RegistryAclExtensions type.

For reference, in case you were unaware, if there is an extension method and an instance method with the same name on the same type the compiler will always prefer the instance member so having both here isn't going to hurt anything.

@sepidehMS Do I have to also write tests for these methods? Or will they be written as part dotnet/runtime#14232?

@darxis Tests are usually written at the same time new APIs are being added. If you have time, it would be great if you could write some tests for these APIs as well. Thanks for your contribution.

@sepidehMS Ok, I'm going to write some tests and update the PR when done.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

noahfalk picture noahfalk  路  3Comments

chunseoklee picture chunseoklee  路  3Comments

yahorsi picture yahorsi  路  3Comments

v0l picture v0l  路  3Comments

sahithreddyk picture sahithreddyk  路  3Comments