Skip to content

[DoctrineBridge] Middleware fails to handle binary values #46744

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
geoffrey-brier opened this issue Jun 23, 2022 · 6 comments
Closed

[DoctrineBridge] Middleware fails to handle binary values #46744

geoffrey-brier opened this issue Jun 23, 2022 · 6 comments
Labels
Bug DoctrineBridge Status: Needs Review

Comments

@geoffrey-brier
Copy link
Contributor

geoffrey-brier commented Jun 23, 2022 •
edited
Loading

Symfony version(s) affected

>= 5.4

Description

Hello there,

I just upgraded my application today (I switched from doctrine bundle 2.6.3 to 2.7.0) which is in 6.1 and my tests detected something weird.

Here is a part of the stack trace :

      Behat\Testwork\Call\Exception\FatalThrowableError: Type error: Symfony\Bridge\Doctrine\Middleware\Debug\Query::setValue(): Argument #2 ($value) must be of type string|int|float|bool|null, resource given, called in vendor/symfony/doctrine-bridge/Middleware/Debug/Statement.php on line 48 in vendor/symfony/doctrine-bridge/Middleware/Debug/Query.php:55
      Stack trace:
      #0 vendor/symfony/doctrine-bridge/Middleware/Debug/Statement.php(48): Symfony\Bridge\Doctrine\Middleware\Debug\Query->setValue(5, Resource id #2538, 16)
      #1 vendor/doctrine/dbal/src/Statement.php(115): Symfony\Bridge\Doctrine\Middleware\Debug\Statement->bindValue(5, Resource id #2538, 16)
      #2 vendor/doctrine/orm/lib/Doctrine/ORM/Persisters/Entity/BasicEntityPersister.php(274): Doctrine\DBAL\Statement->bindValue(5, Resource id #2538, Object(Doctrine\DBAL\Types\BinaryType))
      #3 vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php(1129): Doctrine\ORM\Persisters\Entity\BasicEntityPersister->executeInserts()
      #4 vendor/doctrine/orm/lib/Doctrine/ORM/UnitOfWork.php(426): Doctrine\ORM\UnitOfWork->executeInserts(Object(Doctrine\ORM\Mapping\ClassMetadata))
      #5 vendor/doctrine/orm/lib/Doctrine/ORM/EntityManager.php(398): Doctrine\ORM\UnitOfWork->commit(NULL)
      #6 var/cache/test/ContainerZdJq7y7/EntityManager_9a5be93.php(136): Doctrine\ORM\EntityManager->flush(NULL)
      #7 src/Services/NetworkIpv6Manager.php(163): ContainerZdJq7y7\EntityManager_9a5be93->flush()
      #8 src/Controller/Ipv6Controller.php(136): App\Services\NetworkIpv6Manager->splitIpv6Subnet(Object(App\Entity\ConfNetworkclass), '2001:bc8:3000:1...')

After digging a bit, I realized that this commit introduced a change in the query loggers (dbal logger -> middleware) which is the starting point of the issue. Indeed, it looks like the Query class does not handle binary values so that's why we end up with a fatal error.

How to reproduce

Create an entity with a field of 'binary' type (I guess it would work for blob values as well) ... pushing further the thought I'm even wondering if the DateTime would be handled correctly but I'm far from being an expert on this one 🤔

Possible Solution

No response

Additional Context

No response
@marforon
Copy link

It should also accept arrays as an argument, for example array of ids.

@dmaicher
Copy link
Contributor

@l-vo could this maybe be relaxed to mixed?

@l-vo
Copy link
Contributor

l-vo commented Jun 27, 2022

@dmaicher, yes, using mixed should fix the problem. doctrine/dbal 4.x uses a mixed type for the value. I'm going to suggest a fix.

This was referenced Jun 29, 2022
Closed
Merged
@l-vo
Copy link
Contributor

l-vo commented Jun 29, 2022

PR proposed. Actually, versions 5.4 and 6.0 are not affected.

@fabpot fabpot closed this as completed Jul 2, 2022
fabpot added a commit that referenced this issue Jul 2, 2022
@fabpot
bug #46811 [DoctrineBridge] Fix comment for type on Query::setValue (…
06400fb 
…middlewares) (l-vo)

This PR was merged into the 5.4 branch.

Discussion
----------

[DoctrineBridge] Fix comment for type on Query::setValue (middlewares)

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        |

See #46810 and #46744

Commits
-------

928a754 [DoctrineBridge] Fix comment for type on Query::setValue (middlewares)
@chetzel
Copy link

chetzel commented Apr 25, 2023

Hi, thank you for the fix for setValue(), but how about setParam() 11 lines above it (line 46 in current implementations of Query) - looks like the exact same issue. I know bindParam() is marked as deprecated, still current solutions rely on this behavior.

@l-vo l-vo mentioned this issue May 8, 2023
Merged
@l-vo
Copy link
Contributor

l-vo commented May 8, 2023

@chetzel indeed, I don't know why I didn't fix this at the same time. Thank you for reporting. Fix proposed on #50268

fabpot added a commit that referenced this issue May 10, 2023
@fabpot
bug #50268 Allow resources in Query::setParam (l-vo)
3cefdcb 
This PR was merged into the 6.2 branch.

Discussion
----------

Allow resources in Query::setParam

| Q             | A
| ------------- | ---
| Branch?       | 6.2
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        |

See #46744 (comment)

(failures in tests seem not related)

Commits
-------

318e0e4 Allow resources in Query::setParam
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug DoctrineBridge Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@fabpot @dmaicher @chetzel @geoffrey-brier @carsonbot @l-vo @marforon