Invalid IRI causes 500 TypeError in IriFilter instead of returning 400 Bad Request

Open
  • Is something wrong?
#8,834 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 2 days

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
57/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Active
Tech stack
php, symfony
Domain
backend-api-design

Research direction

Start by reading api-platform/core/src/State/ParameterProvider/IriConverterParameterProvider.php and the IriFilter call path that produces the reported ClassMetadata::getIdentifierValues() error. Reproduce the request with a malformed participant IRI, then check how the two caught exception types are handled and what filter settings already exist. Done means invalid or missing IRIs produce the intended client error without the reported 500; the issue leaves the exact compatibility behavior open.

Written by the indexing model from the issue text.

Description

API Platform version(s) affected: At least 5.0.5 and 5.0.6

Description
When an API endpoint is called with an IriFilter and the user passes it an invalid IRI, they will get a 500 error stating that 'Doctrine\ORM\Mapping\ClassMetadata::getIdentifierValues()' received a string instead of an entity. This is caused by 'api-platform/core/src/State/ParameterProvider/IriConverterParameterProvider.php' catching both InvalidArgumentExceptions and ItemNotFoundExceptions and then swallowing them with only a log entry.

For ItemNotFoundException (caused by an IRI being passed that does technically meet the expected components, for example /activities/horseriding being passed when /activities/{type} is expected but horseriding does not exist) there is a property that can be enabled on a per filter basis that would solve this (defaulting to throwing 500 messages for user error and having to manually fix that for every endpoint feels wrong to me too, but at least there is a solution).

For InvalidArgumentException (caused by an IRI being passed that does not match the expected components at all, for example /stufftodo/horseriding or activities/horse/riding for where /activities/{type} is expected) there is no way to fix the issue however. It is always swallowed with just a log entry and there is no extra property that can be set to make it return a 400 response anyway.

In both cases, when an InvalidArgumentException is thrown by the iriconverter or an ItemNotFoundException with the extra property not being set explicitly, it will result in the string given in the parameter of the IriFilter being passed into a method that expects us to have retrieved an object by now, thus throwing us a 500 exception that has no meaning to someone using the API, as there is not really a way for them to know the cause was them passing an invalid value.

How to reproduce
Reproducton should be relatively simpel. Create two objects, for example Activity and Participant with a mapping between them, for example a ManyToMany. Create at least one of each entity in the database, both of which map to eachother. Then create a GetCollection endpoint for Activity with an IriFilter on Participant (lets say the endpoint becomes api.site.com/activities). Now call the endpoint with the participant filter containing a faulty iri. So lets say the iri for participant is /participants/{id}, do something like 'api.site.com/activities.jsonld?participant=/some/wrong/iri'. This should return a 500 message.

Possible Solution
We would ideally like to be able to throw a 400 for both InvalidArgumentExceptions and ItemNotFound exceptions from the IriConverter by default, for example by setting a value in a config file, as we do not think a wrong value given by a user should throw 500's. We would personally like that to be default bevahiour, but enabling that by default would cause backwards compatability issues for people who DO want the 500, so having to enable a config setting is fine. Ofcourse manual per-filter settings could still be made to override this config default.

If that is not desired by ApiPlatforms design standards, and you want to keep throwing 500's by default unless explicitly specified on a per-filter basis, adding an adition ExtraParameter option like this would also be a solution. This would need to be implemented in both catch blocks in the IriConverterParameterProvider, either above or below the existing similar IF statement for ItemNotFoundExceptions.

if ($exception instanceof InvalidArgumentException && true === ($extraProperties['throw_invalid_argument'] ?? false)) {
     throw $exception;
}

Additional Context
The exact error mesage in case it's usefull: "Doctrine\\ORM\\Mapping\\ClassMetadata::getIdentifierValues(): Argument #1 ($entity) must be of type object, string given, called in /app/vendor/api-platform/core/src/Doctrine/Orm/Filter/IriFilter.php on line 110"

Dominant language
PHP
Stars
2.6k
Forks
988
Avg merge
4d 16h
Merged PRs (30d)
118

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from api-platform/core

All issues in api-platform/core

Similar issues

More PHP issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.