LibreCat / Catmandu

Catmandu - a data processing toolkit
https://librecat.org
176 stars 31 forks source link

Bug in JSON importer #103

Closed nichtich closed 9 years ago

nichtich commented 10 years ago

There is a bug in JSON importer. If JSON::XS is not installed, the JSON module uses JSON::PP which is in core since Perl 5.14 but it has a bug:

use JSON::PP ();
my $json = JSON::PP->new;
$json->incr_parse('{"foo');
my $value = $json->incr_parse; # should return undef, croaks instead

I added a test in a branch to show how this affects Catmandu: https://github.com/LibreCat/Catmandu/commit/ce4ce7a153e32400a333b8fea319fbf80a079311

Solution:

  1. Fix JSON::PP (a pull request aready exists, see https://github.com/makamaka/JSON-PP/pull/7) and wait for nex release
  2. Add this new version of JSON::PP as dependency to Catmandu

Alternative Solution (which I don't prefer because XS modules are more difficult to install):

  1. Add JSON::XS as new dependency

Without a fix, Catmandu is not able to import arbitrary JSON files but only one-liners, I'd call this a serious bug.

vpeil commented 10 years ago

Waiting for a new release of JSON-PP seems to be no option (no activity since 2013). I would consider this module as orphaned. Somebody else should take over maintenance.

vpeil commented 10 years ago

Look at this CPAN Adotion List. JSON and JSON::PP are there and a lot of other important modules, which are not up-to-date. I wonder, how the community will face the challenge...

nics commented 9 years ago

i would vote to add JSON::XS as a dep. it's already a dependency of a dependency of some Catmandu addons. YAML::XS is also already a dependency.

nics commented 9 years ago

switched to JSON::XS in df423486fa944b98acdd6b0a900b50a2ffdc24bd