Skip to content

JSON parser does not spot illegal ',' leading to a crash in Matlab #533

@amwink

Description

@amwink

The settings files for ExploreASL have moved from the .m format to the .json format. To accommodate spurious occurrences of '=' for assignment, which should be ':' in proper json, we have added copied the ':' parser rule for '='.

If the ':' is replaced by a comma ',' then the JSON file is syntactically incorrect and leads to an unusable set of tokens, but is accepted by the parser of jsmn. As a result the SPM script spm_jsonread crashes Matlab during the assignment of the contents to Matlab variables.

We wrote a new parser rule in jsmn which does not allow commas unless an assignment has started (after '=' or ':'). In a purposely botched file the new parser is able to return an error when such an illegal comma is found, instead of letting Matlab crash. Unfortunately, this also returns an error in the middle of a syntactically correct JSON line.

Original JSON

{
"x": [{
		"name": "ABILITY",
		"subject_regexp": "^1\\d{2}_\\d{1}$",
		"exclusion": "",
		"SESSIONS": ["ASL_1","ASL_2"],
		"nSessions": "length(x.SESSIONS)",
		"M0": "separate_scan",
		"DummyScanPositionInASL4D": [87,88],
		"M0PositionInASL4D": [89, 90],
		"Q":
		{
			"BackgroundSuppressionNumberPulses": 0,
			"LabelingType": "CASL",
			"Initial_PLD": 1666,
			"LabelingDuration": 1500,
			"SliceReadoutTime": [0,65.5,131,196.5,262,327.5,393,0,65.5,131,196.5,262,327.5,393,0,65.5,131,196.5,262,327.5,393,0,65.5,131,196.5,262,327.5,393,0,65.5,131,196.5,262,327.5,393,0,65.5,131,196.5,262,327.5,393],
		},
		"readout_dim": "2D",
		"Quality": 1,
		"DELETETEMP": 1,
		"Vendor": "Siemens"
	}]
}

In the new parser with the '=' rule, if we replace line 7 by

		"nSessions"= "length(x.SESSIONS)",

then the parser gives a warning but the file is read correctly.
If we replace line 7 by

		"nSessions", "length(x.SESSIONS)",

then Matlab will crash during spm_jsonread but after parsing.
If we add the parser check for illegal commas then jsmn, and consequently spm_jsonread exit with an error, but then the line

			"SliceReadoutTime": [0,65.5,131,196.5,262,327.5,393,0,65.5,131,196.5,262,327.5,393,0,65.5,131,196.5,262,327.5,393,0,65.5,131,196.5,262,327.5,393,0,65.5,131,196.5,262,327.5,393,0,65.5,131,196.5,262,327.5,393],

also causes this error: after reading the number 262.

In summary, with the new parser rule for illegal commas, jsmn throws this error halfway a list of legal comma-separated numbers. (if we make the list half as long the parser behaves as expected)

For now, we have added the '=' rule but commented out the ',' rule to support arrays above a certain size.
This means the JSON reader can still crash Matlab with illegally places commas.

This probably requires a fix in jsmn, though it is not clear where. However on its GitHub page, it is stated that
... jsmn is designed to be robust (it should work fine even with erroneous data) ...
which goes against everything parsers stand for?

Release notes

Nothing. Will be done in #1311

Metadata

Metadata

Assignees

Labels

minor improvementsFixes that don't change main pipeline functionality

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions