Doesn't quote paths in the invocation of dcm2niix - directory with a space makes execution fail

[yarikoptic]

Summary

Originally reported by a fella co-developer against heudiconv that it crashed when path contained a space in it.

In heudiconv we depend on nipype to just run dcm2niix interface: https://github.com/nipy/heudiconv/blob/master/heudiconv/convert.py#L803

Actual behavior

*(Pdb) p convertnode.inputs

anon_bids = <undefined>
args = <undefined>
bids_format = True
comment = <undefined>
compress = y
compression = <undefined>
crop = False
environ = {}
has_private = False
ignore_deriv = <undefined>
merge_imgs = False
out_filename = sub-b0dwiForFmap_acq-b0dwi_epi_heudiconv133
output_dir = /tmp/tr icky/pytest-of-yoh/pytest-1/test_b0dwi_for_fmap0/sub-b0dwiForFmap/fmap
philips_float = <undefined>
series_numbers = <undefined>
single_file = False
source_dir = <undefined>
source_names = ['/home/yoh/proj/heudiconv/heudiconv-master/heudiconv/tests/data/b0dwiForFmap/b0dwi_for_fmap+00001.dcm', '/home/yoh/proj/heudiconv/heudiconv-master/heudiconv/tests/data/b0dwiForFmap/b0dwi_for_fmap+00002.dcm', '/home/yoh/proj/heudiconv/heudiconv-master/heudiconv/tests/data/b0dwiForFmap/b0dwi_for_fmap+00003.dcm']
to_nrrd = <undefined>
verbose = False

observe that output_dir has space in /tmp/tr icky and then running that node kabooms:

(Pdb) convertnode.run()
230915-17:50:37,584 nipype.workflow INFO:
	 [Node] Setting-up "convert" in "/tmp/tr icky/dcm2niixiftfysju/convert".
230915-17:50:37,588 nipype.workflow INFO:
	 [Node] Executing "convert" <nipype.interfaces.dcm2nii.Dcm2niix>
230915-17:50:37,633 nipype.interface INFO:
	 stdout 2023-09-15T17:50:37.633592:Chris Rorden's dcm2niiX version v1.0.20220720  (JP2:OpenJPEG) GCC13.2.0 x86-64 (64-bit Linux)
230915-17:50:37,633 nipype.interface INFO:
	 stdout 2023-09-15T17:50:37.633592:Warning: only processing last of 2 input files (recompile with 'myEnableMultipleInputs' to recursively process multiple files)
230915-17:50:37,633 nipype.interface INFO:
	 stderr 2023-09-15T17:50:37.633709:Error: Input folder invalid: icky/dcm2niixiftfysju/convert
230915-17:50:37,645 nipype.workflow INFO:
	 [Node] Finished "convert", elapsed time 0.020344s.
230915-17:50:37,646 nipype.workflow WARNING:
	 [Node] Error on "convert" (/tmp/tr icky/dcm2niixiftfysju/convert)
*** nipype.pipeline.engine.nodes.NodeExecutionError: Exception raised while executing Node convert.

Cmdline:
	dcm2niix -b y -z y -x n -t n -m n -f sub-b0dwiForFmap_acq-b0dwi_epi_heudiconv133 -o /tmp/tr icky/pytest-of-yoh/pytest-1/test_b0dwi_for_fmap0/sub-b0dwiForFmap/fmap -s n -v n /tmp/tr icky/dcm2niixiftfysju/convert
Stdout:
	Chris Rorden's dcm2niiX version v1.0.20220720  (JP2:OpenJPEG) GCC13.2.0 x86-64 (64-bit Linux)
	Warning: only processing last of 2 input files (recompile with 'myEnableMultipleInputs' to recursively process multiple files)
Stderr:
	Error: Input folder invalid: icky/dcm2niixiftfysju/convert
Traceback:
	RuntimeError: subprocess exited with code 5.
(Pdb) import nipype 
(Pdb) nipype.__version__
'1.8.6'

Expected behavior

to not crash

How to replicate the behavior

get heudiconv env, run tests with TMPDIR pointing to a folder with a space in the filename

mkdir /tmp/tr\ icky
TMPDIR=/tmp/tr\ icky python -m pytest -s -v heudiconv/tests/test_convert.py

https://docs.python.org/3/library/shlex.html#shlex.quote might be of help to quote individual args for POSIX shells if cannot be just passed as a list and shell invocation need to be composed (didn't look what is happening here)

[yarikoptic]

FWIW, I would recommend in the tests to add some matrix run or just generally create a tmp folder with a space name in and run tests under it. In DataLad we even manufacture the trickiest filename supported on the filesystem to use in some tests: https://github.com/datalad/datalad/blob/maint/datalad/tests/utils_pytest.py#L1451 , e.g. on my system

❯ python -c 'from datalad.tests.utils_pytest import OBSCURE_FILENAME; print(repr(OBSCURE_FILENAME))'
' |;&%b5{}\'"<>ΔЙקم๗あ .datc '

;-)

[neurolabusc]

dcm2niix issue 749 is related to this topic, and it might be worth developing in that consensuscharacters dcm2niix censors from filenames. In particular, in section 7 David Wheeler suggests danger for the characters $*?:[]"<>|(){}&'!\;. It is worth noting that BIDS strictly limits entity names as the MUST only consist of letters and/or numbers (other characters, including spaces and underscores, are not allowed. My own sense is that dcm2niix should be more agressive in censoring filenames, as these evoke unexpected behaviors in many scripts.