Skip to content

fix: required TypeEnum deserializer should reject null values#1798

Open
TabishRiazBajwa wants to merge 2 commits into
Adyen:mainfrom
TabishRiazBajwa:fix/ienum-required-typeenum-nullable-contract
Open

fix: required TypeEnum deserializer should reject null values#1798
TabishRiazBajwa wants to merge 2 commits into
Adyen:mainfrom
TabishRiazBajwa:fix/ienum-required-typeenum-nullable-contract

Conversation

@TabishRiazBajwa

Copy link
Copy Markdown

Fixes #1678

Read() in the generated TypeEnum JsonConverter silently accepts null during deserialization even when the field is marked as required/non-nullable. This violates the non-nullable contract — a required enum field should never deserialize to null without throwing.

Description
The generated TypeEnumJsonConverter.Read() returns null when the JSON value is null, regardless of whether the property is required. This means a required TypeEnum field can silently become null at runtime, bypassing the non-nullable contract and causing unexpected null reference exceptions downstream rather than at the deserialization boundary where the error actually is.

Fixed by checking for null in the Read() method of the generated converter and throwing a JsonException when a null value is encountered for a required field.

Tested scenarios

  • Required TypeEnum field throws JsonException when JSON contains null
  • Optional TypeEnum field continues to deserialize null correctly
  • All existing IEnum and TypeEnum deserialization tests continue to pass

Fixed issue: #1678

Fixes Adyen#1678

`Read()` in the generated `TypeEnum` JsonConverter silently accepts `null` during deserialization even when the field is marked as required/non-nullable. This violates the non-nullable contract — a required enum field should never deserialize to null without throwing.

**Description**
The generated `TypeEnumJsonConverter.Read()` returns `null` when the JSON value is `null`, regardless of whether the property is required. This means a required `TypeEnum` field can silently become `null` at runtime, bypassing the non-nullable contract and causing unexpected null reference exceptions downstream rather than at the deserialization boundary where the error actually is.

Fixed by checking for null in the `Read()` method of the generated converter and throwing a `JsonException` when a null value is encountered for a required field.

**Tested scenarios**
- Required TypeEnum field throws `JsonException` when JSON contains null
- Optional TypeEnum field continues to deserialize null correctly
- All existing IEnum and TypeEnum deserialization tests continue to pass

**Fixed issue**: Adyen#1678
@TabishRiazBajwa TabishRiazBajwa requested a review from a team as a code owner June 29, 2026 10:55

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the C# templates to prevent null values for required, non-nullable string-based enums during JSON deserialization, throwing a JsonException if encountered, and adds corresponding unit tests. The feedback recommends updating the signature of FromStringOrDefault to accept a nullable string to prevent compiler warnings when Nullable Reference Types are enabled. Additionally, it is suggested to extend this null-checking logic to required, non-nullable numeric enums for completeness.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread templates-v7/csharp/libraries/generichost/modelInnerEnum.mustache
Comment thread templates-v7/csharp/libraries/generichost/JsonConverter.mustache
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Required TypeEnum deserializer silently accepts null, violating non-nullable contract

1 participant