Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

ModelState BadRequest inconsistent behaviour #5672

Closed
adnan-kamili opened this issue Jan 10, 2017 · 6 comments
Closed

ModelState BadRequest inconsistent behaviour #5672

adnan-kamili opened this issue Jan 10, 2017 · 6 comments

Comments

@adnan-kamili
Copy link

adnan-kamili commented Jan 10, 2017

I have a simple model:

public class Item : Entity<int>
{
    [Required]
    public string Name { get; set; }
    [Required]
    public int Cost { get; set; }
}

Inside controller:

[HttpPost]
public async Task<IActionResult> Create([FromBody] Item item)
{
    if(!ModelState.IsValid) 
    {
       return BadRequest(ModelState);
    }
    repository.Create(item);
    await repository.SaveAsync();
    return Created($"/api/v1/items/{item.Id}", new { message = "Item was created successfully!" });
}

Now, for following three incorrect sample inputs I get following responses:

Sample #1:

POST http://localhost:5000/api/v1/items
content-type: application/json

{
    "name": "sample1",
    "cost": $100000,
}

Response:

{
  "cost": [
    "The input was not valid."
  ]
}

Sample #2:

POST http://localhost:5000/api/v1/items
content-type: application/json

{
    "name": "sample2",
    "cost": "10000000000000000000000000",
}

Response:

{
  "cost": [
    "The input was not valid."
  ]
}

Sample #3: This one seems like a bug. An empty key appears (instead of int out of range error)

POST http://localhost:5000/api/v1/items
content-type: application/json

{
    "name": "sample3",
    "cost": 10000000000000000000000000",
}

Response:

{
  "": [
    "The input was not valid."
  ],
  "cost": [
    "The input was not valid."
  ]
}
@Eilon
Copy link
Member

Eilon commented Jan 11, 2017

This appears to just be how Json.NET is reporting the invalid JSON syntax to MVC's formatter.

In general it can be difficult to predict how invalid syntax is interpreted because it can be ambiguous. I'm guessing that the extra " at the end of the value of "cost" is causing the JSON parser to think it's the start of another key (which has no name).

@JamesNK any thoughts on this?

@JamesNK
Copy link
Member

JamesNK commented Jan 11, 2017

Json.NET serializes what you give it. If you give it a dictionary with an empty string key then that is what it will serialize.

@Eilon
Copy link
Member

Eilon commented Jan 12, 2017

@JamesNK this is about deserialization error behavior for invalid JSON.

@Eilon
Copy link
Member

Eilon commented Jan 12, 2017

E.g. this JSON:

{
    "name": "sample3",
    "cost": 10000000000000000000000000",
}

Is clearly invalid for at least three reasons:

  1. The number is presumably too long.
  2. There's an extra quote at the end of the long 1000000000" number
  3. There's an erroneous trailing comma at the end of the "cost" field

@adnan-kamili
Copy link
Author

adnan-kamili commented Jan 12, 2017

@Eilon @JamesNK
The issue replicates even if there is no quote or comma at the end (a valid JSON):

Sample 4: This one seems like a bug. An empty key appears (instead of int out of range error)

POST http://localhost:5000/api/v1/items
content-type: application/json

{
    "name": "sample4",
    "cost": 10000000000000000000000000
}

Response:

{
  "": [
    "The input was not valid."
  ],
  "cost": [
    "The input was not valid."
  ]
}

The exception logger, logs following error in console:



   Newtonsoft.Json.JsonSerializationException: Unexpected end when deserializing object. Path '', line 4, position 1.
   at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.ThrowUnexpectedEndException(JsonReader reader, JsonContract contract, Object currentObject,
 String message)
Newtonsoft.Json.JsonReaderException: JSON integer 10000000000000000000000000000000000000 is too large or small for an Int32. Path 'cost', line 3, position 50.

@Eilon
Copy link
Member

Eilon commented Jan 20, 2017

Hi, we discussed this and we have an idea that would help mitigate this somewhat. See this issue for more details: #5705

But for the specific issue here, ultimately ASP.NET Core is just processing the errors returned by Json.NET and it would be risky to change the behavior in how the errors are processed, so we are closing this issue.

@Eilon Eilon closed this as completed Jan 20, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants