Adding the following to AbstractJsonParserTests will cause all our parser implementations to fail, however, BasicJsonParser fails with an ugly NullPointerException rather than something that tells the user that the JSON is bad:

@Test
void malformedJson() {
    String input = "[tru,erqett,{\"foo\":fatrue,true,true,true,tr''ue}]";
    List<Object> list = this.parser.parseList(input);
    assertThat(list).hasSize(3);
}

Comment From: philwebb

@wilkinsona pointed out that { "foo" } is enough to trigger the NPE

Comment From: wilkinsona

YamlJsonParser is a bit funky at the moment. It parses { "foo" } into a map of:

  • foo -> null

[tru,erqett,{\"foo\":fatrue,true,true,true,tr''ue}] becomes a list of:

  • tru
  • erqett
  • A map of:
  • foo -> fatrue
  • true -> null
  • tr''ue -> null

Flagging for a team meeting as I'd like to discuss if there's anything we can and should do about this.

I've also noticed that we aren't consistently using tryParse. Doing so would mean that each JsonParser implementation would consistently throw a JsonParseException when the underlying parser throws an exception.

My changes thus far are in this branch.

Comment From: philwebb

We're going to merge this but ignore the YAML test failure. #31498 will remove the YAML parser in 3.0