All Astronauts Posted December 15, 2020 Share Posted December 15, 2020 /** * Decode JSON * * @param bool $asArray Whether to decode as an array or not * @return array * @throws \RuntimeException * @li BAD_JSON */ public function decodeJson( $asArray=TRUE ) { $json = json_decode( $this->content, $asArray ); if ( $json === FALSE ) { throw new \RuntimeException('BAD_JSON'); } return $json; } You are testing for a return value of FALSE and that is not returned by PHP json_decode. If the content to be decoded cannot be decoded (malformed json, not json) that method returns null. null is returned if the json cannot be decoded or if the encoded data is deeper than the nesting limit. If you would rather not swap that FALSE out for null, @Colonel_mortis suggests JSON_THROW_ON_ERROR flag in json_decode as work around. CoffeeCake 1 Link to comment Share on other sites More sharing options...
Solution bfarber Posted December 16, 2020 Solution Share Posted December 16, 2020 I've raised your suggestion internally for further consideration in a future release. Have you been directly impacted by this issue in some way? Link to comment Share on other sites More sharing options...
All Astronauts Posted December 16, 2020 Author Share Posted December 16, 2020 IPS asks us to use internal methods when possible. Decoding malformed/bad json with this method lets it through as "valid". Have I been directly impacted by this? Not much - I saw the problem and wrote around it - it's trivial code and a trivial fix (though expanding it with try catch and pushing out the actual JSON errors ala @Colonel_mortis 's suggestion is a better solution) As it stands now the fix for this method is just $json === FALSE >>> $json === null. I did a bit more myself. This ain't a big deal, but it is broken. A different/better/somewhat related question is whether apps with API structure are going to run into the same problem files in the interface directory have on the new CIC2 structure. You guys said interface is alright now but is the API directory also in the clear? Link to comment Share on other sites More sharing options...
Recommended Posts