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

Đang mở
  • Có gì không ổn?
#8,834 0 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 2 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức phù hợp với người mới
57/100
Loại issue
Lỗi
Độ rõ ràng
Khá rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
php, symfony
Lĩnh vực
backend-api-design

Hướng nghiên cứu

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.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

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"

Ngôn ngữ chính
PHP
Star
2.6k
Fork
988
Merge trung bình
4 ngày 16 giờ
Pull request đã merge (30 ngày)
118

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của api-platform/core

Tất cả issue của api-platform/core

Issue tương tự

Thêm issue về PHP

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.