parse-string should not accept trailing junk

[dakrone]

parse-string should not accept trailing junk

[dakrone]

I looked into this, I think it's Jackson actually consuming the rest of the stream.

I checked that the StringReader had been consumed by calling (.read string-reader) and verifying it was still -1 after parsing, even with trailing junk.

I also double-checked with another project that uses Jackson, and it ignores trailing junk also.

Is the trailing data causing problems for you?

[cespare]

I looked into this, I think it's Jackson actually consuming the rest of the stream.

I checked that the StringReader had been consumed by calling (.read string-reader) and verifying it was still -1 after parsing, even with trailing junk.

Indeed. This solution was a guess, and it wasn't the right approach. I looked into it more closely and I'm pretty sure the answer is that you need to check that (.nextToken jp) is nil at the end of parse/parse. Here's a REPL session to show what I mean.

user=> (def jp (.createJsonParser (JsonFactory.) (StringReader. "{\"foo\": 123}")))
#'user/jp
user=> (.nextToken jp)
#<JsonToken START_OBJECT>
user=> (.nextToken jp)
#<JsonToken FIELD_NAME>
user=> (.nextToken jp)
#<JsonToken VALUE_NUMBER_INT>
user=> (.nextToken jp)
#<JsonToken END_OBJECT>
user=> (.nextToken jp)
nil

user=> (def jp (.createJsonParser (JsonFactory.) (StringReader. "{\"foo\": 123}null")))
#'user/jp
user=> (.nextToken jp)
#<JsonToken START_OBJECT>
user=> (.nextToken jp)
#<JsonToken FIELD_NAME>
user=> (.nextToken jp)
#<JsonToken VALUE_NUMBER_INT>
user=> (.nextToken jp)
#<JsonToken END_OBJECT>
user=> (.nextToken jp)
#<JsonToken VALUE_NULL>

user=> (def jp (.createJsonParser (JsonFactory.) (StringReader. "{\"foo\": 123}junk")))
#'user/jp
user=> (.nextToken jp)
#<JsonToken START_OBJECT>
user=> (.nextToken jp)
#<JsonToken FIELD_NAME>
user=> (.nextToken jp)
#<JsonToken VALUE_NUMBER_INT>
user=> (.nextToken jp)
#<JsonToken END_OBJECT>
user=> (.nextToken jp)

JsonParseException Unrecognized token 'junk': was expecting ('true', 'false' or 'null')
 at [Source: java.io.StringReader@58a606e3; line: 1, column: 33]  com.fasterxml.jackson.core.JsonParser._constructError (JsonParser.java:1524)

I also double-checked with another project that uses Jackson, and it ignores trailing junk also.

Yeah, as I mentioned I saw that data.json and clj-json have this same behavior as well. It's easy for this to happen when building a batch parsing function from a streaming parser.

Is the trailing data causing problems for you?

Yes, we parse a very large amount of JSON data and we see all manner of corruption.

[cespare]

Any updates?

[dakrone]

@cespare I could add a wrapper around the regular parsing functions that would verify the nil on .nextToken after parsing has completed, then it could optionally be used (to not incur the overhead for everyone, just people who need it)

Does that sound like it would work for you?

[cespare]

Sure, that would let us fix this problem.

This should not be a wrapper, it should be the default behavior. It's misleading that parse-string only looks for a valid JSON prefix in the given string. The extra cost is very small compared with doing JSON parsing in the first place, and well worth avoiding the confusion.

If the current behavior is somehow useful, then perhaps the function should be named parse-json-prefix. More likely to be useful would be an API for reading successive JSON values from a reader (the current parse-string cannot do that because the StringReader is just thrown away after the first value is read).

[philc]

Any update on this? Would love to see a fix.

[Satshabad]

Also would love to see a fix

[mkwatson]

Me too! This was very surprising!

[overthink]

+1 Just bit me also.

[dakrone]

Closed by #122

[cespare]

I still think it's misleading that the default parse-string function accepts a string that's invalid JSON, but at least there's a workaround for people after they're bitten by this :|