From bd83cbabe290b5b2cde4d909e904cdc805178b2a Mon Sep 17 00:00:00 2001 From: PowerShell Team Bot <69177312+pwshBot@users.noreply.github.com> Date: Tue, 3 Jun 2025 13:07:35 -0700 Subject: [PATCH] [release/v7.5] Move .NET method invocation logging to after the needed type conversion is done for method arguments (#25357) Co-authored-by: Dongbo Wang Co-authored-by: Travis Plunk Co-authored-by: Travis Plunk --- .../engine/runtime/Binding/Binders.cs | 107 ++++++++++++++---- 1 file changed, 86 insertions(+), 21 deletions(-) diff --git a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs index de36b9d724..f402054743 100644 --- a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs +++ b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs @@ -6943,20 +6943,6 @@ namespace System.Management.Automation.Language expr = Expression.Block(expr, ExpressionCache.AutomationNullConstant); } - // Expression block runs two expressions in order: - // - Log method invocation to AMSI Notifications (can throw PSSecurityException) - // - Invoke method - string targetName = methodInfo.ReflectedType?.FullName ?? string.Empty; - expr = Expression.Block( - Expression.Call( - CachedReflectionInfo.MemberInvocationLoggingOps_LogMemberInvocation, - Expression.Constant(targetName), - Expression.Constant(name), - Expression.NewArrayInit( - typeof(object), - args.Select(static e => e.Expression.Cast(typeof(object))))), - expr); - // If we're calling SteppablePipeline.{Begin|Process|End}, we don't want // to wrap exceptions - this is very much a special case to help error // propagation and ensure errors are attributed to the correct code (the @@ -7119,6 +7105,7 @@ namespace System.Management.Automation.Language invocationType != MethodInvocationType.NonVirtual; var parameters = mi.GetParameters(); var argExprs = new Expression[parameters.Length]; + var argsToLog = new List(Math.Max(parameters.Length, args.Length)); for (int i = 0; i < parameters.Length; ++i) { @@ -7143,16 +7130,21 @@ namespace System.Management.Automation.Language if (expandParameters) { - argExprs[i] = Expression.NewArrayInit( - paramElementType, - args.Skip(i).Select( - a => a.CastOrConvertMethodArgument( + IEnumerable elements = args + .Skip(i) + .Select(a => + a.CastOrConvertMethodArgument( paramElementType, paramName, mi.Name, allowCastingToByRefLikeType: false, temps, - initTemps))); + initTemps)) + .ToList(); + + argExprs[i] = Expression.NewArrayInit(paramElementType, elements); + // User specified the element arguments, so we log them instead of the compiler-created array. + argsToLog.AddRange(elements); } else { @@ -7163,13 +7155,18 @@ namespace System.Management.Automation.Language allowCastingToByRefLikeType: false, temps, initTemps); + argExprs[i] = arg; + argsToLog.Add(arg); } } else if (i >= args.Length) { - Diagnostics.Assert(parameters[i].IsOptional, + // We don't log the default value for an optional parameter, as it's not specified by the user. + Diagnostics.Assert( + parameters[i].IsOptional, "if there are too few arguments, FindBestMethod should only succeed if parameters are optional"); + var argValue = parameters[i].DefaultValue; if (argValue == null) { @@ -7207,17 +7204,25 @@ namespace System.Management.Automation.Language var psRefValue = Expression.Property(args[i].Expression.Cast(typeof(PSReference)), CachedReflectionInfo.PSReference_Value); initTemps.Add(Expression.Assign(temp, psRefValue.Convert(temp.Type))); copyOutTemps.Add(Expression.Assign(psRefValue, temp.Cast(typeof(object)))); + argExprs[i] = temp; + argsToLog.Add(temp); } else { - argExprs[i] = args[i].CastOrConvertMethodArgument( + var convertedArg = args[i].CastOrConvertMethodArgument( parameterType, paramName, mi.Name, allowCastingToByRefLikeType, temps, initTemps); + + argExprs[i] = convertedArg; + // If the converted arg is a byref-like type, then we log the original arg. + argsToLog.Add(convertedArg.Type.IsByRefLike + ? args[i].Expression + : convertedArg); } } } @@ -7263,6 +7268,12 @@ namespace System.Management.Automation.Language } } + // We need to add one expression to log the .NET invocation before actually invoking: + // - Log method invocation to AMSI Notifications (can throw PSSecurityException) + // - Invoke method + string targetName = mi.ReflectedType?.FullName ?? string.Empty; + string methodName = mi.Name is ".ctor" ? "new" : mi.Name; + if (temps.Count > 0) { if (call.Type != typeof(void) && copyOutTemps.Count > 0) @@ -7273,8 +7284,13 @@ namespace System.Management.Automation.Language copyOutTemps.Add(retValue); } + AddMemberInvocationLogging(initTemps, targetName, methodName, argsToLog); call = Expression.Block(call.Type, temps, initTemps.Append(call).Concat(copyOutTemps)); } + else + { + call = AddMemberInvocationLogging(call, targetName, methodName, argsToLog); + } return call; } @@ -7566,6 +7582,55 @@ namespace System.Management.Automation.Language } } +#nullable enable + private static Expression AddMemberInvocationLogging( + Expression expr, + string targetName, + string name, + List args) + { +#if UNIX + // For efficiency this is a no-op on non-Windows platforms. + return expr; +#else + Expression[] invocationArgs = new Expression[args.Count]; + for (int i = 0; i < args.Count; i++) + { + invocationArgs[i] = args[i].Cast(typeof(object)); + } + + return Expression.Block( + Expression.Call( + CachedReflectionInfo.MemberInvocationLoggingOps_LogMemberInvocation, + Expression.Constant(targetName), + Expression.Constant(name), + Expression.NewArrayInit(typeof(object), invocationArgs)), + expr); +#endif + } + + private static void AddMemberInvocationLogging( + List exprs, + string targetName, + string name, + List args) + { +#if !UNIX + Expression[] invocationArgs = new Expression[args.Count]; + for (int i = 0; i < args.Count; i++) + { + invocationArgs[i] = args[i].Cast(typeof(object)); + } + + exprs.Add(Expression.Call( + CachedReflectionInfo.MemberInvocationLoggingOps_LogMemberInvocation, + Expression.Constant(targetName), + Expression.Constant(name), + Expression.NewArrayInit(typeof(object), invocationArgs))); +#endif + } +#nullable disable + #endregion }