patch tries to mimic entity_property_values_create_entity()'s logic to only set known properties, otherwise assign them directly to the entity.

This happened to me while using backbone, where I have non-metadata properties being sent to restws.

Comments

Status: Needs review » Needs work

The last submitted patch, restws-dont-set-non-property.patch, failed testing.

klausi’s picture

Hm, that would be an API change. Currently we explicitly require that the properties of an entity are known to not allow arbitrary data (a kind of safe guard). This is also some kind of validation so that the sender can be sure that the data matches what Drupal can process.

Why can't you use hook_entity_property_info() or hook_entity_property_info_alter() to register your custom properties?

amitaibu’s picture

Why can't you use hook_entity_property_info() or hook_entity_property_info_alter() to register your custom properties?

Those are properties sent from Backbone, which are needed only for the JS application to work, but have no meaning on the entity (e.g. I have a "selected" state on a photo indicating if the user clicked on it).

klausi’s picture

So if we want to support use cases where it is allowed to pass unknown properties we also need to change the create operation.
What are the implications of this? Is it a security issue if we allow random data properties on our entity objects? We do have a permission for each resource/entity, so we might just acknowledge that users with that permission can do harmful stuff anyway.

amitaibu’s picture

> we also need to change the create operation

entity_property_values_create_entity() is already doing the same thing, so we actually going to fix update to work as create.

klausi’s picture

The create method does some extra checks after entity_property_values_create_entity(), so we would have to remove that.

amitaibu’s picture

Status: Needs work » Needs review
StatusFileSize
new2.22 KB

Something like this?

amitaibu’s picture

We do have a permission for each resource/entity, so we might just acknowledge that users with that permission can do harmful stuff anyway

I agree

Status: Needs review » Needs work

The last submitted patch, 1607662-restws-dont-set-non-property-7.patch, failed testing.

klausi’s picture

I talked to fago about this and he also said that we should keep the validation of entity properties. RESTWS provides a well-defined interface building on the entity property API, accepting data that violates the service contract makes it intransparent what is happening.

If you want to do that I would recommend to create a class MyentityRestWSResourceController extends RestWSEntityResourceController where you overwrite the update() method. Use hook_restws_resource_info_alter() to register your custom class and you should be done.

amitaibu’s picture

Status: Needs work » Closed (won't fix)

ok :)