Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: enum parsing when encountering unknown values should not default to first member #2049

Merged
merged 3 commits into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 33 additions & 19 deletions src/Microsoft.OpenApi/Extensions/StringExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,47 +2,61 @@
// Licensed under the MIT license.
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;
using Microsoft.OpenApi.Attributes;
using Microsoft.OpenApi.Models;
using Microsoft.OpenApi.Reader;

namespace Microsoft.OpenApi.Extensions
{
/// <summary>
/// String extension methods.
/// </summary>
public static class StringExtensions
internal static class StringExtensions
{
private static readonly ConcurrentDictionary<Type, ConcurrentDictionary<string, object>> EnumDisplayCache = new();
private static readonly ConcurrentDictionary<Type, ReadOnlyDictionary<string, object>> EnumDisplayCache = new();

/// <summary>
/// Gets the enum value based on the given enum type and display name.
/// </summary>
/// <param name="displayName">The display name.</param>
public static T GetEnumFromDisplayName<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] T>(this string displayName)
internal static bool TryGetEnumFromDisplayName<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] T>(this string displayName, ParsingContext parsingContext, out T result) where T : Enum
{
if (TryGetEnumFromDisplayName(displayName, out result))
{
return true;
}

parsingContext.Diagnostic.Errors.Add(new OpenApiError(parsingContext.GetLocation(), $"Enum value {displayName} is not recognized."));
return false;

}
internal static bool TryGetEnumFromDisplayName<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] T>(this string displayName, out T result) where T : Enum
{
var type = typeof(T);
if (!type.IsEnum)
return default;

var displayMap = EnumDisplayCache.GetOrAdd(type, _ => new ConcurrentDictionary<string, object>(StringComparer.OrdinalIgnoreCase));
var displayMap = EnumDisplayCache.GetOrAdd(type, GetEnumValues<T>);

if (displayMap.TryGetValue(displayName, out var cachedValue))
return (T)cachedValue;

{
result = (T)cachedValue;
return true;
}

foreach (var field in type.GetFields(BindingFlags.Public | BindingFlags.Static))
result = default;
return false;
}
private static ReadOnlyDictionary<string, object> GetEnumValues<T>(Type enumType) where T : Enum
{
var result = new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase);
foreach (var field in enumType.GetFields(BindingFlags.Public | BindingFlags.Static))
{
var displayAttribute = field.GetCustomAttribute<DisplayAttribute>();
if (displayAttribute != null && displayAttribute.Name.Equals(displayName, StringComparison.OrdinalIgnoreCase))
if (field.GetCustomAttribute<DisplayAttribute>() is {} displayAttribute)
{
var enumValue = (T)field.GetValue(null);
displayMap.TryAdd(displayName, enumValue);
return enumValue;
result.Add(displayAttribute.Name, enumValue);
}
}

Check notice

Code scanning / CodeQL

Missed opportunity to use Where Note

This foreach loop
implicitly filters its target sequence
- consider filtering the sequence explicitly using '.Where(...)'.

return default;
return new ReadOnlyDictionary<string, object>(result);
}
internal static string ToFirstCharacterLowerCase(this string input)
=> string.IsNullOrEmpty(input) ? string.Empty : char.ToLowerInvariant(input[0]) + input.Substring(1);
Expand Down
10 changes: 5 additions & 5 deletions src/Microsoft.OpenApi/Models/OpenApiSecurityScheme.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
/// <summary>
/// REQUIRED. The type of the security scheme. Valid values are "apiKey", "http", "oauth2", "openIdConnect".
/// </summary>
public virtual SecuritySchemeType Type { get; set; }
public virtual SecuritySchemeType? Type { get; set; }

/// <summary>
/// A short description for security scheme. CommonMark syntax MAY be used for rich text representation.
Expand All @@ -32,7 +32,7 @@
/// <summary>
/// REQUIRED. The location of the API key. Valid values are "query", "header" or "cookie".
/// </summary>
public virtual ParameterLocation In { get; set; }
public virtual ParameterLocation? In { get; set; }

/// <summary>
/// REQUIRED. The name of the HTTP Authorization scheme to be used
Expand Down Expand Up @@ -82,10 +82,10 @@
/// </summary>
public OpenApiSecurityScheme(OpenApiSecurityScheme securityScheme)
{
Type = securityScheme?.Type ?? Type;
Type = securityScheme?.Type;

Check warning

Code scanning / CodeQL

Virtual call in constructor or destructor Warning

Avoid virtual calls in a constructor or destructor.
Description = securityScheme?.Description ?? Description;
Name = securityScheme?.Name ?? Name;
In = securityScheme?.In ?? In;
In = securityScheme?.In;

Check warning

Code scanning / CodeQL

Virtual call in constructor or destructor Warning

Avoid virtual calls in a constructor or destructor.
Scheme = securityScheme?.Scheme ?? Scheme;
BearerFormat = securityScheme?.BearerFormat ?? BearerFormat;
Flows = securityScheme?.Flows != null ? new(securityScheme?.Flows) : null;
Expand All @@ -111,7 +111,7 @@
SerializeInternal(writer, OpenApiSpecVersion.OpenApi3_0, (writer, element) => element.SerializeAsV3(writer));
}

