-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Fix OpenApiJsonSchema array parsing (#62051) #62118
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
Conversation
Hi @BrennanConroy ! Considering the main PR was merged last week, will this one be merged soon, too? :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a code refactor related question.
None = 1 | ||
} | ||
|
||
public class EnumArrayTypeConverter : JsonConverter<EnumArrayType> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The EnumArrayTypeConverter
and EnumObjectTypeConverter
classes has been used only in the tests. Can we not make those classes as private
?
public class EnumArrayTypeConverter : JsonConverter<EnumArrayType> | |
private class EnumArrayTypeConverter : JsonConverter<EnumArrayType> |
} | ||
} | ||
|
||
public class EnumObjectTypeConverter : JsonConverter<EnumArrayType> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public class EnumObjectTypeConverter : JsonConverter<EnumArrayType> | |
private class EnumObjectTypeConverter : JsonConverter<EnumArrayType> |
Backport of #62051
Fix OpenApiJsonSchema array parsing
Description
When using the
MapOpenApi()
endpoint with a custom json converter that outputs an array with a default value, the app would crash with a stack overflow. The issue was that we never advanced theUtf8JsonReader
when calling a recursive function. So we'd end up looking at the same json value over and over until stack overflowing.Fixes #62023
Customer Impact
App can crash when accessing the OpenApi endpoint.
Regression?
Risk
Obvious bug once pointed out. Fixed missing test coverage.
Verification
Packaging changes reviewed?