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

Web API error response for malformed json using Text.Json is misleading and is exposing internals. #40202

Closed
1 task done
KillerBoogie opened this issue Feb 14, 2022 · 7 comments
Assignees
Labels
feature-model-binding Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels Status: No Recent Activity

Comments

@KillerBoogie
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

When a malformed Json is sent to a route the default serializer Text.Json creates a misleadig error message and exposes internal information. For example a malformed Guid or DateTime produces an error like:

"$.accountId": [
          "The JSON value could not be converted to Elwis.Orders.Application.CreateGroupDTO.
          Path: $.accountId | LineNumber: 1 | BytePositionInLine: 52."
        ]

Additionally, the variable name of the DTO is exposed and considered a field:

"createGroupDTO": [
          "The createGroupDTO field is required."
        ],

Expected Behavior

The error message for a malformed value must state that type only:
"The JSON value could not be converted to "Guid" or
"The JSON value could not be converted to "DateTime"

The name of the body variable should not be exposed in the error message. Only JSON values that can not be converted should be listed in the message.

Steps To Reproduce

Define DTO:

public record CreateGroupDTO(
            Guid AccountId,
            DateTime Start 
           );

Create method in GroupsController

[HttpPost]
[Route("")]
public ActionResult<GroupDTO> CreateGroup(CreateGroupDTO createGroupDTO) {...}

Post with malformed Json body

1) Guid

{
  "accountId": "4-5717-4562-b3fc-2c963f66afa6",
  "start": "2022-02-14T12:39:59.244Z"
}

response body:

{
  "type": "https://tools.ietf.org/html/rfc7231#section-6.5.1",
  "title": "One or more validation errors occurred.",
  "status": 400,
  "traceId": "00-44f0bdb2706fd407dbfa1282f04b66e1-5e64394e46fa6152-00",
  "errors": {
    "createGroupDTO": [
      "The createGroupDTO field is required."
    ],
    "$.accountId": [
      "The JSON value could not be converted to Elwis.Orders.Application.CreateGroupDTO. Path: $.accountId | LineNumber: 1 | BytePositionInLine: 46."
    ]
  }
}

2) DateTime

{
  "accountId": "3fa85f64-5717-4562-b3fc-2c963f66afa6",
  "start": "2022-02-30"
}

response body:

{
  "type": "https://tools.ietf.org/html/rfc7231#section-6.5.1",
  "title": "One or more validation errors occurred.",
  "status": 400,
  "traceId": "00-8f49393621db7dea28f797f4ee69f203-2612b8bd5fe0aa0d-00",
  "errors": {
    "createGroupDTO": [
      "The createGroupDTO field is required."
    ],
    "$.start": [
      "The JSON value could not be converted to Elwis.Orders.Application.CreateGroupDTO. Path: $.start | LineNumber: 2 | BytePositionInLine: 23."
    ]
  }
}

Exceptions (if any)

No response

.NET Version

6.0.101

Anything else?

No response

@javiercn javiercn added the old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Feb 14, 2022
@pranavkm
Copy link
Contributor

@pranavkm pranavkm added the ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. label Feb 14, 2022
@ghost ghost added the Status: Resolved label Feb 14, 2022
@KillerBoogie
Copy link
Author

Thank you for this link. This is much better. I'm getting now the error:

{
  "type": "https://tools.ietf.org/html/rfc7231#section-6.5.1",
  "title": "One or more validation errors occurred.",
  "status": 400,
  "traceId": "00-a5036b75970685554b4cf7c87c452939-ca3293d1faef0d84-00",
  "errors": {
    "createGroupDTO": [
      "The createGroupDTO field is required."
    ],
    "$.accountId": [
      "The input was not valid."
    ]
  }
}

Is there also a way to hide the variable name of the body parameter and its error?

[HttpPost]
[Route("")]
public ActionResult<GroupDTO> CreateGroup(CreateGroupDTO createGroupDTO) {...}

It still states that "The createGroupDTO field is required."! This response makes no sense since it is not a field in the Json structure and just an internal name.

@pranavkm pranavkm removed ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved labels Feb 14, 2022
@rafikiassumani-msft
Copy link
Contributor

@brunolins16 Can you investigate this issue? You may have some more context based on your recent experience fixing a similar issue.

@rafikiassumani-msft rafikiassumani-msft added this to the .NET 7 Planning milestone Feb 17, 2022
@ghost
Copy link

ghost commented Feb 17, 2022

Thanks for contacting us.
We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@brunolins16
Copy link
Member

@KillerBoogie sorry for the long delay in responding. Probably you have already found a workaround for your problem :(.

Anyway, quick looking into your example I don't see any option to hide the parameter name but I think about two options:

  1. Probably that is not what you want but the message came from the RequiredAttribute implicited added because the parameter is not nullable, if your action parameter is optional, you could change it to CreateGroupDTO? createGroupDTO
  2. You explicitly add the RequiredAttribute you can specify the ErrorMessage that you desire. This option will change the message, however the key will keep the same:
[Required(ErrorMessage = "Input required")]CreateGroupDTO createGroupDTO
       "createGroupDTO": [
            "Input required"
        ],

@brunolins16 brunolins16 added Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. and removed investigate labels Aug 29, 2022
@ghost
Copy link

ghost commented Aug 29, 2022

Hi @KillerBoogie. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

@ghost
Copy link

ghost commented Sep 2, 2022

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

See our Issue Management Policies for more information.

@ghost ghost closed this as completed Sep 5, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Oct 5, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-model-binding Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels Status: No Recent Activity
Projects
None yet
Development

No branches or pull requests

5 participants