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

If the query condition field is binary and not the primary key, error will be reported under some binary cases. #251

Open
vistart opened this issue Jul 25, 2022 · 4 comments
Labels
status:ready for adoption Feel free to implement this issue. type:bug Bug

Comments

@vistart
Copy link
Contributor

vistart commented Jul 25, 2022

What steps will reproduce the problem?

https://github.com/vistart/yii2-redis/blob/master/tests/data/ar/CustomerBinary.php
https://github.com/vistart/yii2-redis/blob/master/tests/data/ar/CustomerBinaryQuery.php

I modify the Customer class and added a guid field without modifying the primary key.

Then in ActiveRecordTest, save an instance of the CustomeryBinary and assign the guid to the binary form of "51a4e62e-1b1a-56c9-e9e5-9efe21f55276".

When testing the find() based on the guid value, it was found that the value was escaped in the buildHashCondition() method. The escaped result is inconsistent with the saved result.

The test results are as follows:
https://github.com/vistart/yii2-redis/runs/7493880767

All modifications are as follows:
master...vistart:yii2-redis:master

If the primary key is a binary value, this error will not be occurred because the finding process does not go through the escaping. But if the primary key is a composite fields, which contains a binary field, this error will still be triggered.

What's expected?

Can use non-primary key binary values as query conditions.

What do you get instead?

If it is a binary value, it should not be escaped.

Additional info

Q A
Yii vesion 2.0.0 or later
yii2-redis version 2.0.0 or later
PHP version 5.4 ~ 8.1
Operating system All
@samdark samdark added type:bug Bug status:ready for adoption Feel free to implement this issue. labels Jul 25, 2022
@samdark
Copy link
Member

samdark commented Jul 25, 2022

Do you have time for a pull request?

@vistart
Copy link
Contributor Author

vistart commented Jul 25, 2022

Do you have time for a pull request?

Next I will think carefully about how to solve this problem. It may take a few days.

vistart added a commit to vistart/yii2-redis that referenced this issue Jul 25, 2022
@vistart
Copy link
Contributor Author

vistart commented Jul 25, 2022

The only way I can think of now is to give up escaping \032.

@vistart
Copy link
Contributor Author

vistart commented Feb 3, 2023

Bumps!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:ready for adoption Feel free to implement this issue. type:bug Bug
Projects
None yet
Development

No branches or pull requests

2 participants