Compression fails if filename contains $ about dcm2niix HOT 16 CLOSED

@pwrightkcl as a temporary fix, you could run -z i instead of -z y to use the internal miniz or zlib compressor rather than the external pigz compression. @satra this is related to issue 398 as well as earlier discussions where dcm2niix's censoring of file naming was made more permissive. Do you think filenaming should be modified to censor the $ character?

from dcm2niix.

In pre-history, dcm2niix was very restrictive for filenaming characters (similar to BIDS) but a consensus was formed to allow all characters that were legal for file names in all operating systems. On the other hand, David Wheeler's section 7 notes we should be cautious about a wider range of control characters: $*?:[]"<>|(){}&'!\;.

While the discussion is lost (as we moved dcm2niix from my private github to a github organization), the community did decide that dcm2niix should try to preserve as much information as possible, at the risk of outcomes like yours. So I think this is a result of a known tradeoff that the community made in the design of dcm2niix. In my memory @satra made a strong case for not censoring filenames.

from dcm2niix.

I don't think the $ should be censored (having ready the previous discussion), but maybe inputs to pigz can be single-quoted or escaped if they contain characters like this.

from dcm2niix.

@pwrightkcl why don't you try my latest commit to the development branch. It uses single rather than double quotes to wrap filenames. This works on my MacOS computer. I am on the road, but we should make sure this also works for Windows computers that might have different conventions.

from dcm2niix.

@neurolabusc I compiled on my Ubuntu 22.04 machine and it successfully compressed the file with $1 in the name. It added 'r4527.34' to the end, though. Is that because I'm using the development version?

./dcm2niix -o $output -f test_%t_%s_%d -z y $input

Chris Rorden's dcm2niiX version v1.0.20230904  (JP2:OpenJPEG) (JP-LS:CharLS) GCC11.4.0 x86-64 (64-bit Linux)
Found 22 DICOM file(s)
Image Decompression is new: please validate conversions
Philips Scaling Values RS:RI:SS = 2.46496:0:0.00581155 (see PMC3998685)
Convert 22 DICOM as /data/tmp/dcm2niix_dollar_test/test_20220202090000_501_Brain_T2W_TSE$1_r4527.34 (256x256x22x1)
Compress: "/usr/local/bin/pigz" -b 960 -n -f -6 '/data/tmp/dcm2niix_dollar_test/test_20220202090000_501_Brain_T2W_TSE$1_r4527.34.nii'
Conversion required 1.628203 seconds (0.452984 for core code).

from dcm2niix.

@pwrightkcl that sounds like an unintended consequence of handling issue 743. I have very limited access to Philips MR data - is it possible to share a troublesome dataset with my institutional email?

from dcm2niix.

@neurolabusc link sent.

from dcm2niix.

@pwrightkcl can you test the latest commit to the development branch.

from dcm2niix.

@pwrightkcl can you confirm that the latest commit resolves your issues?

from dcm2niix.

Sorry for delay. Yes, @neurolabusc the new commit converts the DICOMs with $ in the series description with compression and does not add the r... suffix to the end.

from dcm2niix.

Unfortunately, the change introduces an error with a different dataset that includes single quotes in the series description and therefore in the filename:

sh: 1: Syntax error: Unterminated quoted string
Compress: "/usr/local/bin/pigz" -b 960 -n -f -6 './test_anondate_16_Sag_T2_frFSE_POST_GAD'.nii'

This affected only 7 scans in the 1.2 million I'm processing right now. I wonder if the radiographer is like this Mom:

https://xkcd.com/327/

from dcm2niix.

@sarta as you were a prime advocate for current permissive file naming scheme. Do you have your thoughts about this? Do we forbid $ as it can be interpreted as a control character, or do we allow it but forbid single quotes, or do we add a lot of extra logic to wrap control characters?

from dcm2niix.

I may be wrong, but based on the edits I've done to my own scripts processing these images, I think as long as you properly escape single quotes, then single quoting the rest of the filename will handle any other control characters.

from dcm2niix.

I agree. However, if we do not censor these characters one fears unintended consequences for subsequent tools and scripts. For example, the BIDS standard strictly limits the character set for entities, as they MUST only consist of letters and/or numbers (other characters, including spaces and underscores, are not allowed. I worry that letting the troublesome $ character through can cause unintended consequences down stream.

from dcm2niix.

I agree, but I think it's the responsibility of whoever writes subsequent tools and scripts to handle dodgy inputs. I expect you have more specific examples in mind, though, and have a much broader knowledge than I do about the possibities, so this is just my limited view.

In my use case, I know my clinical source data will have illegal characters and missing essential data. I wouldn't expect any off-the-shelf code to be able to make these BIDS-compliant, which is why I had to write my own.

from dcm2niix.

@pwrightkcl please test the latest commit. My original change had unintended consequences as for the Windows Command Prompt, only double quotes are interpreted correctly. Therefore, I have reverted to double quotes and the dollar sign becomes a forbidden character (it will be replaced with an underscore).

For future reference, one can replicate this issue into any DICOM by adding a dollar sign in the description and then asking dcm2niix to use the description in the filename:

dcmodify -m '(0008,103E)=de$cription' file.dcm
./dcm2niix -f %d -z y /path/to/file.dcm

from dcm2niix.