From e845112a8cf0d0c8c5208cb834272972f2d4d18f Mon Sep 17 00:00:00 2001 From: Jonny Eskew Date: Tue, 28 Jan 2025 14:24:15 -0500 Subject: [PATCH] Reuse already updated object types when recursively removing type property flags (#16235) Resolves #16219 The server crash reported in #16219 was caused by https://github.com/Azure/bicep/pull/15825/files#diff-7d66583c4922f57735a4eac96cd074d356e8eace34ec5900860f093f3d0cbc69R701. The FindPossibleSecretsVisitor relies on object identity to detect and stop infinitely walking recursive types, and the linked line caused otherwise identical object types within the assigned type of a `resource` symbol to have different identities. ###### Microsoft Reviewers: [Open in CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/Azure/bicep/pull/16235) --- global.json | 2 +- src/Bicep.Cli/packages.lock.json | 6 ++-- .../ScenarioTests.cs | 30 ++++++++++++++++++ .../Common/FindPossibleSecretsVisitorTests.cs | 5 ++- .../Common/FindPossibleSecretsVisitor.cs | 21 ++++++------- src/Bicep.Core/Emit/ExpressionConverter.cs | 6 ++-- src/Bicep.Core/TypeSystem/TypeHelper.cs | 31 ++++++++++++++----- src/Bicep.Core/packages.lock.json | 6 ++-- src/Bicep.Decompiler/packages.lock.json | 6 ++-- src/Bicep.IO/packages.lock.json | 6 ++-- src/Bicep.Local.Deploy/packages.lock.json | 6 ++-- .../packages.lock.json | 6 ++-- src/Bicep.Wasm/packages.lock.json | 6 ++-- 13 files changed, 91 insertions(+), 46 deletions(-) diff --git a/global.json b/global.json index 1148640d2a8..9d54dc67373 100644 --- a/global.json +++ b/global.json @@ -4,7 +4,7 @@ }, "sdk": { "allowPrerelease": false, - "version": "8.0.404", + "version": "8.0.405", "rollForward": "disable" } } diff --git a/src/Bicep.Cli/packages.lock.json b/src/Bicep.Cli/packages.lock.json index 31f60fbe9bb..f425de12f81 100644 --- a/src/Bicep.Cli/packages.lock.json +++ b/src/Bicep.Cli/packages.lock.json @@ -27,9 +27,9 @@ }, "Microsoft.NET.ILLink.Tasks": { "type": "Direct", - "requested": "[8.0.11, )", - "resolved": "8.0.11", - "contentHash": "zk5lnZrYJgtuJG8L4v17Ej8rZ3PUcR2iweNV08BaO5LbYHIi2wNaVNcJoLxvqgQdnjLlKnCCfVGLDr6QHeAarQ==" + "requested": "[8.0.12, )", + "resolved": "8.0.12", + "contentHash": "FV4HnQ3JI15PHnJ5PGTbz+rYvrih42oLi/7UMIshNwCwUZhTq13UzrggtXk4ygrcMcN+4jsS6hhshx2p/Zd0ig==" }, "Microsoft.SourceLink.GitHub": { "type": "Direct", diff --git a/src/Bicep.Core.IntegrationTests/ScenarioTests.cs b/src/Bicep.Core.IntegrationTests/ScenarioTests.cs index dc82de7cec2..36b35db17c0 100644 --- a/src/Bicep.Core.IntegrationTests/ScenarioTests.cs +++ b/src/Bicep.Core.IntegrationTests/ScenarioTests.cs @@ -6747,4 +6747,34 @@ param descriptionParam string ("BCP265", DiagnosticLevel.Error, "The name \"description\" is not a function. Did you mean \"sys.description\"?"), }); } + + [TestMethod] + public void Test_Issue16219() + { + // https://www.github.com/Azure/bicep/issues/16219 + var result = CompilationHelper.Compile(""" + @description('Required. The name of the Public IP Address.') + param name string + + resource publicIpAddress 'Microsoft.Network/publicIPAddresses@2023-09-01' = { + name: name + location: resourceGroup().location + sku: { + name: 'Basic' + tier: 'Regional' + } + properties: { + publicIPAddressVersion: 'IPv4' + publicIPAllocationMethod: 'Static' + idleTimeoutInMinutes: 4 + ipTags: [] + } + } + + @description('The public IP address of the public IP address resource.') + output ipAddress string = contains(publicIpAddress.properties, 'ipAddress') ? publicIpAddress.properties.ipAddress : '' + """); + + result.ExcludingDiagnostics("use-safe-access").Should().NotHaveAnyDiagnostics(); + } } diff --git a/src/Bicep.Core.UnitTests/Diagnostics/Linter/Common/FindPossibleSecretsVisitorTests.cs b/src/Bicep.Core.UnitTests/Diagnostics/Linter/Common/FindPossibleSecretsVisitorTests.cs index bea6636c0f4..fe69fa992e9 100644 --- a/src/Bicep.Core.UnitTests/Diagnostics/Linter/Common/FindPossibleSecretsVisitorTests.cs +++ b/src/Bicep.Core.UnitTests/Diagnostics/Linter/Common/FindPossibleSecretsVisitorTests.cs @@ -105,7 +105,6 @@ param storageName string output test = v.listAnything().keys[0].value " )] - /* TODO: blocked by https://github.com/Azure/bicep/issues/4833 [DataRow(@" param storageName string @@ -119,8 +118,8 @@ param storageName string value: storage.listAnything().keys[0].value } ", - "Outputs should not contain secrets. function 'listAnything'" - )]*/ + "function 'listAnything'" + )] [DataRow( @" param storageName string diff --git a/src/Bicep.Core/Analyzers/Linter/Common/FindPossibleSecretsVisitor.cs b/src/Bicep.Core/Analyzers/Linter/Common/FindPossibleSecretsVisitor.cs index e15dae42e5a..c7f14697ae9 100644 --- a/src/Bicep.Core/Analyzers/Linter/Common/FindPossibleSecretsVisitor.cs +++ b/src/Bicep.Core/Analyzers/Linter/Common/FindPossibleSecretsVisitor.cs @@ -17,6 +17,7 @@ public record PossibleSecret(SyntaxBase Syntax, string FoundMessage) { } private readonly SemanticModel semanticModel; private readonly List possibleSecrets = new(); + private readonly HashSet currentlyProcessing = new(); private uint trailingAccessExpressions = 0; /// @@ -70,18 +71,16 @@ public override void VisitArrayAccessSyntax(ArrayAccessSyntax syntax) private static string PossibleSecretMessage(string possibleSecretName) => string.Format(CoreResources.PossibleSecretMessageSecureParam, possibleSecretName); - private IEnumerable FindPathsToSecureTypeComponents(TypeSymbol type) => FindPathsToSecureTypeComponents(type, "", new()); + private IEnumerable FindPathsToSecureTypeComponents(TypeSymbol type) => FindPathsToSecureTypeComponents(type, ""); - private IEnumerable FindPathsToSecureTypeComponents(TypeSymbol type, string path, HashSet visited) + private IEnumerable FindPathsToSecureTypeComponents(TypeSymbol type, string path) { // types can be recursive. cut out early if we've already seen this type - if (visited.Contains(type)) + if (!currentlyProcessing.Add(type)) { yield break; } - visited.Add(type); - if (type.ValidationFlags.HasFlag(TypeSymbolValidationFlags.IsSecure)) { yield return path; @@ -89,7 +88,7 @@ private IEnumerable FindPathsToSecureTypeComponents(TypeSymbol type, str if (type is UnionType union) { - foreach (var variantPath in union.Members.SelectMany(m => FindPathsToSecureTypeComponents(m.Type, path, visited))) + foreach (var variantPath in union.Members.SelectMany(m => FindPathsToSecureTypeComponents(m.Type, path))) { yield return variantPath; } @@ -118,25 +117,25 @@ private IEnumerable FindPathsToSecureTypeComponents(TypeSymbol type, str case ObjectType obj: if (obj.AdditionalProperties?.TypeReference.Type is TypeSymbol addlPropsType) { - foreach (var dictMemberPath in FindPathsToSecureTypeComponents(addlPropsType, $"{path}.*", visited)) + foreach (var dictMemberPath in FindPathsToSecureTypeComponents(addlPropsType, $"{path}.*")) { yield return dictMemberPath; } } - foreach (var propertyPath in obj.Properties.SelectMany(p => FindPathsToSecureTypeComponents(p.Value.TypeReference.Type, $"{path}.{p.Key}", visited))) + foreach (var propertyPath in obj.Properties.SelectMany(p => FindPathsToSecureTypeComponents(p.Value.TypeReference.Type, $"{path}.{p.Key}"))) { yield return propertyPath; } break; case TupleType tuple: - foreach (var pathFromIndex in tuple.Items.SelectMany((ITypeReference typeAtIndex, int index) => FindPathsToSecureTypeComponents(typeAtIndex.Type, $"{path}[{index}]", visited))) + foreach (var pathFromIndex in tuple.Items.SelectMany((ITypeReference typeAtIndex, int index) => FindPathsToSecureTypeComponents(typeAtIndex.Type, $"{path}[{index}]"))) { yield return pathFromIndex; } break; case ArrayType array: - foreach (var pathFromElement in FindPathsToSecureTypeComponents(array.Item.Type, $"{path}[*]", visited)) + foreach (var pathFromElement in FindPathsToSecureTypeComponents(array.Item.Type, $"{path}[*]")) { yield return pathFromElement; } @@ -144,7 +143,7 @@ private IEnumerable FindPathsToSecureTypeComponents(TypeSymbol type, str } } - visited.Remove(type); + currentlyProcessing.Remove(type); } diff --git a/src/Bicep.Core/Emit/ExpressionConverter.cs b/src/Bicep.Core/Emit/ExpressionConverter.cs index ab7b3d69ab6..a75412cd2c1 100644 --- a/src/Bicep.Core/Emit/ExpressionConverter.cs +++ b/src/Bicep.Core/Emit/ExpressionConverter.cs @@ -448,9 +448,9 @@ LanguageExpression getResourceInfoExpression() case "outputs": var moduleSymbol = reference.Module; - var hasSecureValues = expression.SourceSyntax is null - || FindPossibleSecretsVisitor.FindPossibleSecretsInExpression(context.SemanticModel, expression.SourceSyntax).Any(); - if (context.SemanticModel.Features.SecureOutputsEnabled && hasSecureValues) + if (context.SemanticModel.Features.SecureOutputsEnabled && + (expression.SourceSyntax is null || + FindPossibleSecretsVisitor.FindPossibleSecretsInExpression(context.SemanticModel, expression.SourceSyntax).Any())) { var deploymentResourceId = GetFullyQualifiedResourceId(moduleSymbol, reference.IndexContext?.Index); var apiVersion = new JTokenExpression(EmitConstants.NestedDeploymentResourceApiVersion); diff --git a/src/Bicep.Core/TypeSystem/TypeHelper.cs b/src/Bicep.Core/TypeSystem/TypeHelper.cs index 4705d867aca..a829fe76923 100644 --- a/src/Bicep.Core/TypeSystem/TypeHelper.cs +++ b/src/Bicep.Core/TypeSystem/TypeHelper.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Collections.Concurrent; using System.Collections.Immutable; using System.Diagnostics.CodeAnalysis; using System.Globalization; @@ -693,14 +694,30 @@ private static ObjectType TransformProperties(ObjectType input, Func TransformProperties(input, p => p with { Flags = p.Flags & ~TypePropertyFlags.Required }); - public static TypeSymbol RemovePropertyFlagsRecursively(TypeSymbol type, TypePropertyFlags flagsToRemove) => type switch - { - ObjectType @object => TransformProperties(@object, property => new( + public static TypeSymbol RemovePropertyFlagsRecursively(TypeSymbol type, TypePropertyFlags flagsToRemove) + => RemovePropertyFlagsRecursively(type, flagsToRemove, new()); + + private static TypeSymbol RemovePropertyFlagsRecursively( + TypeSymbol type, + TypePropertyFlags flagsToRemove, + ConcurrentDictionary transformedObjectCache) => type switch + { + ObjectType @object => transformedObjectCache.GetOrAdd( + @object, + obj => RemovePropertyFlagsRecursively(obj, flagsToRemove, transformedObjectCache)), + _ => type, + }; + + private static ObjectType RemovePropertyFlagsRecursively( + ObjectType @object, + TypePropertyFlags flagsToRemove, + ConcurrentDictionary cache) => TransformProperties(@object, property => new( property.Name, - new DeferredTypeReference(() => RemovePropertyFlagsRecursively(property.TypeReference.Type, flagsToRemove)), + new DeferredTypeReference(() => RemovePropertyFlagsRecursively( + property.TypeReference.Type, + flagsToRemove, + cache)), property.Flags & ~flagsToRemove, - property.Description)), - _ => type, - }; + property.Description)); } } diff --git a/src/Bicep.Core/packages.lock.json b/src/Bicep.Core/packages.lock.json index e24dda30af7..4101dc67e18 100644 --- a/src/Bicep.Core/packages.lock.json +++ b/src/Bicep.Core/packages.lock.json @@ -186,9 +186,9 @@ }, "Microsoft.NET.ILLink.Tasks": { "type": "Direct", - "requested": "[8.0.11, )", - "resolved": "8.0.11", - "contentHash": "zk5lnZrYJgtuJG8L4v17Ej8rZ3PUcR2iweNV08BaO5LbYHIi2wNaVNcJoLxvqgQdnjLlKnCCfVGLDr6QHeAarQ==" + "requested": "[8.0.12, )", + "resolved": "8.0.12", + "contentHash": "FV4HnQ3JI15PHnJ5PGTbz+rYvrih42oLi/7UMIshNwCwUZhTq13UzrggtXk4ygrcMcN+4jsS6hhshx2p/Zd0ig==" }, "Microsoft.PowerPlatform.ResourceStack": { "type": "Direct", diff --git a/src/Bicep.Decompiler/packages.lock.json b/src/Bicep.Decompiler/packages.lock.json index b05a0726c21..a1cf9a918bd 100644 --- a/src/Bicep.Decompiler/packages.lock.json +++ b/src/Bicep.Decompiler/packages.lock.json @@ -16,9 +16,9 @@ }, "Microsoft.NET.ILLink.Tasks": { "type": "Direct", - "requested": "[8.0.11, )", - "resolved": "8.0.11", - "contentHash": "zk5lnZrYJgtuJG8L4v17Ej8rZ3PUcR2iweNV08BaO5LbYHIi2wNaVNcJoLxvqgQdnjLlKnCCfVGLDr6QHeAarQ==" + "requested": "[8.0.12, )", + "resolved": "8.0.12", + "contentHash": "FV4HnQ3JI15PHnJ5PGTbz+rYvrih42oLi/7UMIshNwCwUZhTq13UzrggtXk4ygrcMcN+4jsS6hhshx2p/Zd0ig==" }, "Microsoft.SourceLink.GitHub": { "type": "Direct", diff --git a/src/Bicep.IO/packages.lock.json b/src/Bicep.IO/packages.lock.json index 0db07db1868..f91ee507afc 100644 --- a/src/Bicep.IO/packages.lock.json +++ b/src/Bicep.IO/packages.lock.json @@ -16,9 +16,9 @@ }, "Microsoft.NET.ILLink.Tasks": { "type": "Direct", - "requested": "[8.0.11, )", - "resolved": "8.0.11", - "contentHash": "zk5lnZrYJgtuJG8L4v17Ej8rZ3PUcR2iweNV08BaO5LbYHIi2wNaVNcJoLxvqgQdnjLlKnCCfVGLDr6QHeAarQ==" + "requested": "[8.0.12, )", + "resolved": "8.0.12", + "contentHash": "FV4HnQ3JI15PHnJ5PGTbz+rYvrih42oLi/7UMIshNwCwUZhTq13UzrggtXk4ygrcMcN+4jsS6hhshx2p/Zd0ig==" }, "Microsoft.SourceLink.GitHub": { "type": "Direct", diff --git a/src/Bicep.Local.Deploy/packages.lock.json b/src/Bicep.Local.Deploy/packages.lock.json index a30fe869dd9..6b71c2e669e 100644 --- a/src/Bicep.Local.Deploy/packages.lock.json +++ b/src/Bicep.Local.Deploy/packages.lock.json @@ -52,9 +52,9 @@ }, "Microsoft.NET.ILLink.Tasks": { "type": "Direct", - "requested": "[8.0.11, )", - "resolved": "8.0.11", - "contentHash": "zk5lnZrYJgtuJG8L4v17Ej8rZ3PUcR2iweNV08BaO5LbYHIi2wNaVNcJoLxvqgQdnjLlKnCCfVGLDr6QHeAarQ==" + "requested": "[8.0.12, )", + "resolved": "8.0.12", + "contentHash": "FV4HnQ3JI15PHnJ5PGTbz+rYvrih42oLi/7UMIshNwCwUZhTq13UzrggtXk4ygrcMcN+4jsS6hhshx2p/Zd0ig==" }, "Microsoft.SourceLink.GitHub": { "type": "Direct", diff --git a/src/Bicep.Local.Extension.Mock/packages.lock.json b/src/Bicep.Local.Extension.Mock/packages.lock.json index 9cb1f5a45f8..041a678b6e3 100644 --- a/src/Bicep.Local.Extension.Mock/packages.lock.json +++ b/src/Bicep.Local.Extension.Mock/packages.lock.json @@ -21,9 +21,9 @@ }, "Microsoft.NET.ILLink.Tasks": { "type": "Direct", - "requested": "[8.0.11, )", - "resolved": "8.0.11", - "contentHash": "zk5lnZrYJgtuJG8L4v17Ej8rZ3PUcR2iweNV08BaO5LbYHIi2wNaVNcJoLxvqgQdnjLlKnCCfVGLDr6QHeAarQ==" + "requested": "[8.0.12, )", + "resolved": "8.0.12", + "contentHash": "FV4HnQ3JI15PHnJ5PGTbz+rYvrih42oLi/7UMIshNwCwUZhTq13UzrggtXk4ygrcMcN+4jsS6hhshx2p/Zd0ig==" }, "Microsoft.SourceLink.GitHub": { "type": "Direct", diff --git a/src/Bicep.Wasm/packages.lock.json b/src/Bicep.Wasm/packages.lock.json index 72f56ce01be..a43fdea2835 100644 --- a/src/Bicep.Wasm/packages.lock.json +++ b/src/Bicep.Wasm/packages.lock.json @@ -23,9 +23,9 @@ }, "Microsoft.NET.ILLink.Tasks": { "type": "Direct", - "requested": "[8.0.11, )", - "resolved": "8.0.11", - "contentHash": "zk5lnZrYJgtuJG8L4v17Ej8rZ3PUcR2iweNV08BaO5LbYHIi2wNaVNcJoLxvqgQdnjLlKnCCfVGLDr6QHeAarQ==" + "requested": "[8.0.12, )", + "resolved": "8.0.12", + "contentHash": "FV4HnQ3JI15PHnJ5PGTbz+rYvrih42oLi/7UMIshNwCwUZhTq13UzrggtXk4ygrcMcN+4jsS6hhshx2p/Zd0ig==" }, "Microsoft.NET.Sdk.WebAssembly.Pack": { "type": "Direct",