OutOfMemory bug and fix (bug in JSONReader causing an OutOfMemory exception)

[GoogleCodeExporter]

http://jsonplugin.googlecode.com/svn/trunk/src/main/java/com/googlecode/jsonplug
in/JSONReader.java

This is the bug:

private char next() {
        this.c = this.it.next();

        return this.c;
    }

It should be:
private char next() {
        assert c!=CharacterIterator.DONE; // I do the assert before calling
next() on purpose, because for legal JSON strings, the parser calls next()
one extra time
        c = it.next();
        return c;
    }

Example of code that fails:

        JSONReader reader = new JSONReader();
        return reader.read("\"never ending string");

Original issue reported on code.google.com by Yoav.Zibin@gmail.com on 2 Jul 2009 at 8:00

[GoogleCodeExporter]

While the patch shown does catch that particular problem, there is a larger 
issue to
consider.

The Stringtree JSON parser, which is being used in this case, comes as 
(essentially)
two classes JSONReader and JSONValidator. They both have different tasks to 
perform
and are optimised for their tasks. 

JSONReader is optimised for speed, low memory footprint, and accuracy when given
valid JSON.

JSONValidator is optimised for catching all forms of invalid JSON, and attempts 
to
give a useful and informative error message in the great majority of cases.

By design, JSONReader does *NO* validation at all. It works only for valid 
JSON. This
is so that there is no validation code to get in the way of raw speed in the 
common
case of machine-generated, guaranteed-valid input.

If you are unsure about the validity of supplied JSON, you should run the
JSONValidator first, and only pass the data on to the JSONReader if it is 
valid. To
make this simpler, Stringtree JSON also includes a JSONValidatingReader class, 
which
extends JSONReader to do just this.

In short, if your code is ever likely to be given invalid JSON of any form, you
should always use JSONValidatingReader rather than JSONReader. Only use the 
"raw"
JSONReader if you are sure that you are always supplied with valid JSON markup.

For interest, the case of the never ending string is already in the 
JSONValidator
test cases. See:
http://stringtree.svn.sourceforge.net/viewvc/stringtree/trunk/src/test/java/test
s/json/JSONValidatorInvalidStringTest.java?revision=175&view=markup

I hope that makes sense.

Original comment by frank.ca...@googlemail.com on 22 Jul 2009 at 11:24

[GoogleCodeExporter]

sure, thanks! I'll use the validator.
But I still think that JSONReader should not get into an infinite loop in case 
of
wrong input (it is ok to be like "iterators" and fail unexpectedly, but 
infinite loop
is something that can kill your application completely...)

Original comment by Yoav.Zibin@gmail.com on 22 Jul 2009 at 11:53

[GoogleCodeExporter]

thanks for reporting, are you having this problem with the last version? I 
think this
was fixed in r60 and this works for me:

 public void testInfiniteLoop2() throws JSONException {
        try {
            JSONReader reader = new JSONReader();
            reader.read("\"never ending string");
        } catch (JSONException e) {
            //I can't get JUnit to ignore the exception
            // @Test(expected = JSONException.class)
        }
    }

Original comment by musa...@gmail.com on 28 Jul 2009 at 4:32

• Changed state: Duplicate