diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index 0c1e39de86..95a3166456 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -1465,6 +1465,46 @@ namespace System.Management.Automation // String.WhitespaceChars will trim aggressively than what the underlying FS does (for ex, NTFS, FAT). internal static readonly char[] PathSearchTrimEnd = { (char)0x9, (char)0xA, (char)0xB, (char)0xC, (char)0xD, (char)0x20, (char)0x85, (char)0xA0 }; } + +#if !UNIX + // This is to reduce the runtime overhead of the feature query + private static readonly Type ComObjectType = typeof(object).Assembly.GetType("System.__ComObject"); +#endif + + internal static bool IsComObject(PSObject psObject) + { +#if UNIX + return false; +#else + if (psObject == null) { return false; } + + object obj = PSObject.Base(psObject); + return IsComObject(obj); +#endif + } + + internal static bool IsComObject(object obj) + { +#if UNIX + return false; +#else + // We can't use System.Runtime.InteropServices.Marshal.IsComObject(obj) since it doesn't work in partial trust. + // + // There could be strongly typed RWCs whose type is not 'System.__ComObject', but the more specific type should + // derive from 'System.__ComObject'. The strongly typed RWCs can be created with 'new' operation via the Primay + // Interop Assembly (PIA). + // For example, with the PIA 'Microsoft.Office.Interop.Excel', you can write the following code: + // var excelApp = new Microsoft.Office.Interop.Excel.Application(); + // Type type = excelApp.GetType(); + // Type comObjectType = typeof(object).Assembly.GetType("System.__ComObject"); + // Console.WriteLine("excelApp type: {0}", type.FullName); + // Console.WriteLine("Is __ComObject assignable from? {0}", comObjectType.IsAssignableFrom(type)); + // and the results are: + // excelApp type: Microsoft.Office.Interop.Excel.ApplicationClass + // Is __ComObject assignable from? True + return obj != null && ComObjectType.IsAssignableFrom(obj.GetType()); +#endif + } } } diff --git a/src/System.Management.Automation/engine/parser/Compiler.cs b/src/System.Management.Automation/engine/parser/Compiler.cs index a0c5540163..321617d773 100644 --- a/src/System.Management.Automation/engine/parser/Compiler.cs +++ b/src/System.Management.Automation/engine/parser/Compiler.cs @@ -504,6 +504,9 @@ namespace System.Management.Automation.Language internal static readonly MethodInfo VariableOps_SetVariableValue = typeof(VariableOps).GetMethod(nameof(VariableOps.SetVariableValue), staticFlags); + internal static readonly MethodInfo Utils_IsComObject = + typeof(Utils).GetMethod(nameof(Utils.IsComObject), staticFlags, binder: null, types: new Type[] {typeof(object)}, modifiers: null); + internal static readonly MethodInfo ClassOps_ValidateSetProperty = typeof(ClassOps).GetMethod(nameof(ClassOps.ValidateSetProperty), staticPublicFlags); internal static readonly MethodInfo ClassOps_CallBaseCtor = diff --git a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs index 2dad4d8ef0..ca1bdb607d 100644 --- a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs +++ b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs @@ -338,34 +338,74 @@ namespace System.Management.Automation.Language internal static class DynamicMetaObjectBinderExtensions { - internal static DynamicMetaObject DeferForPSObject(this DynamicMetaObjectBinder binder, - params DynamicMetaObject[] args) + internal static DynamicMetaObject DeferForPSObject(this DynamicMetaObjectBinder binder, DynamicMetaObject target, bool targetIsComObject = false) { + Diagnostics.Assert(target.Value is PSObject, "target must be a psobject"); + + BindingRestrictions restrictions = BindingRestrictions.Empty; + Expression expr = ProcessOnePSObject(target, ref restrictions, argIsComObject: targetIsComObject); + return new DynamicMetaObject(DynamicExpression.Dynamic(binder, binder.ReturnType, expr), restrictions); + } + + internal static DynamicMetaObject DeferForPSObject(this DynamicMetaObjectBinder binder, DynamicMetaObject target, DynamicMetaObject arg, bool targetIsComObject = false) + { + Diagnostics.Assert(target.Value is PSObject || arg.Value is PSObject, "At least one arg must be a psobject"); + + BindingRestrictions restrictions = BindingRestrictions.Empty; + Expression expr1 = ProcessOnePSObject(target, ref restrictions, argIsComObject: targetIsComObject); + Expression expr2 = ProcessOnePSObject(arg, ref restrictions, argIsComObject: false); + return new DynamicMetaObject(DynamicExpression.Dynamic(binder, binder.ReturnType, expr1, expr2), restrictions); + } + + internal static DynamicMetaObject DeferForPSObject(this DynamicMetaObjectBinder binder, DynamicMetaObject[] args, bool targetIsComObject = false) + { + Diagnostics.Assert(args != null && args.Length > 0, "args should not be null or empty"); Diagnostics.Assert(args.Any(mo => mo.Value is PSObject), "At least one arg must be a psobject"); Expression[] exprs = new Expression[args.Length]; BindingRestrictions restrictions = BindingRestrictions.Empty; - for (int i = 0; i < args.Length; i++) + + // Target maps to arg[0] of the binder. + exprs[0] = ProcessOnePSObject(args[0], ref restrictions, targetIsComObject); + for (int i = 1; i < args.Length; i++) { - var baseValue = PSObject.Base(args[i].Value); - if (baseValue != args[i].Value) - { - exprs[i] = Expression.Call(CachedReflectionInfo.PSObject_Base, - args[i].Expression.Cast(typeof(object))); - restrictions = restrictions - .Merge(args[i].GetSimpleTypeRestriction()) - .Merge(BindingRestrictions.GetExpressionRestriction(Expression.NotEqual(exprs[i], args[i].Expression))); - } - else - { - exprs[i] = args[i].Expression; - restrictions = restrictions.Merge(args[i].PSGetTypeRestriction()); - } + exprs[i] = ProcessOnePSObject(args[i], ref restrictions, argIsComObject: false); } return new DynamicMetaObject(DynamicExpression.Dynamic(binder, binder.ReturnType, exprs), restrictions); } + private static Expression ProcessOnePSObject(DynamicMetaObject arg, ref BindingRestrictions restrictions, bool argIsComObject = false) + { + Expression expr = null; + object baseValue = PSObject.Base(arg.Value); + if (baseValue != arg.Value) + { + expr = Expression.Call(CachedReflectionInfo.PSObject_Base, arg.Expression.Cast(typeof(object))); + + if (argIsComObject) + { + // The 'base' is a COM object, so bake that in the rule. + restrictions = restrictions + .Merge(arg.GetSimpleTypeRestriction()) + .Merge(BindingRestrictions.GetExpressionRestriction(Expression.Call(CachedReflectionInfo.Utils_IsComObject, expr))); + } + else + { + // Use a more general condition for the rule: 'arg' is a PSObject and 'base != arg'. + restrictions = restrictions + .Merge(arg.GetSimpleTypeRestriction()) + .Merge(BindingRestrictions.GetExpressionRestriction(Expression.NotEqual(expr, arg.Expression))); + } + } + else + { + expr = arg.Expression; + restrictions = restrictions.Merge(arg.PSGetTypeRestriction()); + } + return expr; + } + internal static DynamicMetaObject UpdateComRestrictionsForPsObject(this DynamicMetaObject binder, DynamicMetaObject[] args) { // Add a restriction that prevents PSObject arguments (so that they get based) @@ -582,7 +622,7 @@ namespace System.Management.Automation.Language GetRestrictions(target))).WriteToDebugLog(this); } - if (IsComObject(targetValue)) + if (Utils.IsComObject(targetValue)) { // Pretend that all com objects are enumerable, even if they aren't. We do this because it's technically impossible // to know if a com object is enumerable without just trying to cast it to IEnumerable. We could generate a rule like: @@ -594,8 +634,8 @@ namespace System.Management.Automation.Language // EnumerableOps.NonEnumerableObjectEnumerator for more comments on how this works. var bindingRestrictions = BindingRestrictions.GetExpressionRestriction( - Expression.Call(typeof(PSEnumerableBinder).GetMethod("IsComObject", BindingFlags.Static | BindingFlags.NonPublic), - target.Expression)); + Expression.Call(CachedReflectionInfo.Utils_IsComObject, + Expression.Call(CachedReflectionInfo.PSObject_Base, target.Expression))); return new DynamicMetaObject( Expression.Call(CachedReflectionInfo.EnumerableOps_GetCOMEnumerator, target.Expression), bindingRestrictions).WriteToDebugLog(this); } @@ -669,25 +709,6 @@ namespace System.Management.Automation.Language return (result == DynamicMetaObjectExtensions.FakeError) ? null : result; } - // This is to reduce the runtime overhead of the feature query - private static readonly TypeInfo s_comObjectTypeInfo = GetComObjectType(); - - private static TypeInfo GetComObjectType() - { -#if UNIX - return null; -#else - return typeof(object).GetTypeInfo().Assembly.GetType("System.__ComObject").GetTypeInfo(); -#endif - } - - internal static bool IsComObject(object obj) - { - // we can't use System.Runtime.InteropServices.Marshal.IsComObject(obj) since it doesn't work in partial trust - obj = PSObject.Base(obj); - return obj != null && s_comObjectTypeInfo != null && s_comObjectTypeInfo.IsAssignableFrom(obj.GetType().GetTypeInfo()); - } - private static IEnumerator AutomationNullRule(CallSite site, object obj) { return obj == AutomationNull.Value @@ -697,7 +718,7 @@ namespace System.Management.Automation.Language private static IEnumerator NotEnumerableRule(CallSite site, object obj) { - if (!(obj is PSObject) && !(obj is IEnumerable) && !(obj is IEnumerator) && !(obj is DataTable) && !IsComObject(obj)) + if (!(obj is PSObject) && !(obj is IEnumerable) && !(obj is IEnumerator) && !(obj is DataTable) && !Utils.IsComObject(obj)) { return null; } @@ -4975,9 +4996,13 @@ namespace System.Management.Automation.Language if (target.Value is PSObject && (PSObject.Base(target.Value) != target.Value)) { Object baseObject = PSObject.Base(target.Value); - if ((baseObject != null) && (baseObject.GetType().FullName.Equals("System.__ComObject"))) + if (baseObject != null && Utils.IsComObject(baseObject)) { - return this.DeferForPSObject(target).WriteToDebugLog(this); + // We unwrap only if the 'base' is a COM object. It's unnecessary to unwrap in other cases, + // especially in the case of strings, we would lose instance members on the PSObject. + // Therefore, we need to use a stricter restriction to make sure PSObject 'target' with other + // base types doesn't get unwrapped. + return this.DeferForPSObject(target, targetIsComObject: true).WriteToDebugLog(this); } } @@ -5837,9 +5862,13 @@ namespace System.Management.Automation.Language (value.Value is PSObject && (PSObject.Base(value.Value) != value.Value))) { Object baseObject = PSObject.Base(target.Value); - if ((baseObject != null) && (baseObject.GetType().FullName.Equals("System.__ComObject"))) + if (baseObject != null && Utils.IsComObject(baseObject)) { - return this.DeferForPSObject(target, value).WriteToDebugLog(this); + // We unwrap only if the 'base' of 'target' is a COM object. It's unnecessary to unwrap in other cases, + // especially in the case that 'target' is a string, we would lose instance members on the PSObject. + // Therefore, we need to use a stricter restriction to make sure PSObject 'target' with other base types + // doesn't get unwrapped. + return this.DeferForPSObject(target, value, targetIsComObject: true).WriteToDebugLog(this); } } @@ -6354,9 +6383,13 @@ namespace System.Management.Automation.Language args.Any(mo => mo.Value is PSObject && (PSObject.Base(mo.Value) != mo.Value))) { Object baseObject = PSObject.Base(target.Value); - if ((baseObject != null) && (baseObject.GetType().FullName.Equals("System.__ComObject"))) + if (baseObject != null && Utils.IsComObject(baseObject)) { - return this.DeferForPSObject(args.Prepend(target).ToArray()).WriteToDebugLog(this); + // We unwrap only if the 'base' of 'target' is a COM object. It's unnecessary to unwrap in other cases, + // especially in the case that 'target' is a string, we would lose instance members on the PSObject. + // Therefore, we need to use a stricter restriction to make sure other type of PSObject 'target' + // doesn't get unwrapped. + return this.DeferForPSObject(args.Prepend(target).ToArray(), targetIsComObject: true).WriteToDebugLog(this); } } diff --git a/test/powershell/engine/COM/COM.Basic.Tests.ps1 b/test/powershell/engine/COM/COM.Basic.Tests.ps1 index f98139f9b8..e0d277a290 100644 --- a/test/powershell/engine/COM/COM.Basic.Tests.ps1 +++ b/test/powershell/engine/COM/COM.Basic.Tests.ps1 @@ -16,7 +16,11 @@ try { $items = $folder.Items() ## $items is a collection of all items belong to the folder, and it should be enumerated. + $items.Count | Should Be 3 $items | Measure-Object | ForEach-Object Count | Should Be $items.Count + + $names = $items | ForEach-Object { $_.Name } + $names -join "," | Should Be "file1,file2,file3" } It "Should enumerate IEnumVariant interface object without exception" { @@ -26,6 +30,7 @@ try { ## $enumVariant is an IEnumVariant interface of all items belong to the folder, and it should be enumerated. $enumVariant = $items._NewEnum() + $items.Count | Should Be 3 $enumVariant | Measure-Object | ForEach-Object Count | Should Be $items.Count } @@ -43,6 +48,52 @@ try { } } + Describe 'GetMember/SetMember/InvokeMember binders should have more restricted rule for COM object' -Tags "CI" { + BeforeAll { + if ([System.Management.Automation.Platform]::IsWindowsDesktop) { + $null = New-Item -Path $TESTDRIVE/bar -ItemType Directory -Force + + $shell = New-Object -ComObject "Shell.Application" + $folder = $shell.Namespace("$TESTDRIVE") + $item = $folder.Items().Item(0) + $item = [psobject]::AsPSObject($item) + + ## Create a PSObject that has an instance member 'Name' and a script method 'Windows' + $str = Add-Member -InputObject "abc" -MemberType NoteProperty -Name Name -Value "Hello" -PassThru + $str = Add-Member -InputObject $str -MemberType ScriptMethod -Name Windows -Value { "Windows" } -PassThru + } + } + + It "GetMember binder should differentiate PSObject that wraps COM object from other PSObjects" { + ## GetMember on the member name 'Name'. + $entry1 = ($item, "bar") + $entry2 = ($str, "Hello") + + foreach ($pair in ($entry1, $entry2, $entry2, $entry1, $entry1, $entry2)) { + $pair[0].Name | Should Be $pair[1] + } + } + + It "SetMember binder should differentiate PSObject that wraps COM object from other PSObjects" { + ## SetMember on the member name 'Name' + $entry1 = ($item, "foo") + $entry2 = ($str, "World") + + foreach ($pair in ($entry1, $entry2)) { + $pair[0].Name = $pair[1] + $pair[0].Name | Should Be $pair[1] + } + } + + It "InvokeMember binder should differentiate PSObject that wraps COM object from other PSObjects" { + ## InvokeMember on the member name 'Windows' + $shell | ForEach-Object { $_.Windows() } > $null + + ## '$str' is a PSObject that wraps a string, but with ScriptMethod 'Windows' + $str.Windows() | Should Be "Windows" + } + } + } finally { $global:PSdefaultParameterValues = $defaultParamValues }