Core: Trying to get entity by wrong id (string instead of number) produces 500, instead of 404

[Strate]

I have an entity with an integer id. I am trying to get it by wrong id: make a GET request to "/countries/someid". I goot 500 error insteadof 404:

{"@context":"\/contexts\/Error","@type":"Error","hydra:title":"An error occurred","hydra:description":"An exception occurred while executing \u0027SELECT t0.id AS id_1, t0.name AS name_2 FROM template_store.country t0 WHERE t0.id = ?\u0027 with params [\u0022blablabla\u0022]:\n\nSQLSTATE[22P02]: Invalid text representation: 7 ERROR:  invalid input syntax for integer: \u0022blablabla\u0022","trace":[{"file":"\/devel\/src\/bitbucket\/template-store\/vendor\/doctrine\/dbal\/lib\/Doctrine\/DBAL\/DBALException.php","line":116 # cutted

This happens with postgresql database. It seems that there should be requirement on {id} parameter of route, based on type of identifier.

[dunglas]

It looks like an edge case related to Postgres: IDs can be strings (uuid for instances).

[dunglas]

Closing as won't fix. When using Postgres, routes must enforce type of ids, and it's now easy to do wit the new config system of v2.

[bwegrzyn]

I ran in to this issue as well, so here is what I did to fix this.

Enforcing UUID syntax on routes sort of works, but it's tedious without writing some custom route loader, and it doesn't cover the use case of providing IRIs for relations when creating or updating a resource.

I ended up decorating the Doctrine ItemDataProvider service, and registering it with a higher priority than the ItemDataProvider service provided by the API Platform Bundle.

services:
  app.api.item_data_provider:
    class: AppBundle\Doctrine\Orm\ItemDataProvider
    arguments: ['@api_platform.doctrine.orm.default.item_data_provider']
    tags:
      - { name: 'api_platform.item_data_provider', priority: 64 } # Any priority higher than 0 will do

namespace AppBundle\Doctrine\Orm;


use ApiPlatform\Core\Bridge\Doctrine\Orm\ItemDataProvider as DecoratedItemDataProvider;
use ApiPlatform\Core\DataProvider\ItemDataProviderInterface;
use Doctrine\DBAL\Exception\DriverException;

class ItemDataProvider implements ItemDataProviderInterface
{

    /**
     * @var DecoratedItemDataProvider
     */
    protected $decorated;

    /**
     * ItemDataProvider constructor.
     *
     * @param DecoratedItemDataProvider $decorated
     */
    public function __construct(DecoratedItemDataProvider $decorated)
    {
        $this->decorated = $decorated;
    }

    /**
     * {@inheritdoc}
     */
    public function getItem(string $resourceClass, $id, string $operationName = null, bool $fetchData = false)
    {
        try {
            return $this->decorated->getItem($resourceClass, $id, $operationName, $fetchData);
        } catch (DriverException $e) {

            // Catch "SQLSTATE[22P02]: Invalid text representation" errors
            // This prevents exceptions when requesting data with invalid UUID syntax

            if ($e->getSQLState() === '22P02') {
                return null;
            }

            throw $e;
        }
    }
}

[dunglas]

Nice workaround! Should be integrate it upstream @api-platform/core-team?

[Simperfit]

It would be better for postgres users to have it upstream.

[soyuka]

As long as there are no 22P02 sql states on exceptions with other drivers it's find to add it upstream (maybe add a test on platform->getName() === 'postgres' ?).

Anyway :+1: if the not found error is better than the postgres one (which might lead to misunderstandings in debugging).

[bwegrzyn]

My workaround also covers invalid input syntax for integers (such as passing a string where postgres expects an integer).

A quick Google search did not reveal any other drivers that throw this sql state, but I'm not a database expert.

Anyways, it would be a good idea to test on the platform as well. I didn't do this in my case as I use postgres exclusively.

[soyuka]

My workaround also covers invalid input syntax for integers (such as passing a string where postgres expects an integer).

This is less good, as a developer I want to be aware of the issue instead of it being catch silently

[bwegrzyn]

This is less good, as a developer I want to be aware of the issue instead of it being catch silently

In this particular case, I disagree. This error only occurs when you pass data with invalid syntax to the query, and in most cases this comes from the URL id parameter. This parameter can be manipulated by the end user, so would you rather return a 500 error or a 404 error when a bad ID is passed?

-- GET /user/1234
SELECT * FROM user WHERE id = 1234; -- Returns null if user is not found, and produces a 404 code

-- GET /user/InvalidInputFromUser
SELECT * FROM user WHERE id = 'InvalidInputFromUser'; -- Returns exception, and produces a 500 code 

From what I know, other database systems such as MySQL do not throw an error when you pass erroneous input data, they just return nothing.

[soyuka]

It does with Oracle :) : Error ORA-00932: inconsistent datatypes: expected DATE got NUMBER for example. And yes, your front end developer might send a string instead of a number or whatever and would like to know why it ends up returning a 404 instead of a proper error (which he will eventually discover after few debug time).
That's only my opinion ofc.

[bwegrzyn]

Fair enough on the Oracle point, I don't use it much ;)

I'm not concerned about the front end developer passing bad data to the API, but most UIs simply take the ID parameter from the URL and pass it straight to the API. Should it be left up to the UI to validate the syntax of the ID? The UI doesn't (and probably shouldn't) know the syntax that the database expects for the ID parameter.

The syntax should be validated in the API platform before passing to the DBAL driver, or it should gracefully handle the error returned by DBAL, or maybe DBAL should handle it?

I'm not sure what the correct fix here is, but I know that the API probably shouldn't return a 500 error when a malformed ID is passed, and my fix above accomplishes that for me.

Maybe instead of returning a 404 Not Found, it should return a 400 Bad Request? It could be configurable as a "enforce_strict_input_syntax" option which defaults to true and returns a 400, or if it's set to false it just returns 404?

[teohhanhui]

I do not agree that it should respond with HTTP 400, even with strong typing in mind. The user has attempted to access a non-existent resource. 404 is the correct response.

But I think this can be handled at the routing level. Symfony Router: add a requirement to the id parameter to only accept integers.