Skip Menu |

This queue is for tickets about the JSON CPAN distribution.

Report information
The Basics
Id: 29139
Status: resolved
Priority: 0/
Queue: JSON

People
Owner: Nobody in particular
Requestors: DAMI [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: 1.14
Fixed in: (no value)



Subject: unary boolean negation generates illegal JSON
The result of print JSON->new->objToJson({foo => !1}); generates {"foo":} which of course is illegal JSON. As far as I can tell this comes from line 206 in JSON::Converter, where inspection of B::FLAGS incorrectly decides NOT to _stringify the $value. But I have no idea why these low-level flags need to be inspected, so I cannot propose a patch.
From: james.farrell [...] ticketmaster.com
We have encountered this problem as well. The statement that returns the empty value is located in JSON::Converter, line 206-207, in _valueToJson: return $value # as is if ($b_obj->FLAGS & B::SVf_IOK or $b_obj->FLAGS & B::SVf_NOK); The problem seems to arise because Perl doesn't have a boolean data type, so the value of !1 can be interpreted as both the number 0 or as the empty string, as illustrated by Devel::Peek: jfarrell@fnord:~$ perl -MDevel::Peek -e 'Dump(!1);' SV = PVNV(0x607128) at 0x603b18 REFCNT = 2147483647 FLAGS = (PADBUSY,PADTMP,IOK,NOK,POK,READONLY,pIOK,pNOK,pPOK) IV = 0 NV = 0 PV = 0x6070e0 ""\0 CUR = 0 LEN = 8 The purpose of lines 206-207 appears to be to determine if $value is a number, in which case either one of the following two modifications would likely solve the problem: return $value + 0 # as is if ($b_obj->FLAGS & B::SVf_IOK or $b_obj->FLAGS & B::SVf_NOK); or return $value # as is if (($b_obj->FLAGS & B::SVf_IOK or $b_obj->FLAGS & B::SVf_NOK) and !( $b_obj->FLAGS & B::SVf_POK )); In the first case, {foo=>!1} would produce the JSON string '{"foo":0}' In the second case, the JSON string would be '{"foo":""}' Both of these seem acceptable. The optimal JSON version of {foo=>!1} is probably '{"foo":false}', but given Perl's lack of a Boolean type, I'm not sure how this could be produced, or whether it even should be.
The attached patch resolves the issue and adds tests to confirm the fix.
diff -urw JSON-1.14/lib/JSON/Converter.pm JSON-1.14-devel1/lib/JSON/Converter.pm --- JSON-1.14/lib/JSON/Converter.pm 2007-05-05 21:15:33.000000000 -0700 +++ JSON-1.14-devel1/lib/JSON/Converter.pm 2007-10-16 08:28:12.000000000 -0700 @@ -204,7 +204,8 @@ my $b_obj = B::svref_2object(\$value); # for round trip problem # SvTYPE is IV or NV? return $value # as is - if ($b_obj->FLAGS & B::SVf_IOK or $b_obj->FLAGS & B::SVf_NOK); + if (($b_obj->FLAGS & B::SVf_IOK or $b_obj->FLAGS & B::SVf_NOK) and + !($b_obj->FLAGS & B::SVf_POK )); # Fix for rt.cpan.org #29139 return _stringfy($value); } diff -urw JSON-1.14/t/02-base.t JSON-1.14-devel1/t/02-base.t --- JSON-1.14/t/02-base.t 2007-05-05 21:52:32.000000000 -0700 +++ JSON-1.14-devel1/t/02-base.t 2007-10-16 08:29:07.000000000 -0700 @@ -1,6 +1,6 @@ use Test::More; use strict; -BEGIN { plan tests => 60 }; +BEGIN { plan tests => 64 }; use JSON; ######################### @@ -124,6 +124,19 @@ $obj = jsonToObj($js); is($obj->[0],"\e"); +# Next 4 tests related to rt.cpan.org #29139 +$obj = [!1]; +$js = objToJson($obj); +is( $js, '[""]' ); +$obj = jsonToObj($js); +is($obj->[0],!1); + +$obj = [!!2]; +$js = objToJson($obj); +is( $js, '[1]' ); +$obj = jsonToObj($js); +is($obj->[0],!!2); + $js = '{"id":"}'; eval q{ jsonToObj($js) }; #jsonToObj($js);
From: roman.yepishev [...] gmail.com
This has been broken since 1.12, after introduction of low level type checks so all versions starting from 1.12 are affected.
From: jeffrey.klein [...] priorityhealth.com
On Tue Oct 16 11:34:21 2007, jfarrell wrote: Show quoted text
> The attached patch resolves the issue and adds tests to confirm the
fix. The problem is that boolean tests return false as the special value PL_sv_no, which is an empty string with IOK|NOK set so that is can be used as a number without warnings. For the round-trip issue, the solution should ensure that JSON::Converter only encodes values as numbers that JSON::Parser will decode as numbers. JSON::Parser::value calls number() if the first character is '-' or a digit, so this patch preserves that. No idea how to handle '0 but true', though...
--- lib/JSON/Converter.pm 2007-05-06 00:15:33.000000000 -0400 +++ ./Converter.pm 2007-11-09 10:00:25.000000000 -0500 @@ -204,7 +204,8 @@ my $b_obj = B::svref_2object(\$value); # for round trip problem # SvTYPE is IV or NV? return $value # as is - if ($b_obj->FLAGS & B::SVf_IOK or $b_obj->FLAGS & B::SVf_NOK); + if ($b_obj->FLAGS & B::SVf_IOK or $b_obj->FLAGS & B::SVf_NOK) + and $value =~ /^[\d\-]/; # JSON::parse will decode as number return _stringfy($value); }
Fixed by 1.15. I modified the code: return $value # as is if ( ($b_obj->FLAGS & B::SVf_IOK or $b_obj->FLAGS & B::SVp_IOK or $b_obj->FLAGS & B::SVf_NOK or $b_obj- Show quoted text
>FLAGS & B::SVp_NOK
) and !($b_obj->FLAGS & B::SVf_POK ) ); Its result: objToJson([!1]) => [""] objToJson([!!1]) => ["1"] Thanks for reports and patches!