Skip to content
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.

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

Could not convert database value "11ead97b-ffa1-7458-a..." to Doctrine Type uuid_binary_ordered_time. Expected format: UuidV1 #329

Closed
537mfb opened this issue Aug 8, 2020 · 18 comments · Fixed by #361
Labels

Comments

@537mfb
Copy link

537mfb commented Aug 8, 2020

Describe the bug

For the last couple of weeks I've noticed that in symfony UuidOrderedTimeGenerator is generating other than v1 Uuids intead of the expected v1, and that leads to throwing a converter error when validating the value

Exception message:
Could not convert database value "xxxxxxxx-xxxx-xxxx-x..." to Doctrine Type uuid_binary_ordered_time. Expected format: UuidV1

To Reproduce...

  1. Setup a new symfony project with registration and authentication:

    composer create-project symfony/website-skeleton symf
    cd symf
    composer require ramsey/uuid-doctrine
    composer require symfonycasts/verify-email-bundle
    bin/console make:user
    bin/console make:registration-form
    during this process choose to login automatically on registration success
    bin/console make:auth

  2. Basic project configuration:

    Follow instructions given by make:auth to fix the return in onAuthenticationSuccess to a valid route (use bin/console make:controller DefaultController to create an empty controller to use here)
    change config/packages/ramsey._uuid_doctrine.yaml to be:

doctrine:
    dbal:
        types:
            uuid: 'Ramsey\Uuid\Doctrine\UuidType'
            uuid_binary: 'Ramsey\Uuid\Doctrine\UuidBinaryType'
            uuid_binary_ordered_time: 'Ramsey\Uuid\Doctrine\UuidBinaryOrderedTimeType'
        mapping_types:
            uuid_binary: binary
            uuid_binary_ordered_time: binary
In src/Entity/User.php change the type of $id to uuid
use \Ramsey\Uuid\UuidInterface;
....
/**
 * @ORM\Id()
 * @ORM\Column(type="uuid_binary_ordered_time", unique=true)
 * @ORM\GeneratedValue(strategy="CUSTOM")
 * @ORM\CustomIdGenerator(class="Ramsey\Uuid\Doctrine\UuidOrderedTimeGenerator")
 */
private $id;
...
public function getId(): ?UuidInterface
{
    return $this->id;
}
change .env so that `MAILER_DSN` and `DATABASE_URL` are both set and valid according to your own mail and database servers (i'm using my ISP's mail server and mariadb-10.4.7 in my test)
  1. Prepare your database:

    bin/console doctrine:migrations:diff
    bin/console doctrine:migrations:migrate

  2. Run symfony's server or setup a vhost and go to the registration page http:///register in a browser. Fill out the form and click the register button

  3. See output similar to the following:

    on registration success a new entry is added to the user table in the database but during the login process the following exception is thrown and unhandled

    Could not convert database value "11ead97b-ffa1-7458-a..." to Doctrine Type uuid_binary_ordered_time. Expected format: UuidV1
    

Examining the value of the id it is not in fact validated as version 1

Expected behavior

A new entry is added to the user table in the database using the information typed in the form, login suing those credentials and redirect to the route setup in onAuthenticationSuccess .

Screenshots or output

Symfony Exception
ConversionException
HTTP 500 Internal Server Error
Could not convert database value "11ead97b-ffa1-7458-a..." to Doctrine Type uuid_binary_ordered_time. Expected format: UuidV1
in vendor/ramsey/uuid-doctrine/src/UuidBinaryOrderedTimeType.php :: conversionFailedFormat (line 190)

        {
            if (1 !== $value->getVersion()) { 
                throw ConversionException::conversionFailedFormat(
                    $value->toString(),
                    self::NAME,
                    self::ASSERT_FORMAT
                );
            }
        }
        /**

in vendor/ramsey/uuid-doctrine/src/UuidBinaryOrderedTimeType.php -> assertUuidV1 (line 204)

         *
         * @throws ConversionException
         */
        private function encode(UuidInterface $uuid)
        {
            $this->assertUuidV1($uuid);
            return $this->getCodec()->encodeBinary($uuid);
        }
        /**

in vendor/ramsey/uuid-doctrine/src/UuidBinaryOrderedTimeType.php -> encode (line 108)

            if ($value === null || $value === '') {
                return null;
            }
            if ($value instanceof UuidInterface) {
                return $this->encode($value);  
            }
            try {
                if (is_string($value) || method_exists($value, '__toString')) {
                    $uuid = $this->getUuidFactory()->fromString((string) $value);


Environment details

  • OS: Linux (Ubuntu 20.04)
  • PHP version: 7.4.5
  • ramsey/uuid version: 4.1.0
  • ramsey/uuid-doctrine version: 1.6.0
  • ramsey/collection version: 1.0.1

Additional context

This started happenning a couple of weeks back when i updated ramsey/uuid using composer update - can't remember what version i had before

@537mfb 537mfb added the bug label Aug 8, 2020
@kylehqcom
Copy link

Hi Luis, I may be able to help here as I'm getting this issue also with a new Symfony5 project. Except for me, the error would toggle. One request would work, refresh the page and then fail.

Luckily I've used these ID's in past as a standalone package so I had an idea as to how to address. To be clear, I am no sure that this is the "correct" way, there could well be a bug here. The issue being that you may be dealing with string hex id values when you need to find using bytes.

As part of the Symfony user auth mechanism the vendor/symfony/doctrine-bridge/Security/User/EntityUserProvider.php file calls the refreshUser method. If you look at this method, it checks first if the $user repository is a UserProviderInterface, and if not (like my current setup), the code will then get the Repository for the $user and call a common doctrineRepo->find($id).

It's likely that you have a @ORM\Entity(repositoryClass=UserRepository::class) on your User entity. Because of this, you can overwrite the find method for the user. This is my example currently

namespace App\Repository;

use App\Entity\User;
use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository;
use Doctrine\Common\Collections\Criteria;
use Doctrine\Persistence\ManagerRegistry;
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
use Symfony\Component\Security\Core\User\UserInterface;

/**
 * @method User|null find($id, $lockMode = null, $lockVersion = null)
 * @method User|null findOneBy(array $criteria, array $orderBy = null)
 * @method User[]    findAll()
 * @method User[]    findBy(array $criteria, array $orderBy = null, $limit = null, $offset = null)
 */
