Merge pull request #2328 from microsoft/fix/security-reports

baywet · web-flow · commit 00721e1e7f28 · 2025-04-16T12:44:27.000-04:00
fix/security reports
diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml
@@ -6,6 +6,7 @@ on:
   pull_request:
   schedule:
     - cron: '0 8 * * *'
+  workflow_dispatch:
 
 permissions:
   contents: read # these permissions are required to run the codeql analysis
diff --git a/src/Microsoft.OpenApi/Expressions/RuntimeExpression.cs b/src/Microsoft.OpenApi/Expressions/RuntimeExpression.cs
@@ -78,23 +78,29 @@ public static RuntimeExpression Build(string expression)
         /// </summary>
         public override int GetHashCode()
         {
-            return Expression.GetHashCode();
+            return StringComparer.Ordinal.GetHashCode(Expression);
         }
 
         /// <summary>
         /// Equals implementation for IEquatable.
         /// </summary>
         public override bool Equals(object? obj)
         {
-            return Equals(obj as RuntimeExpression);
+            if (obj == null)
+            {
+                return false;
+            }
+            if (ReferenceEquals(this, obj))
+            {
+                return true;
+            }
+            return obj.GetType() == GetType() && Equals((RuntimeExpression)obj);
         }
 
-        /// <summary>
-        /// Equals implementation for object of the same type.
-        /// </summary>
-        public bool Equals(RuntimeExpression? obj)
+        /// <inheritdoc />
+        public bool Equals(RuntimeExpression? other)
         {
-            return obj != null && obj.Expression == Expression;
+            return other is not null && StringComparer.Ordinal.Equals(Expression, other.Expression);
         }
 
         /// <inheritdoc />
diff --git a/src/Microsoft.OpenApi/Extensions/OpenApiReferencableExtensions.cs b/src/Microsoft.OpenApi/Extensions/OpenApiReferencableExtensions.cs
@@ -64,10 +64,9 @@ private static IOpenApiReferenceable ResolveReferenceOnHeaderElement(
             if (OpenApiConstants.Examples.Equals(propertyName, StringComparison.Ordinal) &&
                 !string.IsNullOrEmpty(mapKey) &&
                 headerElement?.Examples != null &&
-                headerElement.Examples.TryGetValue(mapKey, out var exampleElement) &&
-                exampleElement is IOpenApiReferenceable referenceable)
+                headerElement.Examples.TryGetValue(mapKey, out var exampleElement))
             {
-                return referenceable;
+                return exampleElement;
             }
             throw new OpenApiException(string.Format(SRResource.InvalidReferenceId, pointer));
         }
@@ -81,10 +80,9 @@ private static IOpenApiReferenceable ResolveReferenceOnParameterElement(
             if (OpenApiConstants.Examples.Equals(propertyName, StringComparison.Ordinal) &&
                 !string.IsNullOrEmpty(mapKey) &&
                 parameterElement?.Examples != null &&
-                parameterElement.Examples.TryGetValue(mapKey, out var exampleElement) &&
-                exampleElement is IOpenApiReferenceable referenceable)
+                parameterElement.Examples.TryGetValue(mapKey, out var exampleElement))
             {
-                return referenceable;
+                return exampleElement;
             }
             throw new OpenApiException(string.Format(SRResource.InvalidReferenceId, pointer));
         }
@@ -99,17 +97,15 @@ private static IOpenApiReferenceable ResolveReferenceOnResponseElement(
             {
                 if (OpenApiConstants.Headers.Equals(propertyName, StringComparison.Ordinal) &&
                     responseElement?.Headers != null &&
-                    responseElement.Headers.TryGetValue(mapKey, out var headerElement) &&
-                    headerElement is IOpenApiReferenceable referenceable)
+                    responseElement.Headers.TryGetValue(mapKey, out var headerElement))
                 {
-                    return referenceable;
+                    return headerElement;
                 }
                 if (OpenApiConstants.Links.Equals(propertyName, StringComparison.Ordinal) &&
                     responseElement?.Links != null &&
-                    responseElement.Links.TryGetValue(mapKey, out var linkElement) &&
-                    linkElement is IOpenApiReferenceable referenceable2)
+                    responseElement.Links.TryGetValue(mapKey, out var linkElement))
                 {
-                    return referenceable2;
+                    return linkElement;
                 }
             }
             throw new OpenApiException(string.Format(SRResource.InvalidReferenceId, pointer));
diff --git a/src/Microsoft.OpenApi/Models/OpenApiDocument.cs b/src/Microsoft.OpenApi/Models/OpenApiDocument.cs
@@ -296,15 +296,12 @@ public void SerializeAsV2(IOpenApiWriter writer)
                         .OfType<OpenApiSchemaReference>()
                         .Where(k => k.Reference?.Id is not null)
                         .ToDictionary<OpenApiSchemaReference, string, IOpenApiSchema>(
-                            k => k.Reference?.Id!,
+                            k => k.Reference.Id!,
                             v => v
                         );
 
 
-                    foreach (var schema in openApiSchemas.Values.ToList())
-                    {
-                        FindSchemaReferences.ResolveSchemas(Components, openApiSchemas!);
-                    }
+                    FindSchemaReferences.ResolveSchemas(Components, openApiSchemas);
 
                     writer.WriteOptionalMap(
                        OpenApiConstants.Definitions,
@@ -723,8 +720,10 @@ internal class FindSchemaReferences : OpenApiVisitorBase
 
         public static void ResolveSchemas(OpenApiComponents? components, Dictionary<string, IOpenApiSchema> schemas)
         {
-            var visitor = new FindSchemaReferences();
-            visitor.Schemas = schemas;
+            var visitor = new FindSchemaReferences
+            {
+                Schemas = schemas
+            };
             var walker = new OpenApiWalker(visitor);
             walker.Walk(components);
         }
diff --git a/src/Microsoft.OpenApi/Writers/OpenApiWriterAnyExtensions.cs b/src/Microsoft.OpenApi/Writers/OpenApiWriterAnyExtensions.cs
@@ -114,7 +114,7 @@ private static void WriteObject(this IOpenApiWriter writer, JsonObject? entity)
 
         private static void WritePrimitive(this IOpenApiWriter writer, JsonValue jsonValue)
         {
-            if (jsonValue.TryGetValue(out string? stringValue))
+            if (jsonValue.TryGetValue(out string? stringValue) && stringValue is not null)
                 writer.WriteValue(stringValue);
             else if (jsonValue.TryGetValue(out DateTime dateTimeValue))
                 writer.WriteValue(dateTimeValue.ToString("o", CultureInfo.InvariantCulture)); // ISO 8601 format
diff --git a/src/Microsoft.OpenApi/Writers/OpenApiWriterBase.cs b/src/Microsoft.OpenApi/Writers/OpenApiWriterBase.cs
@@ -238,55 +238,53 @@ public virtual void WriteValue(object? value)
                 return;
             }
 
-            var type = value.GetType();
-
-            if (type == typeof(string))
+            if (value is string strValue)
             {
-                WriteValue((string)(value));
+                WriteValue(strValue);
             }
-            else if (type == typeof(int) || type == typeof(int?))
+            else if (value is int intValue)
             {
-                WriteValue((int)value);
+                WriteValue(intValue);
             }
-            else if (type == typeof(uint) || type == typeof(uint?))
+            else if (value is uint uintValue)
             {
-                WriteValue((uint)value);
+                WriteValue(uintValue);
             }
-            else if (type == typeof(long) || type == typeof(long?))
+            else if (value is long longValue)
             {
-                WriteValue((long)value);
+                WriteValue(longValue);
             }
-            else if (type == typeof(bool) || type == typeof(bool?))
+            else if (value is bool boolValue)
             {
-                WriteValue((bool)value);
+                WriteValue(boolValue);
             }
-            else if (type == typeof(float) || type == typeof(float?))
+            else if (value is float floatValue)
             {
-                WriteValue((float)value);
+                WriteValue(floatValue);
             }
-            else if (type == typeof(double) || type == typeof(double?))
+            else if (value is double doubleValue)
             {
-                WriteValue((double)value);
+                WriteValue(doubleValue);
             }
-            else if (type == typeof(decimal) || type == typeof(decimal?))
+            else if (value is decimal decimalValue)
             {
-                WriteValue((decimal)value);
+                WriteValue(decimalValue);
             }
-            else if (type == typeof(DateTime) || type == typeof(DateTime?))
+            else if (value is DateTime DateTimeValue)
             {
-                WriteValue((DateTime)value);
+                WriteValue(DateTimeValue);
             }
-            else if (type == typeof(DateTimeOffset) || type == typeof(DateTimeOffset?))
+            else if (value is DateTimeOffset DateTimeOffsetValue)
             {
-                WriteValue((DateTimeOffset)value);
+                WriteValue(DateTimeOffsetValue);
             }
             else if (value is IEnumerable<object> enumerable)
             {
                 WriteEnumerable(enumerable);
             }
             else
             {
-                throw new OpenApiWriterException(string.Format(SRResource.OpenApiUnsupportedValueType, type.FullName));
+                throw new OpenApiWriterException(string.Format(SRResource.OpenApiUnsupportedValueType, value.GetType().FullName));
             }
         }
 
