pantheon-systems / wp-redis

WordPress Object Cache using Redis.
https://wordpress.org/plugins/wp-redis/
GNU General Public License v2.0
225 stars 67 forks source link

[BUGS-7148] Handle duplicate keys in `get_multiple` function #448

Closed Souptik2001 closed 7 months ago

Souptik2001 commented 7 months ago

Hi :wave: , Seems like the WP_Object_Cache:get_multiple function breaks when we pass duplicate keys in the $keys array argument.

When any duplicate key is passed in the array all the values of the keys coming after that duplicate gets messed up. Basically they gets shifted by one after a duplicate. Again after another duplicate is there the values after that second duplicate gets shifted by two, and similarly.

This basically happens due to the way we get the cache values from the two arrays - $remaining_keys and $results. In the results array we don't get the value twice, we just get once. But we iterate through all the $remaining_keys and just access the corresponding index of the $results array. That's why this is happening.

So, the easiest solution for this would be to just fetch the unique values in the beginning of the function only. This will not only fix the bug, but also reduce the minor redundant extra computations of the duplicate keys.

I have also added a test case, which will break if you run it by removing the fix from the object-cache.php.

I think this edge case should be handled here, because this function doesn't deny you to pass duplicate keys, but also doesn't handle duplicate keys.

Thanks! Please let me know if any other info is needed. :slightly_smiling_face:

Souptik2001 commented 7 months ago

Hi @jazzsequence any thoughts or suggestion on this one?

pwtyler commented 7 months ago

Thanks for the PR! Love to see a PR with tests 🎊. Tracking this internally as BUGS-7148. We'll get this queued for review with our team.

Souptik2001 commented 7 months ago

Thank you! :smiley: :bow:

jazzsequence commented 7 months ago

Merging this. We'll get this in the next release, likely sometime next week. Thanks @Souptik2001

Souptik2001 commented 7 months ago

Thanks @jazzsequence!