doctrine / orm

Doctrine Object Relational Mapper (ORM)
https://www.doctrine-project.org/projects/orm.html
MIT License
9.87k stars 2.49k forks source link

Passing an integer array as parameter to doctrine SQL filter failes #5811

Open Nepoh opened 8 years ago

Nepoh commented 8 years ago

I tried to pass an array of integers to Doctrine\ORM\Query\Filter\SQLFilter::setParameter with the Doctrine\DBAL\Connection::PARAM_INT_ARRAY parameter type as I did with regular queries many times before to use the parameter in SQL IN() statements.

The documentation says Doctrine\ORM\Query\Filter\SQLFilter::getParameter takes care of parameter quoting:

The function is responsible for the right output escaping to use the value in a query.

but it looks like the parameter type is not handled correctly and an exception is thrown:

Warning: PDO::quote() expects parameter 1 to be string, array given

Is this a bug or why are array parameters not allowed?


Some code snippets of what I did: I have a filter class

use Doctrine\ORM\Query\Filter\SQLFilter;
use Doctrine\ORM\Mapping\ClassMetadata;

class UserGroupAwareFilter extends SQLFilter
{
    public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias)
    {
        $this->getParameter('ids')

        // ...
    }
}

and a configurator class to pass some parameters

class UserGroupAwareFilterConfigurator
{
    public function onKernelRequest()
    {
        $ids = // get ids ...

        $this->em->getFilters()->enable('UserGroupAware');
        $filter->setParameter(
            'ids',
            $ids,
            Connection::PARAM_INT_ARRAY
        );
    }
}
stedekay commented 7 years ago

Any news on this issue? Also trying to use an array for an IN-statement.

dimkasta commented 7 years ago

Any news? How this should be handled?

Would FIND_IN_SET be a legit solution?

24HOURSMEDIA commented 6 years ago

I have problems with this too, I implemented multiple indexed parameters for the use case but that is definitely not scalable. I don't see why a custom filter is not allowed to accept the raw parameter value..

$this->em->getFilters()->enable('CountryFilter');
$filter->setParameter('country0', 'nl');
$filter->setParameter('country1', 'uk');
// (up to 4 countries)

very bad solution.

Another, perhaps better workaround, would be to create a custom method in your filter, and then take care of escaping etc in the filter yourself:


$filter->setCountries(['nl','uk']);

// then, let the filter implementation handle the countries and escaping instead of using the regular parameters
``
lcobucci commented 6 years ago

That's kind of weird, what version of the ORM are you using? The last stable version uses the ParameterTypeInferer to use the correct type, which behaves properly with arrays.

Could you please send us a failing test case that reproduces that behaviour? It would help us a lot to identify and fix the issue you're describing.

You can find examples on https://github.com/doctrine/doctrine2/tree/388afb46d0cb3ed0c51332e8df0de9e942c2690b/tests/Doctrine/Tests/ORM/Functional/Ticket

24HOURSMEDIA commented 6 years ago

Hi I don't have too much time but I ran this short test code which illustrates the problem. Best I could get in the filter was a quoted php serialized object.. (strings are used instead of the integer mentioned in the issue but the error messages are the same)

The context is within Symfony 3.3 framework, library versions are:

doctrine/annotations                 v1.2.7             Docblock Annotations Parser
doctrine/cache                       v1.6.2             Caching library offering an object-oriented API for many cache backends
doctrine/collections                 v1.3.0             Collections Abstraction library
doctrine/common                      v2.6.2             Common Library for Doctrine projects
doctrine/dbal                        v2.5.13            Database Abstraction Layer
doctrine/doctrine-bundle             1.6.8              Symfony DoctrineBundle
doctrine/doctrine-cache-bundle       1.3.0              Symfony Bundle for Doctrine Cache
doctrine/inflector                   v1.1.0             Common String Manipulations with regard to casing and singular/plural rules.
doctrine/instantiator                1.0.5              A small, lightweight utility to instantiate objects in PHP without invoki...
doctrine/lexer                       v1.0.1             Base library for a lexer that can be used in Top-Down, Recursive Descent ...
doctrine/orm                         v2.5.10            Object-Relational-Mapper for PHP