diff --git a/src/Microsoft.OpenApi/Writers/OpenApiYamlWriter.cs b/src/Microsoft.OpenApi/Writers/OpenApiYamlWriter.cs
@@ -170,7 +170,7 @@ public override void WritePropertyName(string name)
         /// <param name="value">The string value.</param>
         public override void WriteValue(string value)
         {
-            if (!UseLiteralStyle || value?.IndexOfAny(new[] { '\n', '\r' }) == -1)
+            if (!UseLiteralStyle || value.IndexOfAny(['\n', '\r']) == -1)
             {
                 WriteValueSeparator();
 
@@ -190,7 +190,7 @@ public override void WriteValue(string value)
                 WriteChompingIndicator(value);
 
                 // Write indentation indicator when it starts with spaces
-                if (value is not null && value.StartsWith(" ", StringComparison.OrdinalIgnoreCase))
+                if (value[0] == ' ')
                 {
                     Writer.Write(IndentationString.Length);
                 }
diff --git a/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt b/test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt
@@ -115,7 +115,7 @@ namespace Microsoft.OpenApi.Expressions
         public const string Prefix = "$";
         protected RuntimeExpression() { }
         public abstract string Expression { get; }
-        public bool Equals(Microsoft.OpenApi.Expressions.RuntimeExpression? obj) { }
+        public bool Equals(Microsoft.OpenApi.Expressions.RuntimeExpression? other) { }
         public override bool Equals(object? obj) { }
         public override int GetHashCode() { }
         public override string ToString() { }

Original file line number	Diff line number	Diff line change
`@@ -78,23 +78,29 @@ public static RuntimeExpression Build(string expression)`
`78`	`78`	`/// </summary>`
`79`	`79`	`public override int GetHashCode()`
`80`	`80`	`{`
`81`		`- return Expression.GetHashCode();`
	`81`	`+ return StringComparer.Ordinal.GetHashCode(Expression);`
`82`	`82`	`}`
`83`	`83`
`84`	`84`	`/// <summary>`
`85`	`85`	`/// Equals implementation for IEquatable.`
`86`	`86`	`/// </summary>`
`87`	`87`	`public override bool Equals(object? obj)`
`88`	`88`	`{`
`89`		`- return Equals(obj as RuntimeExpression);`
	`89`	`+ if (obj == null)`
	`90`	`+ {`
	`91`	`+ return false;`
	`92`	`+ }`
	`93`	`+ if (ReferenceEquals(this, obj))`
	`94`	`+ {`
	`95`	`+ return true;`
	`96`	`+ }`
	`97`	`+ return obj.GetType() == GetType() && Equals((RuntimeExpression)obj);`
`90`	`98`	`}`
`91`	`99`
`92`		`- /// <summary>`
`93`		`- /// Equals implementation for object of the same type.`
`94`		`- /// </summary>`
`95`		`- public bool Equals(RuntimeExpression? obj)`
	`100`	`+ /// <inheritdoc />`
	`101`	`+ public bool Equals(RuntimeExpression? other)`
`96`	`102`	`{`
`97`		`- return obj != null && obj.Expression == Expression;`
	`103`	`+ return other is not null && StringComparer.Ordinal.Equals(Expression, other.Expression);`
`98`	`104`	`}`
`99`	`105`
`100`	`106`	`/// <inheritdoc />`
Original file line number	Diff line number	Diff line change
`@@ -64,10 +64,9 @@ private static IOpenApiReferenceable ResolveReferenceOnHeaderElement(`
`64`	`64`	`if (OpenApiConstants.Examples.Equals(propertyName, StringComparison.Ordinal) &&`
`65`	`65`	`!string.IsNullOrEmpty(mapKey) &&`
`66`	`66`	`headerElement?.Examples != null &&`
`67`		`- headerElement.Examples.TryGetValue(mapKey, out var exampleElement) &&`
`68`		`- exampleElement is IOpenApiReferenceable referenceable)`
	`67`	`+ headerElement.Examples.TryGetValue(mapKey, out var exampleElement))`
`69`	`68`	`{`
`70`		`- return referenceable;`
	`69`	`+ return exampleElement;`
`71`	`70`	`}`
`72`	`71`	`throw new OpenApiException(string.Format(SRResource.InvalidReferenceId, pointer));`
`73`	`72`	`}`
`@@ -81,10 +80,9 @@ private static IOpenApiReferenceable ResolveReferenceOnParameterElement(`
`81`	`80`	`if (OpenApiConstants.Examples.Equals(propertyName, StringComparison.Ordinal) &&`
`82`	`81`	`!string.IsNullOrEmpty(mapKey) &&`
`83`	`82`	`parameterElement?.Examples != null &&`
`84`		`- parameterElement.Examples.TryGetValue(mapKey, out var exampleElement) &&`
`85`		`- exampleElement is IOpenApiReferenceable referenceable)`
	`83`	`+ parameterElement.Examples.TryGetValue(mapKey, out var exampleElement))`
`86`	`84`	`{`
`87`		`- return referenceable;`
	`85`	`+ return exampleElement;`
`88`	`86`	`}`
`89`	`87`	`throw new OpenApiException(string.Format(SRResource.InvalidReferenceId, pointer));`
`90`	`88`	`}`
`@@ -99,17 +97,15 @@ private static IOpenApiReferenceable ResolveReferenceOnResponseElement(`
`99`	`97`	`{`
`100`	`98`	`if (OpenApiConstants.Headers.Equals(propertyName, StringComparison.Ordinal) &&`
`101`	`99`	`responseElement?.Headers != null &&`
`102`		`- responseElement.Headers.TryGetValue(mapKey, out var headerElement) &&`
`103`		`- headerElement is IOpenApiReferenceable referenceable)`
	`100`	`+ responseElement.Headers.TryGetValue(mapKey, out var headerElement))`
`104`	`101`	`{`
`105`		`- return referenceable;`
	`102`	`+ return headerElement;`
`106`	`103`	`}`
`107`	`104`	`if (OpenApiConstants.Links.Equals(propertyName, StringComparison.Ordinal) &&`
`108`	`105`	`responseElement?.Links != null &&`
`109`		`- responseElement.Links.TryGetValue(mapKey, out var linkElement) &&`
`110`		`- linkElement is IOpenApiReferenceable referenceable2)`
	`106`	`+ responseElement.Links.TryGetValue(mapKey, out var linkElement))`
`111`	`107`	`{`
`112`		`- return referenceable2;`
	`108`	`+ return linkElement;`
`113`	`109`	`}`
`114`	`110`	`}`
`115`	`111`	`throw new OpenApiException(string.Format(SRResource.InvalidReferenceId, pointer));`
Original file line number	Diff line number	Diff line change
`@@ -114,7 +114,7 @@ private static void WriteObject(this IOpenApiWriter writer, JsonObject? entity)`
`114`	`114`
`115`	`115`	`private static void WritePrimitive(this IOpenApiWriter writer, JsonValue jsonValue)`
`116`	`116`	`{`
`117`		`- if (jsonValue.TryGetValue(out string? stringValue))`
	`117`	`+ if (jsonValue.TryGetValue(out string? stringValue) && stringValue is not null)`
`118`	`118`	`writer.WriteValue(stringValue);`
`119`	`119`	`else if (jsonValue.TryGetValue(out DateTime dateTimeValue))`
`120`	`120`	`writer.WriteValue(dateTimeValue.ToString("o", CultureInfo.InvariantCulture)); // ISO 8601 format`
Original file line number	Diff line number	Diff line change
`@@ -238,55 +238,53 @@ public virtual void WriteValue(object? value)`
`238`	`238`	`return;`
`239`	`239`	`}`
`240`	`240`
`241`		`- var type = value.GetType();`
`242`		`-`
`243`		`- if (type == typeof(string))`
	`241`	`+ if (value is string strValue)`
`244`	`242`	`{`
`245`		`- WriteValue((string)(value));`
	`243`	`+ WriteValue(strValue);`
`246`	`244`	`}`
`247`		`- else if (type == typeof(int) \|\| type == typeof(int?))`
	`245`	`+ else if (value is int intValue)`
`248`	`246`	`{`
`249`		`- WriteValue((int)value);`
	`247`	`+ WriteValue(intValue);`
`250`	`248`	`}`
`251`		`- else if (type == typeof(uint) \|\| type == typeof(uint?))`
	`249`	`+ else if (value is uint uintValue)`
`252`	`250`	`{`
`253`		`- WriteValue((uint)value);`
	`251`	`+ WriteValue(uintValue);`
`254`	`252`	`}`
`255`		`- else if (type == typeof(long) \|\| type == typeof(long?))`
	`253`	`+ else if (value is long longValue)`
`256`	`254`	`{`
`257`		`- WriteValue((long)value);`
	`255`	`+ WriteValue(longValue);`
`258`	`256`	`}`
`259`		`- else if (type == typeof(bool) \|\| type == typeof(bool?))`
	`257`	`+ else if (value is bool boolValue)`
`260`	`258`	`{`
`261`		`- WriteValue((bool)value);`
	`259`	`+ WriteValue(boolValue);`
`262`	`260`	`}`
`263`		`- else if (type == typeof(float) \|\| type == typeof(float?))`
	`261`	`+ else if (value is float floatValue)`
`264`	`262`	`{`
`265`		`- WriteValue((float)value);`
	`263`	`+ WriteValue(floatValue);`
`266`	`264`	`}`
`267`		`- else if (type == typeof(double) \|\| type == typeof(double?))`
	`265`	`+ else if (value is double doubleValue)`
`268`	`266`	`{`
`269`		`- WriteValue((double)value);`
	`267`	`+ WriteValue(doubleValue);`
`270`	`268`	`}`
`271`		`- else if (type == typeof(decimal) \|\| type == typeof(decimal?))`
	`269`	`+ else if (value is decimal decimalValue)`
`272`	`270`	`{`
`273`		`- WriteValue((decimal)value);`
	`271`	`+ WriteValue(decimalValue);`
`274`	`272`	`}`
`275`		`- else if (type == typeof(DateTime) \|\| type == typeof(DateTime?))`
	`273`	`+ else if (value is DateTime DateTimeValue)`
`276`	`274`	`{`
`277`		`- WriteValue((DateTime)value);`
	`275`	`+ WriteValue(DateTimeValue);`
`278`	`276`	`}`
`279`		`- else if (type == typeof(DateTimeOffset) \|\| type == typeof(DateTimeOffset?))`
	`277`	`+ else if (value is DateTimeOffset DateTimeOffsetValue)`
`280`	`278`	`{`
`281`		`- WriteValue((DateTimeOffset)value);`
	`279`	`+ WriteValue(DateTimeOffsetValue);`
`282`	`280`	`}`
`283`	`281`	`else if (value is IEnumerable<object> enumerable)`
`284`	`282`	`{`
`285`	`283`	`WriteEnumerable(enumerable);`
`286`	`284`	`}`
`287`	`285`	`else`
`288`	`286`	`{`
`289`		`- throw new OpenApiWriterException(string.Format(SRResource.OpenApiUnsupportedValueType, type.FullName));`
	`287`	`+ throw new OpenApiWriterException(string.Format(SRResource.OpenApiUnsupportedValueType, value.GetType().FullName));`
`290`	`288`	`}`
`291`	`289`	`}`
`292`	`290`
Original file line number	Diff line number	Diff line change
`@@ -170,7 +170,7 @@ public override void WritePropertyName(string name)`
`170`	`170`	`/// <param name="value">The string value.</param>`
`171`	`171`	`public override void WriteValue(string value)`
`172`	`172`	`{`
`173`		`- if (!UseLiteralStyle \|\| value?.IndexOfAny(new[] { '\n', '\r' }) == -1)`
	`173`	`+ if (!UseLiteralStyle \|\| value.IndexOfAny(['\n', '\r']) == -1)`
`174`	`174`	`{`
`175`	`175`	`WriteValueSeparator();`
`176`	`176`
`@@ -190,7 +190,7 @@ public override void WriteValue(string value)`
`190`	`190`	`WriteChompingIndicator(value);`
`191`	`191`
`192`	`192`	`// Write indentation indicator when it starts with spaces`
`193`		`- if (value is not null && value.StartsWith(" ", StringComparison.OrdinalIgnoreCase))`
	`193`	`+ if (value[0] == ' ')`
`194`	`194`	`{`
`195`	`195`	`Writer.Write(IndentationString.Length);`
`196`	`196`	`}`