Check for IDispatch properly

src/coreclr/dlls/mscorrc/mscorrc.rc

@@ -402,7 +402,6 @@ BEGIN
     IDS_EE_CANNOT_COERCE_BYREF_VARIANT      "Object cannot be coerced to the original type of the ByRef VARIANT it was obtained from."
     IDS_EE_WRAPPER_MUST_HAVE_DEF_CONS       "The new wrapper type must have an empty constructor."
     IDS_EE_INVALID_STD_DISPID_NAME          "Standard DISPID member name is formed incorrectly.  The name must be in the following format: [DISPID=XXX]."
-    IDS_EE_NO_IDISPATCH_ON_TARGET           "COM target does not implement IDispatch."
     IDS_EE_NON_STD_NAME_WITH_STD_DISPID     "All named parameters must be specified as [DISPID=XXX] when invoking on a standard members specified as [DISPID=XXX}."
     IDS_EE_INVOKE_NEW_ENUM_INVALID_RETURN   "Variant returned from an Invoke call with a DISPID of -4 does not contain a valid IUnknown pointer."
     IDS_EE_COM_OBJECT_RELEASE_RACE          "COM object that has been separated from its underlying RCW cannot be used. The COM object was released while it was still in use on another thread."
@@ -412,7 +411,6 @@ BEGIN
     IDS_EE_CANNOTCASTSAME                   "[A]%1 cannot be cast to [B]%2. %3. %4."
     IDS_EE_INVALID_VT_FOR_CUSTOM_MARHALER   "Type of the VARIANT specified for a parameter with a custom marshaler is not supported by the custom marshaler."
     IDS_EE_BAD_COMEXTENDS_CLASS             "Types extending from COM objects should override all methods of an interface implemented by the base COM class."
-    IDS_EE_INTERFACE_NOT_DISPATCH_BASED     "The interface does not support late bound calls since it does not derive from IDispatch."
     IDS_EE_MARSHAL_UNMAPPABLE_CHAR          "Cannot marshal: Encountered unmappable character."
 
     IDS_EE_NOCUSTOMMARSHALER                "A call to GetInstance() for custom marshaler '%1' returned null, which is not allowed."

src/coreclr/dlls/mscorrc/resource.h

@@ -195,7 +195,7 @@
 #define IDS_EE_CANNOT_COERCE_BYREF_VARIANT      0x17d2
 #define IDS_EE_WRAPPER_MUST_HAVE_DEF_CONS       0x17d3
 #define IDS_EE_INVALID_STD_DISPID_NAME          0x17d4
-#define IDS_EE_NO_IDISPATCH_ON_TARGET           0x17d5
+
 #define IDS_EE_NON_STD_NAME_WITH_STD_DISPID     0x17d6
 #define IDS_EE_INVOKE_NEW_ENUM_INVALID_RETURN   0x17d7
 #define IDS_EE_COM_OBJECT_RELEASE_RACE          0x17d8
@@ -215,8 +215,6 @@
 #define IDS_EE_MISSING_FIELD                    0x17f7
 #define IDS_EE_MISSING_METHOD                   0x17f8
 
-#define IDS_EE_INTERFACE_NOT_DISPATCH_BASED     0x17f9
-
 #define IDS_EE_UNHANDLED_EXCEPTION              0x17fc
 #define IDS_EE_EXCEPTION_TOSTRING_FAILED        0x17fd
 

src/coreclr/utilcode/check.cpp

@@ -171,7 +171,7 @@ void CHECK::Setup(LPCSTR message, LPCSTR condition, LPCSTR file, INT line)
             // Try to build a stack of condition failures
 
             StackSString context;
