From ab8f591fd9c32144ac6e5dda9996ff37fccb3f34 Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Fri, 18 Sep 2026 22:11:47 -0400 Subject: [PATCH 1/2] BUG: Do not write to the stream that failed to open open_write_stream() returns NULL when its fopen() fails, and reports it. Both callers then used the result without testing it. $ cifti_tool -input c.nii -disp_cext -output /nonexistent-dir/x.txt ** failed to open '/nonexistent-dir/x.txt' for writing Segmentation fault (core dumped) disp_cifti_extension() passes it to fprintf(). eval_cifti_extension() hands it to axml_set_wstream(), so the NULL becomes the xml library's write stream and every later write follows it. Both now return 1; the message has already been printed. This file is behind USE_CIFTI_CODE, which defaults to OFF, so the standard workflows do not compile it. Verified with -DUSE_CIFTI_CODE=ON. (cherry picked from commit f368af9a57106c46ecd7c738cf07817e9a7ef170) --- cifti/cifti_tool.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cifti/cifti_tool.c b/cifti/cifti_tool.c index 718b610f..8714d83c 100644 --- a/cifti/cifti_tool.c +++ b/cifti/cifti_tool.c @@ -219,6 +219,7 @@ int disp_cifti_extension(nifti_image * nim, opts_t * opts) } fp = open_write_stream(opts->fout); + if( !fp ) return 1; /* open_write_stream() has reported the failure */ if( !ext ) { fprintf(fp, "** no CIFTI extension in %s\n",nim->fname?nim->fname:"NULL"); close_stream(fp); @@ -242,6 +243,7 @@ int eval_cifti_extension(afni_xml_t * ax, opts_t * opts) opts->fout ? opts->fout : "DEFAULT" ); fp = open_write_stream(opts->fout); + if( !fp ) return 1; /* open_write_stream() has reported the failure */ axml_set_wstream(fp); if( opts->verb > 1 ) fprintf(stderr, "-- recursive eval from %s\n", From 91e537ab15649bbecba8014a53ded005afc92738 Mon Sep 17 00:00:00 2001 From: "Hans J. Johnson" Date: Tue, 22 Sep 2026 08:53:21 -0500 Subject: [PATCH 2/2] ENH: Cover the unopenable -output path in cifti_tool Driven through a cmake -P script rather than add_test, because PASS_REGULAR_EXPRESSION ignores the exit status and a crash that had already printed the diagnostic would pass. The script pins that the tool exits normally and reports the failure, and that an openable -output is still written with the extension in it, so refusing every write would not pass. (cherry picked from commit 5c08eb39ce357756f14e9d0ea46a813af325db24) --- cifti/CMakeLists.txt | 10 ++++++++ cifti/test_write_failure.cmake | 42 ++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 cifti/test_write_failure.cmake diff --git a/cifti/CMakeLists.txt b/cifti/CMakeLists.txt index 88dcc657..ef43a4e4 100644 --- a/cifti/CMakeLists.txt +++ b/cifti/CMakeLists.txt @@ -68,4 +68,14 @@ if(NIFTI_BUILD_TESTING AND NIFTI_BUILD_APPLICATIONS) -input ${CMAKE_CURRENT_LIST_DIR}/testdata/deep_nesting.xml ) set_tests_properties( ${TEST_PREFIX}_deep_nesting PROPERTIES PASS_REGULAR_EXPRESSION "BrainModel" ) + + # A -output path that cannot be opened must be reported and refused, not + # written to anyway. + add_test( NAME ${TEST_PREFIX}_write_failure + COMMAND ${CMAKE_COMMAND} + -DTOOL=$ + -DINPUT=${CMAKE_CURRENT_LIST_DIR}/testdata/cext_unterminated.nii + -DBADOUT=${CMAKE_CURRENT_BINARY_DIR}/no_such_dir/out.txt + -DGOODOUT=${CMAKE_CURRENT_BINARY_DIR}/write_failure_ok.txt + -P ${CMAKE_CURRENT_LIST_DIR}/test_write_failure.cmake ) endif() diff --git a/cifti/test_write_failure.cmake b/cifti/test_write_failure.cmake new file mode 100644 index 00000000..ea264050 --- /dev/null +++ b/cifti/test_write_failure.cmake @@ -0,0 +1,42 @@ +# Drives cifti_tool at an unopenable -output path. A plain add_test cannot +# express this: PASS_REGULAR_EXPRESSION ignores the exit status, so a crash +# that printed the diagnostic first would still pass. + +if(NOT TOOL OR NOT INPUT OR NOT BADOUT OR NOT GOODOUT) + message(FATAL_ERROR "TOOL, INPUT, BADOUT and GOODOUT are all required") +endif() + +function(run_tool expect_result out_var) + execute_process(COMMAND ${TOOL} ${ARGN} + RESULT_VARIABLE result + OUTPUT_VARIABLE out + ERROR_VARIABLE err) + set(${out_var} "${out}${err}" PARENT_SCOPE) + if(NOT result STREQUAL "${expect_result}") + message(FATAL_ERROR + "expected result ${expect_result}, got '${result}'\n${out}${err}") + endif() +endfunction() + +run_tool(0 bad_disp -input ${INPUT} -disp_cext -output ${BADOUT}) +if(NOT bad_disp MATCHES "failed to open") + message(FATAL_ERROR "missing the open-failure diagnostic:\n${bad_disp}") +endif() + +run_tool(0 bad_eval -input ${INPUT} -eval_cext -eval_type show_summary + -output ${BADOUT}) +if(NOT bad_eval MATCHES "failed to open") + message(FATAL_ERROR "missing the open-failure diagnostic:\n${bad_eval}") +endif() + +# An openable -output must still be written, so that refusing every write +# would not pass. +file(REMOVE ${GOODOUT}) +run_tool(0 good_disp -input ${INPUT} -disp_cext -output ${GOODOUT}) +if(NOT EXISTS ${GOODOUT}) + message(FATAL_ERROR "nothing written to ${GOODOUT}") +endif() +file(READ ${GOODOUT} written) +if(NOT written MATCHES "MapName") + message(FATAL_ERROR "extension not displayed:\n${written}") +endif()