class UserRepository extends ServiceEntityRepository implements PasswordUpgraderInterface
{
    public function __construct(ManagerRegistry $registry)
    {
        parent::__construct($registry, User::class);
    }

    public function find($id, $lockMode = null, $lockVersion = null)
    {
        if (is_array($id)) {
            $uuidInstance = reset($id);
            $uuidCriteria = Criteria::create()->where(Criteria::expr()->eq('u.id', $uuidInstance->getBytes()));
        } else {
            dd("complete UserRepository find");
        }
        return $this->createQueryBuilder('u')
            ->addCriteria($uuidCriteria)
            ->getQuery()
            ->getOneOrNullResult()
        ;
    }

A couple of points here - the Symfony SecurityUser calls the find with an array of values, eg $id == [$uuidInstance] so at this point I'm just checking on array and getting the instance.

You must use a Criteria::expr() in your where clause and call the getBytes(). This is something I've learned previously. If you use a :? param in a where clause, then the parsing of the param will then invoke the __toString method on the UUID instance and therefore never find the "byte" id. Using an expression respects the bytes returned.

If you overwrite this find method, then it should work ok. Again, I think there is also an issue here but this is how I have dealt with it.

As it's likely that I will have hex strings that I will need to convert to bytes, eg an api call with a user_id hex string, then I normally end up overwriting these methods to convert the string into a valid UUID instance to retrieve from the db. Or I make explicit repository calls. Then I know exactly what is going on and how to cast each $id value that is passed to the find. Note that I have never had an issue with ->getRelation() methods of save graphs, that all works as expected in my experience.

So if this is a bug and it gets fixed, then great. Otherwise here is a workaround and a good tip for future to use the Criteria and getBytes() when needing to retrieve from the DB.
Best
Kyle

@kylehqcom
Copy link

Also to help with the bug report, the $value->getVersion() is always returning null, although the underlying isNil() method should be returning false because there is definitely a valid byte string present.

@537mfb
Copy link
Author

537mfb commented Aug 12, 2020

Hi @kylehqcom

My case is slightly different than yours as I am implementing UserInterface and I search by email and not by id but i get what you're getting at with overriding the find method and do things manually.

I'll try that workaround tomorrow - it's past 1am here now

Also in my case I've seen $value->getVersion() be null, 1, 2 and 3 (I guess when it's 1 it's what you meant by some requests working for you) - but yes, null seems to be the norm

Also it was working perfectly before my last update of ramsey/uuid so I think either a bug or a breaking change was introduced by accident somewhere and it doesn't play nice with ramsey/uuid-doctrine anymore

@kylehqcom
Copy link

Hopefully you won't need to "work around" as this seems related to here #327

Let me know if I can help out in any other way. Enjoy your sleep! Boa noite

@537mfb
Copy link
Author

537mfb commented Aug 15, 2020

Just noted something: if you get the string representation of the UUID, the 13th character is the version indicator. However, the UUIDs being created have the 13th digit go off the range of valid versions.

Here's some examples of generated UUIDs:

11ead97b-ffa1-7458-a66b-00155d000701    <-- Version 7?
11eadc33-e890-a6ca-ba10-00155d000701    <-- Version 10?
11eadd9d-e00a-bbba-8789-00155d000701    <-- Version 11?

That's why getVersion ends up null - it doesn't recognize the version identifier as valid
Not sure why this is happening though - tested all generators and all work by itself

Uuid::uuid1();
4ca15652-decb-11ea-9c38-00155d000701   <-- Version 1

Uuid::uuid2(Uuid::DCE_DOMAIN_PERSON);
00000021-decb-21ea-8800-00155d000701   <-- Version 2

Uuid::uuid3(Uuid::NAMESPACE_URL, 'https://www.php.net');
3f703955-aaba-3e70-a3cb-baff6aa3b28f   <-- Version 3

Uuid::uuid4();
f3bf1ff6-393b-4ac8-80c4-ea8007eac922   <-- Version 4

Uuid::uuid5(Uuid::NAMESPACE_URL, 'https://www.php.net');
a8f6ae40-d8a7-58f0-be05-a22f94eca9ec   <-- Version 5

Uuid::uuid6();
1eadecb4-ca1c-601a-8e91-00155d000701   <-- Version 6

Uuid::fromDateTime($datetime);
4ca15652-decb-11ea-8312-00155d000701   <-- Version 1

@kylehqcom
Copy link

Hmmm, weird? Nice catch on your part however. That should help narrow down the issue 👍

@537mfb
Copy link
Author

537mfb commented Aug 16, 2020

weird indeed - so far havent spotted the culprit though

@537mfb
Copy link
Author

537mfb commented Aug 18, 2020

So i found a possible fix
Although I'm not sure what the code I commented was there for so I may be doing something I shouldn't

Hopefully someone who know the code better than me can pitch in

in file ramsey/uuid/codec/OrderedTimeCodec.php I changed:

Lines 70 - 73

return $bytes/*[6] . $bytes[7]
            . $bytes[4] . $bytes[5]
            . $bytes[0] . $bytes[1] . $bytes[2] . $bytes[3]
            . substr($bytes, 8)*/;

Lines 93 - 96

$rearrangedBytes = $bytes/*[4] . $bytes[5] . $bytes[6] . $bytes[7]
            . $bytes[2] . $bytes[3]
            . $bytes[0] . $bytes[1]
            . substr($bytes, 8)*/;

After this changes the error went away and so far it seems to work

Could anyone tell me the purpose of that byte rearranging please?

@Microtribute
Copy link

@537mfb I am getting the exact same issue.

@537mfb
Copy link
Author

537mfb commented Aug 24, 2020

@Microtribute does commenting that byte rearrangement work for you as well? Do you know of any downsides/side effects to commenting them?

@Microtribute
Copy link

@537mfb I don't know a possible fix. I used the uuid_binary type instead.

@537mfb
Copy link
Author

537mfb commented Sep 5, 2020

@Microtribute Using uuid_binary instead looses what you gain with it beeing ordered in time - everytime you do an insert all indexes could need rearranged due to pagination. In a small/development database that's ok but in a large production database it could be a hit in performance

@eXtreme
Copy link

eXtreme commented Sep 9, 2020

I also encountered this problem after upgrading to 4.1.x. The main issue that was causing this error in my case was.. serializing. The UUID is stored in my Security User that is serialized in Symfony's session. That Uuid comes from database, from entity's uuid_binary_ordered_time id field. When I debugged serialized and unserialized string representation (during serialization and unserialization), they were different. And that unserialized Uuid object was causing the exception in my query ->andWhere('wu.id = :id')->setParameter('id', $id, UuidBinaryOrderedTimeType::NAME).

The solution was to call toString() during serialization, and UuidV1::fromString during unserialization.

However, something was broken also in my testsuite. So only the first data set from data provider in a test that accessed a reference to a fixture object (from Reference Repository) with uuid_binary_ordered_time id worked. Any consecutive data set failed with that "Could not convert database value..." error. I guess it was caused by ProxyReferenceRepository that serializes and unserializes references between data sets or something like this.

I didn't have time to investigate it futher and downgraded to 4.0.1...

@ziarv
Copy link

ziarv commented Oct 1, 2020

@eXtreme can you tell me where to do toString() and UuidV1::fromString

@eXtreme
Copy link

eXtreme commented Oct 1, 2020

@eXtreme can you tell me where to do toString() and UuidV1::fromString

@ziarv in public function serialize() and public function unserialize($serialized) (respectively) of my security user class. The solution was not tu serialize Uuid object but just a string representation.

@ziarv
Copy link

ziarv commented Oct 1, 2020

@eXtreme thanks i will try this

@ziarv
Copy link

ziarv commented Oct 4, 2020

@eXtreme Thank you so much this worked for me

@537mfb
Copy link
Author

537mfb commented Nov 7, 2020

ok - i may have an actual solution

in vendor/ramsey/uuid-doctrine/src/UuidBinaryOrderedTimeType.php change the encode function to be the following:

private function encode(UuidInterface $uuid)
{
    $dec = $this->decode($uuid->getBytes());
    
    $this->assertUuidV1($dec);

    return $this->getCodec()->encodeBinary($dec);
}

symio added a commit to loamok/uuid-doctrine that referenced this issue Dec 5, 2020
…"..." to Doctrine Type uuid_binary_ordered_time. Expected format: UuidV1'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants