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

オープン
  • 何か問題がありますか？
#8,834 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 2 日以内に返信

まだ誰も着手していません。

評価

難易度
4/5
見積もり時間
3〜5日
初心者へのやさしさ
57/100
issue の種類
バグ
明瞭さ
おおむね明確
活発さ
活発
技術スタック
php, symfony
領域
backend-api-design

調査の方向性

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.

索引モデルが issue の本文から書いたものです。

説明

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"

主要言語
PHP
スター
2.6k
フォーク
988
平均マージ
4日 16時間
マージ済み PR（30日）
118

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

api-platform/core のほかの issue

api-platform/core の issue をすべて見る

似ている issue

PHP の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。