zfcampus/zf-content-validation

Rest content validation issue

tigran-m-dev opened this issue · 7 comments

tigran-m-dev commented

In rest service (fetch all for example), if I seet allows_only_fields_in_filter for GET method
I will got this kind of error Unrecognized fields: 0
This because content something like this

array (size=1)
  0 => 
    array (size=1)
      'server_name' => string 'asdasd'

if it is not a get method the content of values are this

  array (size=1)
      'server_name' => string 'asdasd'

This problem just in REST fetch all, please have a look and tell me how I can fix this for now, before your update.

Thanks.

tigran-m-dev commented

@weierophinney Is it possible some quick fix in my end before global update?

weierophinney commented

please have a look and tell me how I can fix this for now, before your update.

I'm not sure what you mean by this exactly... We don't have any planned updates for today, though we did an admin UI release a bit ago (unrelated to this).

I need more information, as I'm unsure how to reproduce the error, exactly. In particular, where do you get the error? Is it when requesting the API? or after submitting something via the admin UI? If via your API, can you provide:

  • Directions showing how you created the input filter
  • Directions demonstrating how you tried to invoke the API

That will help us reproduce and analyze the problem.

Thanks!

weierophinney commented

In an effort to try and reproduce, I've done the following:

  • Started a new Apigility application
  • Created a new API, Test
  • Created a new RPC service for that API, Foo
  • Created an input filter with a single field, server_name, which had a single validator (string length, allowing 3 - 140 chars)
  • Edited the API's module configuration to:
    • Specify this new input filter as usable via GET for the specified controller service
    • enable the allows_only_fields_in_filter flag for the specified controller service

I then edited my controller's action to read as follows:

public function fooAction()
{
    $event = $this->getEvent();
    $inputFilter = $event->getParam(\ZF\ContentValidation\InputFilter::class);
    return $inputFilter->getValues();
}

Finally, I tried accessing the service via each of:

  • /foo?server_name=fo (which should not validate)
  • /foo?server_name=foobar (which should validate)
  • /foo?server_name=foobar&my_name=mwop (which invalidates due to extra values)

In each, it behaved as expected.

So, I need a reproduce case, and the version of zf-content-validation you're using.

Thanks!

weierophinney commented

Okay, I've managed to reproduce it.

  • Created a new REST API, FooBar
  • Created the same input filter as described in the previous comment, and configured it the same way.
  • Updated the new resource's fetchAll() method to read as follows:
public function fetchAll($params = [])
{
    $inputFilter = $this->getInputFilter();
    $entity = new FooBarEntity(array_merge($inputFilter->getValueS(), ['id' => uniqid()]));
    return new FooBarCollection(new \Zend\Paginator\Adapter\ArrayAdapter([$entity]));
}

I also updated FooBarEntity to extend ArrayObject.

Once I did that, I tried the following two requests:

  • /foo-bar?server_name=fo — this correctly failed validation
  • /foo-bar?server_name=foobar — this demonstrated the issue reported
  • /foo-bar?server_name=foobar&my_name=mwop — this also demonstrated the issue

I think this likely makes sense, as operations on the resource collection should be submitting collections, and not individual entity fields.

Indeed, when I then modify the fetch() method to do the same (though using the provided $id value in the returned payload, and returning only the entity itself), it works correctly.

The question is whether or not we should allow validating non-collections when operating on collection endpoints. My personal take is that we shouldn't, and any such validation should be handled by the end-user (usually via a resource listener).

michalbundyra commented

@weierophinney Please have a look on my fix #83.

I think this likely makes sense, as operations on the resource collection should be submitting collections, and not individual entity fields.

Currently, in case of GET request on collection (method without body) params are converted to collection:
https://github.com/zfcampus/zf-content-validation/blob/master/src/ContentValidationListener.php#L249-L251
so it is not even possible to pass a collection.

Anyway, in method fetchAll, variable $params in not a collection, but just array with passed params.
In your example, you are getting these params from the InputFilter (so then it's a collection), and it is reasonable (as we have it in other endpoints - params passed to the method are not filtered, and to get filtered values we have to fetch it from the InputFilter).

What is the reason passing params on GET request of the collection?
IMO it should be used as some kind of filters (for example /album?name=FooBar should returns all albums with name FooBar).

In my PR I've fixed reported issue. In described situation we are not converting params to the collection, and InputFilter is not converted to CollectionInputFilter (in case of request without body).

I think there is a bug in CollectionInputFilter - it wrongly recognizes unknown fields - please see: zendframework/zend-inputfilter#115.

Also I think there is another issue on posting collections. Again, caused by CollectionInputFilter, because it always returns all fields as unknown - please see #84.

tigran-m-dev commented

I manually added #83 fixes and is is working, my issue is resolved.
Thanks @webimpress
@weierophinney are you going to merge this fix with master and create new release? If yes , when it should be done?
Thanks again for quick responce.

weierophinney commented

@developer-devPHP We'll have a release today, likely in the next hour.