aspnet / JsonPatch

[Archived] JSON PATCH library. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
103 stars 48 forks source link

JsonPatchException: Invalid JsonPatch operation thrown on model binding #80

Closed hjaltijon closed 7 years ago

hjaltijon commented 7 years ago

When the operation is invalid, an exception will be thrown on model binding. { "value": "someValue", "path": "/validPath", "op": "invalidOperationName" }

Why is that? I would have thought that the patch document would become null, with an error message in the modelstate, just like when the json body is invalid? I dont want to return a 500 error to the caller when the operation is invalid...

Is this the intended behavior? If so, is there a workaround?

rynowak commented 7 years ago

An exception/500 here is definitely not intended.

Is your action method getting called? Can you include the call stack that's throwing?

hjaltijon commented 7 years ago

my action method looks like this:

[HttpPatch("users/{username}")]
[Authorize]
public async Task<IActionResult> UpsertUser([FromRoute] string username,
            [FromBody] JsonPatchDocument<UserForUpdateDTO> patchDoc)
{
      if (patchDoc == null) return BadRequest();
      ...

the code inside the action method will not be excecuted if the operation name is not valid, for example with this as the request body:

[
    {
      "value": "someValue",
      "path": "/somePath",
      "op": "invalidOperation"
    }
]

an exception would be thrown.

But with this as the request body:

[
    {
      ThisIsNotValidJson :)
    }
]

the code inside the action method will excecute normally and the caller will get a 400 Bad Request response.

Here is the call stack:

Microsoft.AspNetCore.JsonPatch.Exceptions.JsonPatchException: Invalid JsonPatch operation 'invalidOperationName'.
   at Microsoft.AspNetCore.JsonPatch.Operations.OperationBase.get_OperationType()
   at Microsoft.Extensions.Internal.PropertyHelper.CallNullSafePropertyGetter[TDeclaringType,TValue](Func`2 getter, Object target)
   at Microsoft.AspNetCore.Mvc.Internal.DefaultComplexObjectValidationStrategy.Enumerator.MoveNext()
   at Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidationVisitor.VisitChildren(IValidationStrategy strategy)
   at Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidationVisitor.VisitComplexType()
   at Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidationVisitor.Visit(ModelMetadata metadata, String key, Object model)
   at Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidationVisitor.VisitChildren(IValidationStrategy strategy)
   at Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidationVisitor.VisitEnumerableType()
   at Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidationVisitor.Visit(ModelMetadata metadata, String key, Object model)
   at Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidationVisitor.VisitChildren(IValidationStrategy strategy)
   at Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidationVisitor.VisitComplexType()
   at Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidationVisitor.Visit(ModelMetadata metadata, String key, Object model)
   at Microsoft.AspNetCore.Mvc.Internal.DefaultControllerArgumentBinder.<BindModelAsync>d__8.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.AspNetCore.Mvc.Internal.DefaultControllerArgumentBinder.<BindArgumentsCoreAsync>d__6.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.<InvokeNextResourceFilter>d__22.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.Rethrow(ResourceExecutedContext context)
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.<InvokeAsync>d__20.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.AspNetCore.Builder.RouterMiddleware.<Invoke>d__4.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware`1.<Invoke>d__18.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware`1.<Invoke>d__18.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at IdentityServer4.AccessTokenValidation.IdentityServerAuthenticationMiddleware.<Invoke>d__7.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.<Invoke>d__7.MoveNext()
rynowak commented 7 years ago

/cc @kichalla

IefVandepitte commented 7 years ago

same problem here; My request body: [ { } ] My action method :

[HttpPatch(Name = "WijzigEigenschapVanTelefoonVoorContact")] public IActionResult WijzigEigenschapVanTelefoonVoorContact(int contactId, int id, [FromBody] JsonPatchDocument patchdoc ) {...}

never gets accessed, the server returns a 500 instead.

My callstack: ContactManager-20170529.txt

Eilon commented 7 years ago
  1. We should change the OperationBase.OperationType method to not throw from the get method. This is just bad behavior.
  2. We need to change MVC's default list of "type to not validate" to include the JsonPatchDocument base class.