Unhandled exceptions/fatal errors when POST/PATCH documents contain unknown field names

Hi @jlscott, thanks for the detailed bug report! I LOVE when we get reports as clearcut as yours :) (also, good for you for staying on top of those SAs so quickly!)

Given that you were previously on a patched version of the module and the error only presents itself when using JSON API File, I thing I need to close this issue as a duplicate of #2883086: [PP-1] Port RequestHandler + ResourceResponseSubscriber improvements from REST module to JSON API. I will make a note of this bug report in that issue. It sounds like that patch just needs to be "rerolled" for the latest JSON API version.

If that turns out not to be the case, then I would recommend filing an issue with JSON API File.

Or... if you think I'm totally crazy and this really is a separate, new and specific issue to the main JSON API module, feel free to reopen this with your reasoning :)

Edit: I've also opened an issue with JSON API File here: #2946793: Broken with latest version of JSON API (1.10)

I have the same problem
I have tested the following patch: https://www.drupal.org/files/issues/2018-03-20/2883086-54-combined.patch comes on the issue: https://www.drupal.org/project/jsonapi/issues/2883086
but it's not work with 8.x-14 version of jsonapi module

I have the same problem
I applied the patch 2883086-54-combined.patch coming from #2883086: Port RequestHandler + ResourceResponse(Interface) improvements from REST module to JSON API but the problem did not solved
I am using Acquia Lightning 3.0.3 release

I have the same problem I've applied the patch
2883086-54-combined.patch
coming from

#2883086: Port RequestHandler + ResourceResponse(Interface) improvements from REST module to JSON API but the problem did not solved
I am using Acquia Lightning 3.0.3 release

Thanks

This is still a bug in jsonapi that happens when data is posted with an attribute that is not a valid entity field

Here's a trivial patch that resolves the 500 error at least.

Thanks @pwolanin! This is a really good catch.

It looks like this is also possible for PATCH as well.

+++ b/src/Controller/EntityResource.php
@@ -194,10 +194,16 @@ class EntityResource {
+        catch (\InvalidArgumentException $e) {
+          // The field doesn't exist on the entity.
+          continue;
         }

I'm not sure that we want to simply ignore the field. I need to look at the spec. @e0ipso, do you have thoughts? Maybe this should be an InvalidEntityException?

Okay, took a quick look at the spec. I see nothing about handling extra fields. My gut reaction is that this needs to be a 422 Unprocessable Entity to be in line with entity validation. We can't say the request was a "success" if we weren't able to save certain attributes. I think that would be in line with this part of the spec:

A request MUST completely succeed or fail (in a single “transaction”). No partial updates are allowed.

Seems to be a long,open debate within jsonapi: https://github.com/json-api/json-api/issues/246

Yeah, a 422 with a nice error code is the desired outcome.

422++ — otherwise incorrect requests are treated as perfectly okay. And if that non-existing field is then added in the future, all clients would suddenly break.

Possible workaround would be to add computed fields to certain entity type,
but I guess this shouldn't be a part of jsonapi module, but rather a project-specific piece of code,
example for jsonapi_file module:
https://www.drupal.org/project/jsonapi_file/issues/2946793#comment-12566772

#16:

Possible workaround would be to add computed fields to certain entity type,

I don't understand how this would solve the problem? You can't add every unknown field name to a certain entity type?

#17: Thanks for the patch, but that's the wrong solution. The problem lies in the jsonapi_file module, not in the jsonapi module.

That's why I also wrote that "I guess this shouldn't be a part of jsonapi module, but rather a project-specific piece of code". When developer knows which data he expects in source, sorry for confusion, looks like my comments shouldn't arrive here, but rather be sticked to jsonapi_file issues

Ok, so #17–#20 were off-topic/out-of-scope, they were about https://www.drupal.org/project/jsonapi_file.

Let's get this issue back on track: the patch in #10, discussion in #11–#15.

Turns out JSON API already had something like this in a few places:

1. \Drupal\jsonapi\Controller\EntityResource::validateReferencedResource() when GETting relationships
2. \Drupal\jsonapi\Controller\EntityResource::updateEntityField() was already detecting non-existent fields, but it was sending a 400 response.

Related: #2942549: Spec Compliance: JSON API allows POSTing relationship fields in 'attributes' rather than in 'relationships'.

And finally: test coverage! The test-only patch is the interdiff.

+++ b/tests/src/Functional/ResourceTestBase.php
@@ -1339,6 +1340,12 @@ abstract class ResourceTestBase extends BrowserTestBase {
+    $this->assertResourceErrorResponse(422, "The attribute field_nonexistent does not exist on the node--camelids resource type.", $response);

Well of course hard-coding node--camelids will not work for all other resource types 🤣

\Drupal\Tests\jsonapi\Functional\JsonApiFunctionalTest::testWrite() contained an assertion for one of the cases that was supported (see #21) but that this patch fixed/made consistent.

Given that:

• JSON API maintainer @e0ipso wrote this in #14:
  

  Yeah, a 422 with a nice error code is the desired outcome.

• based on a proposal by fellow maintainer @gabesullice in #11 + #12
• and that is exactly what this patch is doing…
• … I'm going to go ahead and RTBC this :)

Especially because it's marked "major" in terms of importance/impact for DX, while it's actually a trivial patch.

Made @pwolanin the patch author because it's thanks to him that this is being fixed!