-            context.Printf("%s\n\t%s%s FAILED: %s\n\t\t%s, line: %d",
+            context.Printf("%s\n\t%s%s FAILED: %s\n\t\t%s:%d",
                            m_condition,
                            message && *message ? message : "",
                            message && *message ? ": " : "",

src/coreclr/utilcode/debug.cpp

@@ -156,7 +156,7 @@ VOID LogAssert(
 
     // Log asserts to the stress log. Note that we can't include the szExpr b/c that
     // may not be a string literal (particularly for formatt-able asserts).
-    STRESS_LOG2(LF_ASSERT, LL_ALWAYS, "ASSERT:%s, line:%d\n", szFile, iLine);
+    STRESS_LOG2(LF_ASSERT, LL_ALWAYS, "ASSERT:%s:%d\n", szFile, iLine);
 
     SYSTEMTIME st;
 #ifndef TARGET_UNIX

src/coreclr/vm/interoputil.cpp

@@ -3082,6 +3082,7 @@ void IUInvokeDispMethod(
     DISPID              MemberID            = 0;
     ByrefArgumentInfo*  aByrefArgInfos      = NULL;
     BOOL                bSomeArgsAreByref   = FALSE;
+    SafeComHolder<IUnknown> pUnk            = NULL;
     SafeComHolder<IDispatch> pDisp          = NULL;
     SafeComHolder<IDispatchEx> pDispEx      = NULL;
     VariantPtrHolder    pVarResult          = NULL;
@@ -3121,7 +3122,7 @@ void IUInvokeDispMethod(
     {
         CorIfaceAttr ifaceType = pInvokedMT->GetComInterfaceType();
         if (!IsDispatchBasedItf(ifaceType))
-            COMPlusThrow(kTargetInvocationException, IDS_EE_INTERFACE_NOT_DISPATCH_BASED);
+            COMPlusThrow(kTargetException, W("TargetInvocation_InterfaceNotIDispatch"));
     }
 
     // Validate that the target is a COM object.
@@ -3156,7 +3157,10 @@ void IUInvokeDispMethod(
     {
         // The invoked type is a dispatch or dual interface so we will make the
         // invocation on it.
-        pDisp = (IDispatch *)ComObject::GetComIPFromRCWThrowing(pTarget, pInvokedMT);
+        pUnk = ComObject::GetComIPFromRCWThrowing(pTarget, pInvokedMT);
+        hr = SafeQueryInterface(pUnk, IID_IDispatch, (IUnknown**)&pDisp);
+        if (FAILED(hr))
+            COMPlusThrow(kTargetException, W("TargetInvocation_TargetDoesNotImplementIDispatch"));
     }
     else
     {
@@ -3167,9 +3171,9 @@ void IUInvokeDispMethod(
         RCWPROTECT_BEGIN(pRCW, *pTarget);
 
         // Retrieve the IDispatch pointer from the wrapper.
-        pDisp = (IDispatch*)pRCW->GetIDispatch();
+        pDisp = pRCW->GetIDispatch();
         if (!pDisp)
-            COMPlusThrow(kTargetInvocationException, IDS_EE_NO_IDISPATCH_ON_TARGET);
+            COMPlusThrow(kTargetException, W("TargetInvocation_TargetDoesNotImplementIDispatch"));
 
         // If we aren't ignoring case, then we need to try and QI for IDispatchEx to
         // be able to use IDispatchEx::GetDispID() which has a flag to control case

src/libraries/System.Private.CoreLib/src/Resources/Strings.resx

@@ -3494,6 +3494,12 @@
   <data name="TargetInvocation_MethodCannotBeInvoked" xml:space="preserve">
     <value>Method cannot be invoked.</value>
   </data>
+  <data name="TargetInvocation_TargetDoesNotImplementIDispatch" xml:space="preserve">
+    <value>COM target does not implement IDispatch.</value>
+  </data>
+  <data name="TargetInvocation_InterfaceNotIDispatch" xml:space="preserve">
+    <value>The interface does not support late bound calls since it does not derive from IDispatch.</value>
+  </data>
   <data name="Task_ContinueWith_ESandLR" xml:space="preserve">
     <value>The specified TaskContinuationOptions combined LongRunning and ExecuteSynchronously.  Synchronous continuations should not be long running.</value>
   </data>

src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetObjectForIUnknownTests.Windows.cs

@@ -2,6 +2,8 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Collections.Generic;
+using System.Reflection;
+using System.Runtime.InteropServices.Marshalling;
 using System.Runtime.InteropServices.Tests.Common;
 using Xunit;
 
@@ -33,5 +35,73 @@ public void GetObjectForIUnknown_ComObject_ReturnsExpected(object o)
         {
             GetObjectForIUnknown_ValidPointer_ReturnsExpected(o);
         }
+
+        [ComImport]
+        [ComVisible(true)]
+        [Guid("20d5e748-3e41-414f-ba43-542c6c47bd21")]
+        [InterfaceType(ComInterfaceType.InterfaceIsIDispatch)]
+        public interface ICallback
+        {
+            void M();
+        }
+
+        class Callback : ICallback
+        {
+            public void M() { }
+        }
+
+        [GeneratedComInterface]
+        [Guid("20d5e748-3e41-414f-ba43-542c6c47bd21")]
+        public partial interface ICallbackWrapper
+        {
+            void M();
+        }
+
+        // Notice this class doesn't implement IDispatch nor it is specified on
+        // the Callback class. The user workaround would be to mark the
+        // Callback class as ComVisible(true) and public. We wrap the input
+        // to avoid the automatic detection the runtime does on COM objects.
+        // This simulates the failure mode we are trying to detect.
+        [GeneratedComClass]
+        public partial class CallbackWrapper : ICallbackWrapper
+        {
+            private IntPtr _wrapper;
+            public CallbackWrapper(IntPtr wrapper)
+            {
+                _wrapper = wrapper;
+                Marshal.AddRef(_wrapper);
+            }
+
+            ~CallbackWrapper()
+            {
+                Marshal.Release(_wrapper);
+            }
+
+            public void M() => throw new NotImplementedException();
+        }
+
+        [UnmanagedCallersOnly]
+        private static unsafe IntPtr WrapCallback(IntPtr p)
+        {
+            // See CallbackWrapper for why we wrap the input.
+            var wrapper = new CallbackWrapper(p);
+            return (IntPtr)ComInterfaceMarshaller<ICallbackWrapper>.ConvertToUnmanaged(wrapper);
+        }
+
+        private delegate ICallback WrapDelegate(ICallback cb);
+
+        // This test is validating a niche case is detected where a class implementing COM "callback"
+        // interface is marshalled but fails to indicate it is COM visible and thus IDispatch isn't
+        // provided by the runtime. This most often occurs in out-of-proc COM scenarios.
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsBuiltInComEnabled))]
+        public unsafe void GetObjectForIUnknown_ComObject_MissingIDispatchOnTarget()
+        {
+            // Use a delegate to trigger COM interop marshalling.
+            var fptr = Marshal.GetDelegateForFunctionPointer<WrapDelegate>((IntPtr)(delegate* unmanaged<IntPtr, IntPtr>)&WrapCallback);
+            ICallback icb = fptr(new Callback());
+
+            Exception ex = Assert.Throws<TargetException>(() => icb.M());
+            Assert.Equal("COM target does not implement IDispatch.", ex.Message);
+        }
     }
 }