dotnet / dotnet/wpf

Review COM interface declarations for conflicting ComImport/ComVisible attributes

Open
#1,860 0 comments 0 reactions 0 assignees View on GitHub
Bug
Dominant language
C#
Stars
7.7k
Forks
1.3k
Avg merge
1d 11h
Merged PRs (30d)
61

Description

Follow-up issue from the same problem in dotnet/winforms#1878

There are several interfaces declared with both `[ComImport]` and `[ComVisible(true)]` - this is wrong, ComVisible(true) is only for interfaces owned by the assembly. It probably had little consequence while WPF was part of Desktop Framework, but now that it can be deployed with the application these should be corrected, otherwise you risk people corrupting the registry entries owned by Windows.

There probably should be a general review of all ComVisible(true) attributes to make sure annotated classes/code is actually owned by WPF and not just imported. Imported classes/interfaces should use ComImport attributes. A quick search finds 63 matches in 47 files, most in automation, I don't know if those are correct and WPF actually owns them, or if they should be imported instead.

There are a few which are definitely wrong:
* [IOleCommandTarget](https://github.com/dotnet/wpf/blob/master/src/Microsoft.DotNet.Wpf/src/Shared/MS/Win32/NativeMethodsCLR.cs#L2511) (NativeMethodsCLR.cs)
* [IOleCommandTarget](https://github.com/dotnet/wpf/blob/master/src/Microsoft.DotNet.Wpf/src/Shared/MS/Win32/NativeMethodsOther.cs#L143) (NativeMethodsOther.cs)
* [IInternetSecurityManager](https://github.com/dotnet/wpf/blob/master/src/Microsoft.DotNet.Wpf/src/Shared/MS/Win32/UnsafeNativeMethodsCLR.cs#L3048) (currently in a disabled #if block, but should be fixed anyways unless you want to remove it entirely)
* [IFilter](https://github.com/dotnet/wpf/blob/master/src/Microsoft.DotNet.Wpf/src/PresentationFramework/MS/Internal/Interop/indexingFilter.cs#L525)
* [IPersistStream](https://github.com/dotnet/wpf/blob/master/src/Microsoft.DotNet.Wpf/src/PresentationFramework/MS/Internal/Interop/indexingFilter.cs#L581)
* [IStream](https://github.com/dotnet/wpf/blob/master/src/Microsoft.DotNet.Wpf/src/PresentationFramework/MS/Internal/Interop/indexingFilter.cs#L655)
* [IPersistStreamWithArrays](https://github.com/dotnet/wpf/blob/master/src/Microsoft.DotNet.Wpf/src/PresentationFramework/MS/Internal/Interop/indexingFilter.cs#L748)
* [IPersistFile](https://github.com/dotnet/wpf/blob/master/src/Microsoft.DotNet.Wpf/src/PresentationFramework/MS/Internal/Interop/indexingFilter.cs#L822)

Those should all be only ComImport without a ComVisibile, since they are owned by the OS. Having someone attempt to run WPF assemblies deployed with his application through regasm or equivalent tooling (e.g. during an installer) would overwrite the OS registration of those interfaces.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.