cjson parser too buggy for exchange formats (such as Journal Entry Bundles)

[martin.langhoff]

This commit ​http://git.sugarlabs.org/projects/sugar/repos/mainline/commits/ee4535c98ae74347e7072909d49dcf8a5e16ca7b switched from JSON to CJSON.

The fallout of it is that now the reader of "exchange" formats such as Journal Entry Bundles is failing to read perfectly valid files.

JEBs are sometimes produced outside of Sugar, for example in Moodle code (which is PHP). PHP has no way to produce CJSON -- the _only_ CJSON writer in existence is written in Python, and CJSON is actually an OLPC/Sugar invention.

Attached is a patch fixing simple JEB imports, but it is very limited: if the JEB has a buddies entry, the CJSON parser still kicks in and fails to parse it.

Perhaps a better fix is to just revert the patch above. I just don't know the rationale.

More info on the fallout on this OLPC bug: ​http://dev.laptop.org/ticket/9651

[martin.langhoff]

Changed summary. As Tomeu correctly points out 'import cjson' imports the C implementation of the json parser -- different from 'import bitfrost.util.cjson' which imports the Canonical json implementation.

This is a bug in cjson, the C implementation of the json parser.

the version of cjson is current
$ rpm -q python-cjson
python-cjson-1.0.5-3.fc11.i586

... in a python session:

json.loads('"foo\/bar"')

u'foo/bar'

cjson.decode('"foo\/bar"')

'foo
/bar'

[dsd]

patch

[tomeu]

-        return cjson.decode(open(metadata_path, 'r').read())
+        f = open(metadata_path, 'r')
+        try:
+            json_data = f.read()
+        finally:
+            f.close()
+        return json.read(json_data)

AFAIK, the fd is closed when the file object is released, so json_data = open(metadata_path, 'r').read() should be enough.

Otherwise, r+ for the patch, though before committing I think we should post a big notice to sugar-devel and wait 2 days for any comments.

[alsroot]

maybe instead of adding two old dependencies, add simplejson(for 2.5) and native json for(for 2.6), that have the same API.

[tomeu]

Replying to alsroot:

maybe instead of adding two old dependencies, add simplejson(for 2.5) and native json for(for 2.6), that have the same API.

Fine with me, though I don't see much problem with depending on simplejson while we don't require >2.5.

[dsd]

Closing as this is committed now. Yes, I'd also be in support of dropping the simplejson requirement on systems where the Python inbuilt json parser is available.