New version (v1.0.20220720) adds unreadable characters to json files

[jamielmitchell]

The new version has improper formatting in the "PrescanReuseString" line of the json files it creates.

After running dcm2niix on a macOS Big Sur version 11.6.6, the json file contains

"PrescanReuseString":"TG/s4,
RN/s1,
Coil/s1",

This leads to the file not being recognized by BIDS apps. The previous version (v1.0.20211006) doesn't even generate this field. Looks like this fields formatting is wrong. We don't use this field for our own purposes so we aren't sure what the correct formatting should be but this current version is creating an invalid json file.

[neurolabusc]

@jamielmitchell can you send an example to my institutional email? I do not have GE hardware, but legal PrescanReuseString fields are produced by our validation dataset using the current stable release (v1.0.20220720). dcm2niix does have logic to properly escape special characters. Perhaps @mr-jaemin has some thoughts.

[mr-jaemin]

Could you (@jamielmitchell or @neurolabusc) please advise how to produce the issue by BIDS apps (which apps?)? I could look into the format or special characters used in the PrescanReuseString private DICOM tag but am not familiar with requirements for the JSON file or the BIDS app. In other word, what character (e.g. newline character) was the issue?

[neurolabusc]

@mr-jaemin a guide to json escape characters is here. However, I am unable to replicate the issue and it is unclear if the issue was caused by dcm2niix or a subsequent tool.

[mr-jaemin]

@mr-jaemin a guide to json escape characters is here. However, I am unable to replicate the issue and it is unclear if the issue was caused by dcm2niix or a subsequent tool.

Thanks @neurolabusc! Makes sense and consistent with my observations. I haven't seen yet an example containing newline or line break like the above example for "PrescanReuseString" field although this is relatively new (so limited experience).

Anyway, please let me know if you see any issue with this private tag.

[neurolabusc]

@jamielmitchell thanks for sending me a sample JSON file. The JSON files created by dcm2niix are valid, and you can test your files here to confirm this. This issue must be fixed upstream by the tool that is flagging these files as invalid JSONs so it can support forward slashes. The The backslash (\) is a special character in JSON. However, the JSON spec says you CAN escape forward slash, but you don't have to. The typical reason to escape / would be because the character pair </confounds java scripts. I found a nice description of the issue here.

I have made a commit so dcm2niix escapes forward slashes. However, this only fixes the symptom, and does not treat the fact that your validator is incorrect - it will fail for any valid JSON that includes a forward slash.

Here is an example GE DICOM with the stable release:

	"PrescanReuseString": "TG/s3,RN/s1",

and the latest commit:

	"PrescanReuseString": "TG\/s3,RN\/s1",

both are valid, but the latter will not be flagged by your tool.

[mharms]

Shouldn't this be fixed at the level of the tool rather than escaping the / in the json? Are all backslashes that might occur elsewhere in the json treated in a similar manner?

[neurolabusc]

@mharms yes, my patch escapes all forward slashes in the JSONs. In my experience, backslashes are very common in DICOMs (and must be escaped), but the PreScanReuseString seems to be the instance I have seen of forward slashes. Since the escape is optional, both \/ and / are valid. However, it might cause some regression testing code to detect that a new version of dcm2niix has changed its output.

Typically @satra has nice perspectives on these issues, and I always defer to @effigies for all things BIDS. Happy to revert my commit if it is a step backwards.

[effigies]

I can't think of anything in BIDS that would direct this one way or the other. All BIDS requires is that the files be valid JSON. If the JSON spec says \/ and / both decode to / (I've quickly tested the Python JSON codec and it treats them the same), then the change should be transparent to any tool that properly decodes JSON.

So I'm completely neutral on this change to dcm2niix. I do agree with @mharms that the tools that are exhibiting problems are the better targets of this bug report.

[satra]

@neurolabusc - i agree with @mharms and think the stable release output looks good. i wouldn't unnecessarily add an escape. that should be done, if necessary by any other tool that wants to embed content in script tags or elsewhere.

[neurolabusc]

OK, well I will leave this open to see if anyone has dissenting views, but the consensus seems to be to not escape forward slashes. Perhaps related, it appears that SPM12 does escape forward slashes when converting DICOMs to JSON:

"Private_0051_1016": "M\/MB\/ND\/MOSAIC",

[mharms]

Escaping forward slashes when not necessary (for either JSON or BIDS specifications) doesn't seem advisable to me, esp. given that doing so quite likely might break existing scripts that may be parsing the JSON using simple Linux command line tools.

[neurolabusc]

Reverted behavior so slashes are not escaped. Reflects a limitation of the JSON validator, not dcm2niix. Closing issue.