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

Manage JSON_THROW_ON_ERROR conflict #258

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

etienneroudeix
Copy link

@etienneroudeix etienneroudeix commented Dec 14, 2020

Hi,

Consider the following code using the standard json_decode PHP function

\json_decode('"incorrect json');
echo 'Error : ' . json_last_error() . PHP_EOL;
\json_decode('"correct json agin"');
echo 'Error : ' . json_last_error() . PHP_EOL;

It has the expected behavior.

Error : 3
Error : 0

Now have a look at this the following code using the standard json_decode PHP function

\json_decode('"incorrect json');
echo 'Error : ' . json_last_error() . PHP_EOL;
\json_decode('"correct json"', false, 512, JSON_THROW_ON_ERROR);
echo 'Error : ' . json_last_error() . PHP_EOL;

It has comes to a weird behavior.

Error : 3
Error : 3

Due to the above, the Safe\json_decode function has this bug :

try {
    json_decode('"incorrect json');
} catch (\Exception $e) {
    echo 'Error : ' . json_last_error() . PHP_EOL;
}
json_decode('"correct json"', false, 512, JSON_THROW_ON_ERROR);

This is throwing because of json_last_error() not being updated when using JSON_THROW_ON_ERROR.


For the record I found this bug because of a vendor using standard json_decode and then me calling safe version with JSON_THROW_ON_ERROR.

Copy link
Collaborator

@dbrekelmans dbrekelmans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you could resolve the merge conflict and fix the failing checks, I'm happy to merge this.

@@ -30,6 +30,15 @@
*/
function json_decode(string $json, bool $assoc = false, int $depth = 512, int $options = 0)
{
if ($options & 4194304) { // options has JSON_THROW_ON_ERROR
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ($options & 4194304) { // options has JSON_THROW_ON_ERROR
if ($options & JSON_THROW_ON_ERROR) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants