zfcampus / zf-content-validation

ZF2 module for automated input validation
BSD 3-Clause "New" or "Revised" License
20 stars 25 forks source link

PATCH request to collection endpoint with empty body causes RuntimeException #103

Open aeux opened 5 years ago

aeux commented 5 years ago

When sending a PATCH request to a collection endpoint the following things happen within the ContentValidationListener's onRoute:

$data = in_array($method, $this->methodsWithoutBodies)
    ? $dataContainer->getQueryParams()
    : $dataContainer->getBodyParams();
if (null === $data || '' === $data) {
    $data = [];
}

For a PATCH request with an empty content body, this will result in $data = []

if ($isCollection && ! in_array($method, $this->methodsWithoutBodies)) {
    $collectionInputFilter = new CollectionInputFilter();
    $collectionInputFilter->setInputFilter($inputFilter);
    $inputFilter = $collectionInputFilter;
}

$isCollection is true since we are hitting a collection endpoint and PATCH is not a method without a body, so a collection input filter is created and set to the input filter for this request.

Then the input filter is validated against, for a PATCH request the $this->validatePatch() function is fired:

$status = ($request->isPatch())
    ? $this->validatePatch($inputFilter, $data, $isCollection)
    : $inputFilter->isValid();

Which then will return $inputFilter->isValid(), which will always be a CollectionInputFilter at this point.

This is the beginning CollectionInputFilter's isValid()

public function isValid($context = null)
{
    $this->collectionMessages = [];
    $inputFilter = $this->getInputFilter();
    $valid = true;

    if ($this->getCount() < 1) {
        if ($this->isRequired) {
            $valid = false;
        }
    }

$this->getCount() performs a count() on $this->data and evaluates 0:

public function getCount()
{
    if (null === $this->count) {
        return count($this->data);
    }
    return $this->count;
}

But $this->isRequired is false, unless there is a listener set up for ContentValidationListener::EVENT_BEFORE_VALIDATE which changes the inputFilter's required setting. So without a listener set up, $valid = true at this point.

Here's the next few lines of isValid():

if (count($this->data) < $this->getCount()) {
    $valid = false;
}

if (! $this->data) {
    $this->clearValues();
    $this->clearRawValues();

    return $valid;
}

At this point, $this->getCount() will return the same value as count($this->data) and therefore the conditional will fail and $valid = true.

$this->data is currently [], which is a "falsey" value and will cause the conditional to be true. The method then calls two functions and returns true because $valid = true.

We then return to execution of onRoute() via ContentValidationListener:

if ($status instanceof ApiProblemResponse) {
    return $status;
}
// Invalid? Return a 422 response.
if (false === $status) {
    return new ApiProblemResponse(
        new ApiProblem(422, 'Failed Validation', null, null, [
            'validation_messages' => $inputFilter->getMessages(),
        ])
    );
}

$status is currently true as returned by $this->validatePatch(), so neither of these conditionals is met.

We then get to this part of onRoute:

if (! $inputFilter instanceof UnknownInputsCapableInterface
    || ! $inputFilter->hasUnknown()
) {
    $dataContainer->setBodyParams($data);
    return;
}

CollectionInputFilter is an instanceof UnknownInputsCapableInterface, so the conditional does not short-ciruit and continues with $inputFilter->hasUnknown()

CollectionInputFilter does not define it's own hasUnknown() method and instead inherits it from BaseInputFilter:

public function hasUnknown()
{
    return $this->getUnknown() ? true : false;
}

CollectionInputFilter does define a method for getUnknown() and here's the very beginning:

public function getUnknown()
{
    if (! $this->data) {
        throw new Exception\RuntimeException(sprintf(
            '%s: no data present!',
            __METHOD__
        ));
    }

Since $this->data is currently [], this conditional evaluates to true and a RuntimeException is thrown. Currently the only way of avoiding this RuntimeException is by hooking into ContentValidationListener::EVENT_BEFORE_VALIDATE and validating that the request body is not empty manually.

weierophinney commented 4 years ago

This repository has been closed and moved to laminas-api-tools/api-tools-content-validation; a new issue has been opened at https://github.com/laminas-api-tools/api-tools-content-validation/issues/2.