Strict mode for JSON parsing

[eamonnmcmanus]

As noted in the documentation for JsonReader.setLenient, there are a number of areas where Gson accepts JSON inputs that are not strictly conformant, even when lenient mode is off:

• JsonReader allows the literals true, false and null to have any capitalization, for example fAlSe
• JsonReader supports the escape sequence \', representing a '
• JsonReader supports the escape sequence \LF (with LF being the Unicode character U+000A), resulting in a LF within the read JSON string
• JsonReader allows unescaped control characters (U+0000 through U+001F)

To which we could add that it allows a trailing comma in JSON arrays, for example ["foo", "bar",] as noted in #494.

We could imagine that GsonBuilder would acquire a method setStrict(), mutually exclusive with setLenient() and that JsonReader would likewise acquire setStrict(boolean) and isStrict(). We can't make strict mode the default, for fear of breaking existing users, but we could recommend it.

[Marcono1234]

that it allows a trailing comma in JSON objects arrays¹

As mentioned in #494 (comment) that only seems to apply to lenient mode.

Footnotes

1. Trailing commas in JSON objects always cause a MalformedJsonException for me, regardless of whether the reader is lenient or not. ↩

[eamonnmcmanus]

Thanks @Marcono1234, I've updated the earlier comment to talk about JSON arrays rather than objects.

[eamonnmcmanus]

Now I'm very confused. Apparently trailing commas are already disallowed in both JSON objects and JSON arrays? So the list quoted in my earlier comment is correct without addition.

Anyway I still do think we should have a strict mode, that you can easily get using GsonBuilder.

[marten-voorberg]

My project group and I will try to resolve this issue for a software engineering course at KTH, Sweden.

What should be the interaction between setLenient and setStrict? It seems very strange to us to be able to set both flags on the same Gson instance. Should we throw an exception (maybe IllegalStateException) if you try to set both of these flags?

[eamonnmcmanus]

@marten-voorberg, thanks for the proposal!

I think there are several things that need to happen here, not necessarily in order:

• Determine the things that are accepted today even in non-lenient mode but that are not strictly conformant to the JSON spec. It may well be that the list in JsonReader.setLenient is complete, but it would be worth studying the ECMA specification closely. (It's fairly short.)
• Write test cases for these things. I think some of them are already tested, with comments to the effect of "this should not be accepted but is". To start with, new test cases will presumably assert that a construct can be parsed, even though eventually we want to check that it can't in strict mode.
• Decide what the API should be that allows us to turn on strict parsing. It will probably include changes to both GsonBuilder and JsonReader.
• Implement the API and revise the tests so they cover strict mode correctly.

I've created a branch strictness to which you can send pull requests. That way the mainline won't have partial changes, but also you don't have to do everything in one giant PR.

Concerning the API, I am actually thinking it might be cleaner to use an enum. It might be enum Strictness {LENIENT, DEFAULT, STRICT}, for example. Then GsonBuilder and JsonReader could have something like setStrictness(Strictness). The existing setLenient() would be equivalent to setStrictness(Strictness.LENIENT), and setLenient(boolean lenient) would be equivalent to setStrictness(lenient ? Strictness.LENIENT : Strictness.DEFAULT).

Another more general approach would be to have an enum for the various areas of leniency, for example comments; arbitrary capitalization (fAlse); nonstandard escape sequences. It might look something like this:

public enum Leniency {
  COMMENTS, CAPITALIZATION, ESCAPES, ...;

  public static final Set<Leniency> LENIENT = EnumSet.of(...);
  public static final Set<Leniency> DEFAULT = EnumSet.of(...);
  public static final Set<Leniency> NONE = EnumSet.ofNone(Leniency.class);
}

Then setLenient() would be equivalent to setLeniency(Leniency.LENIENT), etc. We would have to look at everywhere that the code currently checks the lenient boolean and classify it into one or more of the new enum constants.

I'm not sure that this extra generality would really be worthwhile. It would allow users to specify exactly which non-standard constructs they need to accept, while rejecting everything else. But I'm inclined to think you either accept the exact standard or you accept any random crap (which is what lenient mode means today). The main reason for DEFAULT is that the current non-lenient mode is not strict enough.

[marten-voorberg]

@eamonnmcmanus Thanks for the response!

I've submitted a WIP PR which throws exceptions when trying to parse any of the non-spec compliant values discussed in the issue description.

The API for setStrict in this PR is definitely going to change. I think your suggestions for an enum Strictness {LENIENT, STRICT, DEFAULT} is a very good way to do this. I also agree with you that the the more general approach a leniency enum for (dis)allowing each non-spec compliant feature separately seems a bit overkill.

[Marcono1234]

@marten-voorberg, maybe it would also be interesting to add unit tests for the following:

• U+2028 and U+2029 handling
  ECMAScript considers them line terminators but JSON does not. So "\"\u2028\u2029\"" should be considered valid JSON, and "[\u2028]" respectively "[\u2029]" should be considered invalid (all written as Java string literals here).
• Other control characters handling
  There are actually more control characters than defined in the JSON specification. However, since the JSON specification does not list them, they should be allowed unescaped in string values. For example the following should be considered valid JSON: "\"\u007F\u009F\""

[marten-voorberg]

The JsonWriter also has a lenient field. I think it makes sense to also give the JsonWriter a strictness which exhibits the following behavior:

• Strictness.DEFAULT: The behavior is the same as the current lenient = false behavior.
• Stictness.LENIENT: The same behavior as lenient = true currently has, namely allowing infinity and NaN to be written.
• Strictness.STRICT: Currenlty, the reader will escape '\u2028' and '\u2029' but according to the spec, these should not be escaped. I propose that these characters will not be escaped in strict mode.

[Marcono1234]

Currenlty, the reader will escape '\u2028' and '\u2029' but according to the spec, these should not be escaped.

That behavior is actually allowed by the specification, see RFC 8259, section 7¹:

Any character may be escaped.

Maybe there are use cases where users don't want \u2028 and \u2029 to be escaped, but I would assume they are rather rare, and it would also not be related to strict mode (related: #1984).

Footnotes

1. While RFC 8259 and ECMA-404 are supposed to be equivalent (see RFC section 1.2), it would probably be better to stick to the RFC document since that is what documentation of Gson refers to already (though it refers to older RFCs). In case someone is interested in this blog post Tim Bray, the editor of the latest JSON RFCs, explains (subjectively?) the historical reason for ECMA-404. ↩

[eamonnmcmanus]

I agree with @Marcono1234: we can escape \u2028 and \u2029 always. The point about Strictness applying to writes as well as reads is a good one, but for now it doesn't look as if there's any difference between DEFAULT and STRICT there. Nevertheless for consistency I think we should make it so jsonWriter.setLenient(b) is equivalent to jsonWriter.setStrictness(b ? LENIENT : DEFAULT) and so the lenient field is replaced by strictness.

I also agree that RFC 8259 is a better reference than the ECMA spec I linked to earlier. Only one small caveat: there is an erratum with a grammar that seems to suggest that / must be escaped in string literals, and that's clearly wrong. The remaining errata don't affect Gson as far as I can see.

I also found this excellent resource about tricky areas in JSON parsing, including an excellent set of tests.