diff --git a/Settings.StyleCop b/Settings.StyleCop index 4d471946c64..3cd8a6eb5a9 100644 --- a/Settings.StyleCop +++ b/Settings.StyleCop @@ -155,6 +155,7 @@ rs ps op + sb diff --git a/src/System.Management.Automation/engine/CommandInfo.cs b/src/System.Management.Automation/engine/CommandInfo.cs index 00800ec0f2a..ddf90507f42 100644 --- a/src/System.Management.Automation/engine/CommandInfo.cs +++ b/src/System.Management.Automation/engine/CommandInfo.cs @@ -442,8 +442,11 @@ private MergedCommandParameterMetadata GetMergedCommandParameterMetadataSafely() // that can mess up the runspace our CommandInfo object came from. var runspace = (RunspaceBase)_context.CurrentRunspace; - if (!runspace.RunActionIfNoRunningPipelinesWithThreadCheck( - () => GetMergedCommandParameterMetadata(out result))) + if (runspace.CanRunActionInCurrentPipeline()) + { + GetMergedCommandParameterMetadata(out result); + } + else { _context.Events.SubscribeEvent( source: null, diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index bda39429fdc..be342100568 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -806,8 +806,12 @@ internal static bool IsValidPSEditionValue(string editionValue) {nameof(UpdatableHelp), @"Software\Policies\Microsoft\PowerShellCore\UpdatableHelp"}, {nameof(ConsoleSessionConfiguration), @"Software\Policies\Microsoft\PowerShellCore\ConsoleSessionConfiguration"} }; - private static readonly ConcurrentDictionary, PolicyBase> s_cachedPoliciesFromRegistry = - new ConcurrentDictionary, PolicyBase>(); + + private static readonly ConcurrentDictionary> s_cachedPoliciesFromRegistry = + new ConcurrentDictionary>(); + + private static readonly Func> s_subCacheCreationDelegate = + key => new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); /// /// The implementation of fetching a specific kind of policy setting from the given configuration scope. @@ -915,6 +919,8 @@ internal static bool IsValidPSEditionValue(string editionValue) private static T GetPolicySettingFromGPO(ConfigScope[] preferenceOrder) where T : PolicyBase, new() { PolicyBase policy = null; + string policyName = typeof(T).Name; + foreach (ConfigScope scope in preferenceOrder) { if (InternalTestHooks.BypassGroupPolicyCaching) @@ -923,13 +929,10 @@ internal static bool IsValidPSEditionValue(string editionValue) } else { - var key = Tuple.Create(scope, typeof(T).Name); - if (!s_cachedPoliciesFromRegistry.TryGetValue(key, out policy)) + var subordinateCache = s_cachedPoliciesFromRegistry.GetOrAdd(scope, s_subCacheCreationDelegate); + if (!subordinateCache.TryGetValue(policyName, out policy)) { - lock (s_cachedPoliciesFromRegistry) - { - policy = s_cachedPoliciesFromRegistry.GetOrAdd(key, tuple => GetPolicySettingFromGPOImpl(tuple.Item1)); - } + policy = subordinateCache.GetOrAdd(policyName, key => GetPolicySettingFromGPOImpl(scope)); } } diff --git a/src/System.Management.Automation/engine/hostifaces/ConnectionBase.cs b/src/System.Management.Automation/engine/hostifaces/ConnectionBase.cs index 926e55a8912..02622582f90 100644 --- a/src/System.Management.Automation/engine/hostifaces/ConnectionBase.cs +++ b/src/System.Management.Automation/engine/hostifaces/ConnectionBase.cs @@ -971,30 +971,16 @@ protected void StopPipelines() } } - internal bool RunActionIfNoRunningPipelinesWithThreadCheck(Action action) + internal bool CanRunActionInCurrentPipeline() { - bool ranit = false; - bool shouldRunAction = false; lock (_pipelineListLock) { // If we have no running pipeline, or if the currently running pipeline is // the same as the current thread, then execute the action. - var pipelineRunning = _currentlyRunningPipeline as PipelineBase; - if (pipelineRunning == null || - Thread.CurrentThread.Equals(pipelineRunning.NestedPipelineExecutionThread)) - { - shouldRunAction = true; - } + return pipelineRunning == null || + Thread.CurrentThread == pipelineRunning.NestedPipelineExecutionThread; } - - if (shouldRunAction) - { - action(); - ranit = true; - } - - return ranit; } /// diff --git a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs index c728da1d604..f553b520bed 100644 --- a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs +++ b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs @@ -7,14 +7,13 @@ using System.Globalization; using System.IO; using System.Management.Automation.Configuration; +using System.Management.Automation.Internal; using System.Management.Automation.Runspaces; using System.Security; using System.Text; using System.Threading; using System.Threading.Tasks; -using Microsoft.PowerShell.Commands; - namespace System.Management.Automation.Host { /// @@ -927,7 +926,10 @@ internal void TranscribeError(ExecutionContext context, InvocationInfo invocatio /// internal static TranscriptionOption GetSystemTranscriptOption(TranscriptionOption currentTranscript) { - var transcription = Utils.GetPolicySetting(Utils.SystemWideThenCurrentUserConfig); + var transcription = InternalTestHooks.BypassGroupPolicyCaching + ? Utils.GetPolicySetting(Utils.SystemWideThenCurrentUserConfig) + : s_transcriptionSettingCache.Value; + if (transcription != null) { // If we have an existing system transcript for this process, use that. @@ -948,6 +950,9 @@ internal static TranscriptionOption GetSystemTranscriptOption(TranscriptionOptio internal static TranscriptionOption systemTranscript = null; private static object s_systemTranscriptLock = new Object(); + private static Lazy s_transcriptionSettingCache = new Lazy( + () => Utils.GetPolicySetting(Utils.SystemWideThenCurrentUserConfig), + isThreadSafe: true); private static TranscriptionOption GetTranscriptOptionFromSettings(Transcription transcriptConfig, TranscriptionOption currentTranscript) { diff --git a/src/System.Management.Automation/engine/lang/scriptblock.cs b/src/System.Management.Automation/engine/lang/scriptblock.cs index 1e455244105..8a16a5f8726 100644 --- a/src/System.Management.Automation/engine/lang/scriptblock.cs +++ b/src/System.Management.Automation/engine/lang/scriptblock.cs @@ -975,7 +975,8 @@ internal void InvokeWithPipe( try { var runspace = (RunspaceBase)context.CurrentRunspace; - shouldGenerateEvent = !runspace.RunActionIfNoRunningPipelinesWithThreadCheck(() => + if (runspace.CanRunActionInCurrentPipeline()) + { InvokeWithPipeImpl( useLocalScope, functionsToDefine, @@ -986,7 +987,12 @@ internal void InvokeWithPipe( scriptThis, outputPipe, invocationInfo, - args)); + args); + } + else + { + shouldGenerateEvent = true; + } } finally { diff --git a/src/System.Management.Automation/engine/runtime/CompiledScriptBlock.cs b/src/System.Management.Automation/engine/runtime/CompiledScriptBlock.cs index 13a11bec922..36f91298300 100644 --- a/src/System.Management.Automation/engine/runtime/CompiledScriptBlock.cs +++ b/src/System.Management.Automation/engine/runtime/CompiledScriptBlock.cs @@ -322,6 +322,7 @@ private IParameterMetadataProvider DelayParseScriptText() internal Guid Id { get; private set; } internal bool HasLogged { get; set; } + internal bool SkipLogging { get; set; } internal bool IsFilter { get; private set; } internal bool IsProductCode @@ -824,6 +825,13 @@ internal bool HasLogged set { _scriptBlockData.HasLogged = value; } } + internal bool SkipLogging + { + get { return _scriptBlockData.SkipLogging; } + + set { _scriptBlockData.SkipLogging = value; } + } + internal Assembly AssemblyDefiningPSTypes { set; get; } internal HelpInfo GetHelpInfo(ExecutionContext context, CommandInfo commandInfo, bool dontSearchOnRemoteComputer, @@ -1111,7 +1119,7 @@ internal void InvokeWithPipeImpl(ScriptBlockClauseToInvoke clauseToInvoke, { if (context.EngineSessionState.CurrentScope.LocalsTuple == null) { - // If the locals tuple is, that means either: + // If the locals tuple is null, that means either: // * we're invoking a script block for a module // * something unexpected context.EngineSessionState.CurrentScope.LocalsTuple = locals; @@ -1361,53 +1369,61 @@ internal bool Compile(bool optimized) internal static void LogScriptBlockCreation(ScriptBlock scriptBlock, bool force) { + if (scriptBlock.HasLogged && !InternalTestHooks.ForceScriptBlockLogging) + { + // Fast exit if the script block is already logged and we are not force re-logging in tests. + return; + } + ScriptBlockLogging logSetting = GetScriptBlockLoggingSetting(); if (force || logSetting?.EnableScriptBlockLogging == true) { - if (!scriptBlock.HasLogged || InternalTestHooks.ForceScriptBlockLogging) + // If script block logging is explicitly disabled, or it's from a trusted + // file or internal, skip logging. + if (logSetting?.EnableScriptBlockLogging == false || + scriptBlock.ScriptBlockData.IsProductCode) { - // If script block logging is explicitly disabled, or it's from a trusted - // file or internal, skip logging. - if (logSetting?.EnableScriptBlockLogging == false || - scriptBlock.ScriptBlockData.IsProductCode) - { - return; - } + scriptBlock.SkipLogging = true; + return; + } - string scriptBlockText = scriptBlock.Ast.Extent.Text; - bool written = false; + string scriptBlockText = scriptBlock.Ast.Extent.Text; + bool written = false; - // Maximum size of ETW events is 64kb. Split a message if it is larger than 20k (Unicode) characters. - if (scriptBlockText.Length < 20000) - { - written = WriteScriptBlockToLog(scriptBlock, 0, 1, scriptBlock.Ast.Extent.Text); - } - else + // Maximum size of ETW events is 64kb. Split a message if it is larger than 20k (Unicode) characters. + if (scriptBlockText.Length < 20000) + { + written = WriteScriptBlockToLog(scriptBlock, 0, 1, scriptBlock.Ast.Extent.Text); + } + else + { + // But split the segments into random sizes (10k + between 0 and 10kb extra) + // so that attackers can't creatively force their scripts to span well-known + // segments (making simple rules less reliable). + int segmentSize = 10000 + (new Random()).Next(10000); + int segments = (int)Math.Floor((double)(scriptBlockText.Length / segmentSize)) + 1; + int currentLocation = 0; + int currentSegmentSize = 0; + + for (int segment = 0; segment < segments; segment++) { - // But split the segments into random sizes (10k + between 0 and 10kb extra) - // so that attackers can't creatively force their scripts to span well-known - // segments (making simple rules less reliable). - int segmentSize = 10000 + (new Random()).Next(10000); - int segments = (int)Math.Floor((double)(scriptBlockText.Length / segmentSize)) + 1; - int currentLocation = 0; - int currentSegmentSize = 0; - - for (int segment = 0; segment < segments; segment++) - { - currentLocation = segment * segmentSize; - currentSegmentSize = Math.Min(segmentSize, scriptBlockText.Length - currentLocation); + currentLocation = segment * segmentSize; + currentSegmentSize = Math.Min(segmentSize, scriptBlockText.Length - currentLocation); - string textToLog = scriptBlockText.Substring(currentLocation, currentSegmentSize); - written = WriteScriptBlockToLog(scriptBlock, segment, segments, textToLog); - } + string textToLog = scriptBlockText.Substring(currentLocation, currentSegmentSize); + written = WriteScriptBlockToLog(scriptBlock, segment, segments, textToLog); } + } - if (written) - { - scriptBlock.HasLogged = true; - } + if (written) + { + scriptBlock.HasLogged = true; } } + else + { + scriptBlock.SkipLogging = true; + } } private static bool WriteScriptBlockToLog(ScriptBlock scriptBlock, int segment, int segments, string textToLog) @@ -1582,6 +1598,9 @@ private static bool GetAndValidateEncryptionRecipients(ScriptBlock scriptBlock, private static string s_lastSeenCertificate = string.Empty; private static bool s_hasProcessedCertificate = false; private static CmsMessageRecipient[] s_encryptionRecipients = null; + private static Lazy s_sbLoggingSettingCache = new Lazy( + () => Utils.GetPolicySetting(Utils.SystemWideThenCurrentUserConfig), + isThreadSafe: true); // Reset any static caches if the certificate has changed private static void ResetCertificateCacheIfNeeded(string certificate) @@ -1596,7 +1615,12 @@ private static void ResetCertificateCacheIfNeeded(string certificate) private static ScriptBlockLogging GetScriptBlockLoggingSetting() { - return Utils.GetPolicySetting(Utils.SystemWideThenCurrentUserConfig); + if (InternalTestHooks.BypassGroupPolicyCaching) + { + return Utils.GetPolicySetting(Utils.SystemWideThenCurrentUserConfig); + } + + return s_sbLoggingSettingCache.Value; } // Quick check for script blocks that may have suspicious content. If this @@ -1919,17 +1943,16 @@ char ToLower(char c) internal static void LogScriptBlockStart(ScriptBlock scriptBlock, Guid runspaceId) { - // When invoking, log the creation of the script block if it has suspicious - // content - bool forceLogCreation = false; - if (scriptBlock._scriptBlockData.HasSuspiciousContent) + // Fast exit if the script block can skip logging. + if (!scriptBlock.SkipLogging) { - forceLogCreation = true; - } + // When invoking, log the creation of the script block if it has suspicious content + bool forceLogCreation = scriptBlock._scriptBlockData.HasSuspiciousContent; - // We delay logging the creation util the 'Start' so that we can be sure we've - // properly analyzed the script block's security. - LogScriptBlockCreation(scriptBlock, forceLogCreation); + // We delay logging the creation until the 'Start' so that we can be sure we've + // properly analyzed the script block's security. + LogScriptBlockCreation(scriptBlock, forceLogCreation); + } if (GetScriptBlockLoggingSetting()?.EnableScriptBlockInvocationLogging == true) { diff --git a/stylecop.json b/stylecop.json index cc46a542077..de65ff982b4 100644 --- a/stylecop.json +++ b/stylecop.json @@ -25,7 +25,7 @@ }, "namingRules" : { "allowCommonHungarianPrefixes" : true, - "allowedHungarianPrefixes" : [ "n", "r", "l", "i", "io", "is", "fs", "lp", "dw", "h", "rs", "ps", "op" ] + "allowedHungarianPrefixes" : [ "n", "r", "l", "i", "io", "is", "fs", "lp", "dw", "h", "rs", "ps", "op", "sb" ] }, "maintainabilityRules" : { "topLevelTypes" : [