Coming from: #2946746: Unhandled exceptions/fatal errors when POST/PATCH documents contain unknown field names

It looks like this module is not working with the latest security release of JSON API. I haven't looked more into the reasons why. Just wanted to bring it to your attention. Details available in the linked issue.

Patch available to fix this problem.

Comments

gabesullice created an issue. See original summary.

pwolanin’s picture

Just ran into this bug using 8.x-1.15 jsonapi module and 8.x-1.1 of this module

InvalidArgumentException: Field data is unknown. in Drupal\Core\Entity\ContentEntityBase->getTranslatedField() (line 509 of core/lib/Drupal/Core/Entity/ContentEntityBase.php).

Still on core 8.4.6 for the moment

sandboxpl’s picture

After short review, some initials answers are showing up:
1) http://cgit.drupalcode.org/jsonapi_file/tree/src/Normalizer/FileEntityNo...
FileEntityNormalizer tries to pick up data attribute from the request

2) http://cgit.drupalcode.org/drupal/tree/core/lib/Drupal/Core/Entity/Conte...
getTranslatedField() tries to get the definition of field "data" and it fails,
because, of course there no such a field on file entity type.

Possible workaround: add base field definition to file entity type, mark it as computed and call it "data", f.e. with following hook:

/**
 * Implements hook_entity_base_field_info().
 */
function MYMODULE_entity_base_field_info(EntityTypeInterface $entity_type) {

  // Provide custom base field definition for file entity type,
  // to allow jsonapi_file denormalize incoming data.
  // Use string_long type to avoid problems with max length validation.
  // @see \Drupal\Core\Field\Plugin\Field\FieldType\StringLongItem
  if ($entity_type->id() == 'file') {
    $fields = [];
    $fields['data'] = BaseFieldDefinition::create('string_long')
      ->setLabel(t('Data'))
      ->setComputed(TRUE)
      ->setRequired(FALSE)
      ->setTranslatable(FALSE)
      ->setRevisionable(FALSE)
      ->setSetting('case_sensitive', TRUE);

    return $fields;
  }
}

Why computed field?: see https://www.drupal.org/docs/8/api/entity-api/dynamicvirtual-field-values...
I'm not posting a patch atm as this can be easily tested by adding to any *.module file.
This helped me to get rid of exceptions, I've tested this with core 8.4.x and jsonapi 8.x-1.12 / 8.x-1.15

spleshka’s picture

I actually like this idea from #3. I'm being a bit cautious about the potential drawbacks of this implementation, but can't think of any. Other thoughts?

sandboxpl’s picture

I'm testing it since a while and nothing comes up into my mind so far, except the situation where somebody already have a field with machine name "data" defined on file entity, but I believe it's quite simple case and easy to handle. If I'll find anything else I will report it immediately.
In the meantime, here's the original issue which did bring computed properties into the light, there's quite nice explanation about general purpose:
https://www.drupal.org/project/drupal/issues/2392845

jayelless’s picture

I have been investigating this error, and have prepared a patch.

As well as the hook_entity_base_field_info mentioned above, there is a need to update the FileEntityNormaizer to use FileEntity from module file_entity, rather then FileInterface from core, and to update the services definition to match the updates made in the latest release of jsonapi.

However, application of this patch alone is not sufficient, as the JSON response will then be

{
    "errors": [
        {
            "title": "Internal Server Error",
            "status": 500,
            "detail": "The controller result claims to be providing relevant cache metadata, but leaked metadata was detected. Please ensure you are not rendering content too early. Returned object class: Drupal\\jsonapi\\ResourceResponse.",
            "links": {
                "info": "http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.5.1"
            },
            "code": 0
        }
    ]
}

This can be solved by applying the patch #17 from issue #2946746 Unhandled exceptions when POST/PATCH documents contain unknown field names.

jayelless’s picture

Status: Active » Needs review
jayelless’s picture

After prompting from Wim Leers in respect of the patch to jsonapi above, I investigated further and found that the leaked metadata problem was being caused by the denormailzer function setting the file url to an empty string.

After removing this action, the file still validated without problems, and the metadata problem disappeared.

Updated patch attached.

Andreas-2018’s picture

I used patch #8. The upload itself worked.
Still got 403 response. Drupal log showed: warning access denied /de/jsonapi/file/image
I used jsonapi: 1.15 and jsonapi_file 1.1.0. I authenticated the post request with jwt.

jayelless’s picture

@Andreas-2018: Does the user account you authenticated have permission for create the file? I have always used bearer tokens obtained via simple_oauth.

  • Spleshka committed 8ec735e on 8.x-1.x authored by jlscott
    Issue #2946793 by jlscott, gabesullice, sandboxpl, Spleshka, pwolanin:...
spleshka’s picture

Status: Needs review » Fixed

Thank you @jlscott, your patch worked perfectly, all code changes make sense to me. I'll release a new stable version of the module in a few minutes.

@Andreas-2018 I think your issue is related to the authentication problem, not to this particular issue. Feel free to create a support ticket separately or ask community in slack if you'll need any help.

spleshka’s picture

An update - will create a full release a bit later, there are internal reasons to get it slightly postponed. It's in my priority list and will get done ASAP.

  • Spleshka committed 5ecdff7 on 8.x-1.x authored by jlscott
    Issue #2946793 by jlscott, gabesullice, sandboxpl, Spleshka, pwolanin:...
spleshka’s picture

The first commit didn't have all the files from the patch, fixed with the second commit.

lastlink’s picture

@Spleshka how do we pull this update? Was this committed to the dev snapshot? e.g. composer require 'drupal/jsonapi_file:1.x-dev'. The dev snapshot looks like it was updated April 2018. I'm having this same issue. I did just that, but the upload is not persisting to my s3fs based file storage & no longer giving me the field data doesn't exist.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.