ArduinoJson icon indicating copy to clipboard operation
ArduinoJson copied to clipboard

deserialize a non json string gives inconsistent error result

Open AcuarioCat opened this issue 2 years ago • 5 comments

Description Deserializing a non json string sometimes does not return a non zero error code.

Troubleshooter's report

  1. The issue happens at run time
  2. The issue concerns deserialization
  3. The program crashes
  4. Program crashes after calling deserializeJson()

Environment

  • Microcontroller: ESP32
  • Core/Framework: ESP32 2.0.3
  • IDE: Visual Micro

Reproduction code

	DynamicJsonDocument root(800);
	String s = "force an esp exception";
	DeserializationError error = deserializeJson(root, s);
	Serial.printf("Error:%d\n", error);

	s = "definitely not a json";
	error = deserializeJson(root, s);
	Serial.printf("Error:%d\n", error);

	s = "{\"value\":1,";
	error = deserializeJson(root, s);
	Serial.printf("Error:%d\n", error);

Remarks Result from above: Error:0 Error:3 Error:2

Error 0 should not be 0, should return 3

AcuarioCat avatar Jul 27 '22 04:07 AcuarioCat

Digging a little deeper, it appears the string 'force' is the culprit. 'force' returns error 0, 'forc', 'forces', 'esp force' and 'xforces' all return non zero error (as expected).

So an error code 0 only gets returned if the string starts with 'force'.

..and deeper still as this made no sense and 'force' did not occur in the source so very odd it was that simple.. A bit more testing and it seems ANY 5 letter word or sequence that starts with a lower case 'f' returns 0, irrespective of what is after the word/sequence. Upper case F returns (as expected) non zero error.

AcuarioCat avatar Jul 28 '22 08:07 AcuarioCat

Hi @AcuarioCat,

Thank you for reporting this issue.

This problem comes from the conjunction to two characteristics of the ArduinoJson parser:

  1. It stops reading as soon as it gets a complete document to allow deserialization in chunks
  2. It doesn't check for the whole "false" word, but just for the first letter, and the length; this is to reduce the code size.

Still, I consider this to be a bug, and I'll fix it in 6.20.

I see you told the Troubleshooter that your program is crashing; what's up with that?

Best regards, Benoit

bblanchon avatar Jul 28 '22 09:07 bblanchon

Thanks for confirming. It was pure chance I found the bug when trying to solve something else.

Yes it crashes as it doesn't see an error in an incomplete/incorrect json so continues. It then tries to retrieve a string value from a non-existent json element and this causes an exception. I should check for existence of the element before trying to get it's value - but I'm still developing..

Code to cause exception tested on on ESP32 and ESP32C3

` DynamicJsonDocument root(800); DeserializationError error; char mac[20]; String s;

s = "f2345 qqqqq";
error = deserializeJson(root, s);
Serial.printf("Error:%d\n", error);
byte v = root["test"];
Serial.printf("v=%d\n", v);
snprintf(mac, sizeof(mac), root["mac"]);

`

AcuarioCat avatar Jul 28 '22 16:07 AcuarioCat

To avoid the crash when the value is missing and the format string attack, you can do this:

strlcpy(mac, sizeof(mac), root["mac"] | "N/A");

See:

bblanchon avatar Jul 29 '22 07:07 bblanchon

Great, thanks for the guidance.

AcuarioCat avatar Jul 29 '22 08:07 AcuarioCat

This fix was published in ArduinoJson 6.20.0.

bblanchon avatar Dec 26 '22 16:12 bblanchon