internal virtual void SerializeInternal(IOpenApiWriter writer, OpenApiSpecVersion version,
internal virtual void SerializeInternal(IOpenApiWriter writer, OpenApiSpecVersion version,
Action<IOpenApiWriter, IOpenApiSerializable> callback)
{
Utils.CheckArgumentNull(writer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public override string Description
public override string Name { get => Target.Name; set => Target.Name = value; }

/// <inheritdoc/>
public override ParameterLocation In { get => Target.In; set => Target.In = value; }
public override ParameterLocation? In { get => Target.In; set => Target.In = value; }

/// <inheritdoc/>
public override string Scheme { get => Target.Scheme; set => Target.Scheme = value; }
Expand All @@ -89,7 +89,7 @@ public override string Description
public override IDictionary<string, IOpenApiExtension> Extensions { get => Target.Extensions; set => Target.Extensions = value; }

/// <inheritdoc/>
public override SecuritySchemeType Type { get => Target.Type; set => Target.Type = value; }
public override SecuritySchemeType? Type { get => Target.Type; set => Target.Type = value; }

/// <inheritdoc/>
public override void SerializeAsV3(IOpenApiWriter writer)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,8 @@ private static void ProcessIn(OpenApiParameter o, ParseNode n, OpenApiDocument h
case "query":
case "header":
case "path":
o.In = value.GetEnumFromDisplayName<ParameterLocation>();
value.TryGetEnumFromDisplayName<ParameterLocation>(out var _in);
o.In = _in;
break;
default:
o.In = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,24 @@
case "oauth2":
o.Type = SecuritySchemeType.OAuth2;
break;

default:
n.Context.Diagnostic.Errors.Add(new OpenApiError(n.Context.GetLocation(), $"Security scheme type {type} is not recognized."));
break;
}
}
},
{"description", (o, n, _) => o.Description = n.GetScalarValue()},
{"name", (o, n, _) => o.Name = n.GetScalarValue()},
{"in", (o, n, _) => o.In = n.GetScalarValue().GetEnumFromDisplayName<ParameterLocation>()},
{"in", (o, n, _) =>
{
if (!n.GetScalarValue().TryGetEnumFromDisplayName<ParameterLocation>(n.Context, out var _in))
{
return;
}
o.In = _in;
}
},
{
"flow", (_, n, _) => _flowValue = n.GetScalarValue()
},
Expand All @@ -71,7 +83,7 @@
public static OpenApiSecurityScheme LoadSecurityScheme(ParseNode node, OpenApiDocument hostDocument = null)
{
// Reset the local variables every time this method is called.
// TODO: Change _flow to a tempStorage variable to make the deserializer thread-safe.

Check warning on line 86 in src/Microsoft.OpenApi/Reader/V2/OpenApiSecuritySchemeDeserializer.cs

View workflow job for this annotation

GitHub Actions / Build

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
_flowValue = null;
_flow = new();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,14 @@ internal static partial class OpenApiV3Deserializer
},
{
"style",
(o, n, _) => o.Style = n.GetScalarValue().GetEnumFromDisplayName<ParameterStyle>()
(o, n, _) =>
{
if(!n.GetScalarValue().TryGetEnumFromDisplayName<ParameterStyle>(n.Context, out var style))
{
return;
}
o.Style = style;
}
},
{
"explode",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,14 @@ internal static partial class OpenApiV3Deserializer
},
{
"style",
(o, n, _) => o.Style = n.GetScalarValue().GetEnumFromDisplayName<ParameterStyle>()
(o, n, _) =>
{
if(!n.GetScalarValue().TryGetEnumFromDisplayName<ParameterStyle>(n.Context, out var style))
{
return;
}
o.Style = style;
}
},
{
"explode",
Expand Down
19 changes: 13 additions & 6 deletions src/Microsoft.OpenApi/Reader/V3/OpenApiParameterDeserializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ internal static partial class OpenApiV3Deserializer
{
"in", (o, n, _) =>
{
var inString = n.GetScalarValue();

o.In = Enum.GetValues(typeof(ParameterLocation)).Cast<ParameterLocation>()
.Select( e => e.GetDisplayName() )
.Contains(inString) ? n.GetScalarValue().GetEnumFromDisplayName<ParameterLocation>() : null;
if (!n.GetScalarValue().TryGetEnumFromDisplayName<ParameterLocation>(n.Context, out var _in))
{
return;
}
o.In = _in;
}
},
{
Expand All @@ -55,7 +55,14 @@ internal static partial class OpenApiV3Deserializer
},
{
"style",
(o, n, _) => o.Style = n.GetScalarValue().GetEnumFromDisplayName<ParameterStyle>()
(o, n, _) =>
{
if (!n.GetScalarValue().TryGetEnumFromDisplayName<ParameterStyle>(n.Context, out var style))
{
return;
}
o.Style = style;
}
},
{
"explode",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// Licensed under the MIT license.

using System;
using System.Linq;
using Microsoft.OpenApi.Extensions;
using Microsoft.OpenApi.Models;
using Microsoft.OpenApi.Models.References;
Expand All @@ -21,7 +20,14 @@ internal static partial class OpenApiV3Deserializer
{
{
"type",
(o, n, _) => o.Type = n.GetScalarValue().GetEnumFromDisplayName<SecuritySchemeType>()
(o, n, _) =>
{
if (!n.GetScalarValue().TryGetEnumFromDisplayName<SecuritySchemeType>(n.Context, out var type))
{
return;
}
o.Type = type;
}
},
{
"description",
Expand All @@ -33,7 +39,14 @@ internal static partial class OpenApiV3Deserializer
},
{
"in",
(o, n, _) => o.In = n.GetScalarValue().GetEnumFromDisplayName<ParameterLocation>()
(o, n, _) =>
{
if(!n.GetScalarValue().TryGetEnumFromDisplayName<ParameterLocation>(n.Context, out var _in))
{
return;
}
o.In = _in;
}
},
{
"scheme",
Expand Down
29 changes: 13 additions & 16 deletions src/Microsoft.OpenApi/Reader/V3/OpenApiV3VersionService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ public OpenApiReference ConvertToOpenApiReference(
if (id.StartsWith("/components/"))
{
var localSegments = segments[1].Split('/');
var referencedType = localSegments[2].GetEnumFromDisplayName<ReferenceType>();
localSegments[2].TryGetEnumFromDisplayName<ReferenceType>(out var referencedType);
if (type == null)
{
type = referencedType;
Expand Down Expand Up @@ -200,25 +200,22 @@ private OpenApiReference ParseLocalReference(string localReference)

var segments = localReference.Split('/');

if (segments.Length == 4) // /components/{type}/pet
if (segments.Length == 4 && segments[1] == "components") // /components/{type}/pet
{
if (segments[1] == "components")
segments[2].TryGetEnumFromDisplayName<ReferenceType>(out var referenceType);
var refId = segments[3];
if (segments[2] == "pathItems")
{
var referenceType = segments[2].GetEnumFromDisplayName<ReferenceType>();
var refId = segments[3];
if (segments[2] == "pathItems")
{
refId = "/" + segments[3];
};
refId = "/" + segments[3];
}

var parsedReference = new OpenApiReference
{
Type = referenceType,
Id = refId
};
var parsedReference = new OpenApiReference
{
Type = referenceType,
Id = refId
};

return parsedReference;
}
return parsedReference;
}

throw new OpenApiException(string.Format(SRResource.ReferenceHasInvalidFormat, localReference));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ internal static partial class OpenApiV31Deserializer
{
"style", (o, n, _) =>
{
o.Style = n.GetScalarValue().GetEnumFromDisplayName<ParameterStyle>();
if(!n.GetScalarValue().TryGetEnumFromDisplayName<ParameterStyle>(n.Context, out var style))
{
return;
}
o.Style = style;
}
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,11 @@ internal static partial class OpenApiV31Deserializer
{
"style", (o, n, _) =>
{
o.Style = n.GetScalarValue().GetEnumFromDisplayName<ParameterStyle>();
if(!n.GetScalarValue().TryGetEnumFromDisplayName<ParameterStyle>(n.Context, out var style))
{
return;
}
o.Style = style;
}
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ internal static partial class OpenApiV31Deserializer
{
"in", (o, n, _) =>
{
var inString = n.GetScalarValue();
o.In = Enum.GetValues(typeof(ParameterLocation)).Cast<ParameterLocation>()
.Select( e => e.GetDisplayName() )
.Contains(inString) ? n.GetScalarValue().GetEnumFromDisplayName<ParameterLocation>() : null;

if (!n.GetScalarValue().TryGetEnumFromDisplayName<ParameterLocation>(n.Context, out var _in))
{
return;
}
o.In = _in;
}
},
{
Expand Down Expand Up @@ -65,7 +65,11 @@ internal static partial class OpenApiV31Deserializer
{
"style", (o, n, _) =>
{
o.Style = n.GetScalarValue().GetEnumFromDisplayName<ParameterStyle>();
if (!n.GetScalarValue().TryGetEnumFromDisplayName<ParameterStyle>(n.Context, out var style))
{
return;
}
o.Style = style;
}
},
{
Expand Down
Loading
Loading