colinodell/json5

parse json5 codes always thorw JsonException on set JSON_THROW_ON_ERROR

inhere opened this issue · 2 comments

inhere commented

Parse json5 codes always thorw JsonException on set JSON_THROW_ON_ERROR

Detailed description

json5/src/Json5Decoder.php

Lines 75 to 80 in 7b4e50c

if (PHP_VERSION_ID >= 70000) {
$result = \json_decode($source, $associative, $depth, $options);
if (\json_last_error() === \JSON_ERROR_NONE) {
return $result;
}
}

in the code L76, if php version > 7.0 and set option JSON_THROW_ON_ERROR.

always thorw JsonException and interrupt subsequent processing

Your environment

Include as many relevant details about the environment you experienced the bug in and how to reproduce it.

  • Library version used (e.g. 1.0)
  • PHP version used (e.g. PHP 5.6, PHP 7.0): PHP 7.0+
  • Operating system and version (e.g. Ubuntu 16.04, Windows 7):

I think should auto remove JSON_THROW_ON_ERROR option on php > 7.0 in:

if (PHP_VERSION_ID >= 70000) { 
	// auto remove JSON_THROW_ON_ERROR option before call json_decode()
	
}
colinodell commented

Ah yes, I see what you're saying.

I think should auto remove JSON_THROW_ON_ERROR option on php > 7.0

My preference would be to do the opposite - always set JSON_THROW_ON_ERROR and wrap this code with a try...catch block - something like this perhaps:

        if (PHP_VERSION_ID >= 70000) {
            try {
                return \json_decode($source, $associative, $depth, $options | \JSON_THROW_ON_ERROR);
            } catch (\Throwable $t) {
                // Ignore any exceptions that occurred, as we'll attempt to parse it ourselves if standard JSON parsing fails
            }
        }

I have not tested this, but I think it should work? If you or anyone else would like to implement this change I'd gladly accept the PR :) I would only ask that the PR also include a regression test where JSON_THROW_ON_ERROR is explicitly set when calling this to ensure that this issue is indeed fixed.

  • 🎉1
inhere commented

hi, PR has been pushed #16

  • 🎉1