Test code:


<?php
/**
 * Date: 09/10/2017
 */
namespace Doctrine\Tests\ORM\Functional\Ticket;

use Doctrine\DBAL\Connection;
use Doctrine\ORM\Query\Filter\SQLFilter;
use Doctrine\Tests\OrmFunctionalTestCase;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\Tests\Models\CMS\CmsArticle;

class _ArrayFilter5811 extends SQLFilter {

    public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias)
    {

        if ($this->hasParameter('myarray')) {
            $v = $this->getParameter('myarray');
            if (!is_array($v)) {
                throw new \LogicException('$v is expected to be some array or clause that can be used in an IN conditon.. got ' . $v);
            }
        }
        // return nothing as constraint for the test 
        return '';
    }
}

class GithubIssue5811Test extends OrmFunctionalTestCase
{

    public function setUp()
    {
        $this->useModelSet('cms');
        parent::setUp();
    }

    /**
     * fails: PDO::quote() expects parameter 1 to be string, array given
     */
    public function testIssueVariant1()
    {
        $em = $this->_em;
        /* @var $em \Doctrine\ORM\EntityManagerInterface */
        $em->getConfiguration()->addFilter('arrayfilter', 'Doctrine\Tests\ORM\Functional\Ticket\_ArrayFilter5811');

        $filter = $em->getFilters()->enable("arrayfilter");

        // fails: PDO::quote() expects parameter 1 to be string, array given
        $filter->setParameter('myarray', ['value1', 'value2']);
        $em->getRepository(CmsArticle::class)->findAll();

    }

    /**
     * fails: PDO::quote() expects parameter 1 to be string, array given
     */
    public function testIssueVariant2()
    {
        $em = $this->_em;
        /* @var $em \Doctrine\ORM\EntityManagerInterface */
        $em->getConfiguration()->addFilter('arrayfilter', 'Doctrine\Tests\ORM\Functional\Ticket\_ArrayFilter5811');

        $filter = $em->getFilters()->enable("arrayfilter");

        // fails: PDO::quote() expects parameter 1 to be string, array given
        $filter->setParameter('myarray', ['value1', 'value2'], Connection::PARAM_STR_ARRAY);
        $em->getRepository(CmsArticle::class)->findAll();

    }

    /**
     * No exception but within the filter we end up with a quoted php serialized object
     */
    public function testIssueVariant3()
    {
        $em = $this->_em;
        /* @var $em \Doctrine\ORM\EntityManagerInterface */
        $em->getConfiguration()->addFilter('arrayfilter', 'Doctrine\Tests\ORM\Functional\Ticket\_ArrayFilter5811');

        $filter = $em->getFilters()->enable("arrayfilter");

        // the value passed in the filter is a serialized and quoted php array
        // we just want an array that can be used in an 'in' clause
        $filter->setParameter('myarray', ['value1', 'value2'], 'array');
        $em->getRepository(CmsArticle::class)->findAll();
    }

}
spackmat commented 6 years ago

Any news on that? I can confirm this problem with doctrine/orm 2.6.1 and doctrine/dbal 2.7.0 in a Symfony 4 application. The code calls the PDO quote method, where the value of 101 (from Connection::PARAM_INT_ARRAY) is ignored, PDO has no param type for arrays, at least not as a const with that value.

videni commented 5 years ago

I also have this problem

Ocramius commented 5 years ago

Send a test, then we can patch it, if applicable.

guillaumebdx commented 5 years ago

To figure this out, I have made an implode/explode of my array.

Now, I can set an array in filter setParameter like this

<?php 

namespace Acme\AcmeBundle\Filter;

use Doctrine\Common\Persistence\ObjectManager;
use Doctrine\Common\Annotations\Reader;
use Symfony\Component\DependencyInjection\ContainerInterface;

class Configurator
{

    protected $em;
    protected $reader;
    protected $container;

    public function __construct(ObjectManager $em, Reader $reader, ContainerInterface $container)
    {
        $this->em              = $em;
        $this->reader          = $reader;
        $this->container       = $container;
    }

    public function onKernelRequest()
    {
        $filter = $this->em->getFilters()->enable('customer_filter');
        $customerIds = implode(",",$this->container->get('acme.manager.session')->getCustomerIds());
        $filter->setParameter('id', $customerIds);
        $filter->setAnnotationReader($this->reader);
    }

}
<?php 
namespace Acme\AcmeBundle\Filter;

use Doctrine\ORM\Mapping\ClassMetaData;
use Doctrine\ORM\Query\Filter\SQLFilter;
use Doctrine\Common\Annotations\Reader; 

class CustomerFilter extends SQLFilter
{

    protected $reader; 

    public function addFilterConstraint(ClassMetaData $targetEntity, $targetTableAlias)
    {
        if (empty($this->reader)) {
            return '';
        }
        $customerCheck = $this->reader->getClassAnnotation(
            $targetEntity->getReflectionClass(),
            'Acme\\AcmeBundle\\Annotation\\CustomerCheck'
            );
        if (!$customerCheck) {
            return '';
        }
        $customer = $customerCheck->customer;
        try {
             $customerIds = $this->getParameter('id');
             $customerIds = explode(",", substr($customerIds, 1, -1));

        } catch (\InvalidArgumentException $e) {
            return '';
        }
        if (empty($customer)) {
            return '';
        }
        if(count($customerIds) === 1) {
            $query = sprintf('(%s.%s = %s)', $targetTableAlias, $customer, intval($customerIds[0]), $targetTableAlias, $customer);
        } else {
            $query = sprintf('(%s.%s = %s OR', $targetTableAlias, $customer, intval($customerIds[0]), $targetTableAlias, $customer);
            for ($i = 1; $i < count($customerIds); $i++) {
                if($i < count($customerIds) -1) {
                    $query .= sprintf(' %s.%s = %s OR', $targetTableAlias, $customer, intval($customerIds[$i]), $targetTableAlias, $customer);
                } else {
                    $query .= sprintf(' %s.%s = %s)', $targetTableAlias, $customer, intval($customerIds[$i]), $targetTableAlias, $customer);
                }
            }
        }
        return $query;
    }

    public function setAnnotationReader(Reader $reader)
    {
        $this->reader = $reader;
    }

}

Hope this can help somebody

j-walte commented 4 years ago
<?php
declare(strict_types=1);

namespace App\Filter;

use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Query\Filter\SQLFilter;
use Doctrine\DBAL\Query\Expression\ExpressionBuilder;
use Prophecy\Exception\InvalidArgumentException;

class CustomFilter extends SQLFilter
{
    public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias)
    {
        if($targetEntity->getReflectionClass()->name !== 'App\Entity\MyEntity')
        {
            return '';
        }

        // unfortunately setParameter('ids', $array, \Doctrine\DBAL\Connection::PARAM_STR_ARRAY) is not supported
        // by PDO driver, use setParameter('ids', $array, \Doctrine\DBAL\Types\Type::SIMPLE_ARRAY) instead
        try
        {
            $ids = explode(',', substr($this->getParameter('ids'), 1, -1));
        }
        catch(\Exception $e)
        {
            throw new InvalidArgumentException('given value for parameter "ids" is invalid, array of strings expected');
        }

        $connection = $this->getConnection();
        $builder = new ExpressionBuilder($connection);
        return $builder->in('id', array_map([$connection, 'quote'], $ids));
    }
}

For

$filter->setParameter('ids', ['foo', 'bar'], \Doctrine\DBAL\Types\Type::SIMPLE_ARRAY);

Expression should result in

id IN ('foo', 'bar')
edigu commented 4 years ago

I hit the same issue today when implementing a custom filter. For my use case it looks like a weird PDO behavior, reported as bug here in 2009.

Doctrine\ORM\Query\Filter\SQLFilter::getParameter retrieves a \Doctrine\DBAL\Connection instance from EntityManager to quote parameter value through it.

Connection::quote only proxies the parent's interface ():

public function quote($input, $type = ParameterType::STRING)
{
    return parent::quote($input, $type); // parent is PDO here
}

So in my case, the PDO driver was failing, even doctrine properly passes value and type information to underlying layers.

Since MySQL is also happy when parsing such statement on status_id (int) column:

select * from comments where status_id = '2'

I stopped annoying from this.