Fix logical bug in decodeJson() function

[crtlib]

😬

[bzikarsky]

While the description is not descriptive, this commit contains a rather important fix in regards to JSON unpacking. We ran into fun binary-soup-strings without this fix.

[kelunik]

Could you please add a test case?

[crtlib]

Added a simple test for decodeJson() function. Is it ok @kelunik ?

[kelunik]

I was thinking about an integration test reading a json value from the database.

[bzikarsky]

I am not sure whether MySQL/MariaDB can be coerced to sending base64 (or non-base64) JSON on purpose. WDYT @crtlib -you triaged this so far.

[crtlib]

@bzikarsky Hm, good question. Unfortunately, I can't find anything about it on the web 😒
The test I've just pushed is failing for me on MySQL 8.0.22 (without the actual fix).
@kelunik Is it good enough to merge? Or maybe you can help with forcing base64 encoding, so I can enhance the test?

[bzikarsky]

Can you add a second testcase which runs this against the CI's integration test databases? Even if it's sometimes a NOOP, it's still valuable.

[R4c00n]

@bzikarsky Any hints on how to run it agains CI integration test database? Would really like to see this merged.

[bzikarsky]

What do you mean? If you look at the existing test code, you can see it already connects to a test database. The travis scripts install a defined version of MariaDB. It's coming down to bootstrapping a test case with a JSON column and querying it.

Btw. I am not a maintainer and just work in the same org as @crtlib. I'd also like to see this merged. 😉

[kelunik]

Looking at the code, it's obviously wrong, so I've merged this now. Sorry for the long delay, will prepare a release soon.

[R4c00n]

@kelunik Nice, thanks!

[kelunik]

https://github.com/amphp/mysql/releases/tag/v2.1.2 has been tagged now! 🚀