Set IFS to default before use in compiler. (#734)

* Fix: when compiler uses IFS, set it to default Signed-off-by: Bolun Thompson <[email protected]> * fix: remove local path from test Signed-off-by: Bolun Thompson <[email protected]> --------- Signed-off-by: Bolun Thompson <[email protected]> Co-authored-by: Bolun Thompson <[email protected]>
binpash · Nov 19, 2024 · 570ffdc · 570ffdc
1 parent d5a3b13
commit 570ffdc
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 5 deletions.
diff --git a/compiler/orchestrator_runtime/pash_prepare_call_compiler.sh b/compiler/orchestrator_runtime/pash_prepare_call_compiler.sh
@@ -49,13 +49,18 @@ else
     pash_runtime_return_code=1
 fi
 
+# save IFS to restore after field splitting
+[ -n "${IFS+x}" ] && saved_IFS=$IFS
+unset IFS
 # Get assigned process id
 # We need to split the daemon response into elements of an array by
 # shell's field splitting.
 # shellcheck disable=SC2206
 response_args=($daemon_response)
 process_id=${response_args[1]}
 
+[ -n "${saved_IFS+x}" ] && IFS="$saved_IFS"
+
 pash_redir_output echo "$$: (2) Compiler exited with code: $pash_runtime_return_code"
 
 ## only when --assert_all_regions_parallellizable is used do we care about all regions being parallelizable

diff --git a/compiler/orchestrator_runtime/speculative/speculative_runtime.sh b/compiler/orchestrator_runtime/speculative/speculative_runtime.sh
@@ -26,8 +26,12 @@ daemon_response=$(pash_spec_communicate_scheduler "$msg") # Blocking step, daemo
 
 ## Receive an exit code
 if [[ "$daemon_response" == *"OK:"* ]]; then
+    # save IFS to restore after field splitting
+    [ -n "${IFS+set}" ] && saved_IFS=$IFS
+    unset IFS
     # shellcheck disable=SC2206
     response_args=($daemon_response)
+    [ -n "${saved_IFS+set}" ] && IFS=$saved_IFS
     pash_redir_output echo "$$: (2) Scheduler responded: $daemon_response"
     cmd_exit_code=${response_args[1]}
     output_variable_file=${response_args[2]}
@@ -46,10 +50,8 @@ elif [[ "$daemon_response" == *"UNSAFE:"* ]]; then
     ## KK 2023-06-01 Does `eval` work in general? We need to be precise
     ##               about which commands are unsafe to determine how to execute them.
     cmd=$(cat "$PASH_SPEC_NODE_DIRECTORY/$pash_speculative_command_id")
-    ## KK 2023-06-01 Not sure if this shellcheck warning must be resolved:
-    ## > note: Double quote to prevent globbing and word splitting.
-    # shellcheck disable=SC2086
-    eval $cmd
+    ## Word splitting isn't needed since eval combines all the arguments into a single string
+    eval "$cmd"
     cmd_exit_code=$?
 elif [ -z "$daemon_response" ]; then
     ## Trouble... Daemon crashed, rip

diff --git a/evaluation/tests/interface_tests/run.sh b/evaluation/tests/interface_tests/run.sh
@@ -321,6 +321,12 @@ test_redir_dup()
     $shell redir-dup.sh
 }
 
+test_IFS()
+{
+    local shell=$1
+    $shell test-IFS.sh
+}
+
 ## We run all tests composed with && to exit on the first that fails
 if [ "$#" -eq 0 ]; then
     run_test test1
@@ -365,6 +371,7 @@ if [ "$#" -eq 0 ]; then
     run_test test_star
     run_test test_env_vars
     run_test test_redir_dup
+    run_test test_IFS
 else
     for testname in $@
     do

diff --git a/evaluation/tests/interface_tests/test-IFS.sh b/evaluation/tests/interface_tests/test-IFS.sh
@@ -0,0 +1,5 @@
+IFS=/
+curr_dir=/test1/test2/test3/test4
+for name in $curr_dir; do
+  echo "$name"
+done
diff --git a/runtime/wait_for_output_and_sigpipe_rest.sh b/runtime/wait_for_output_and_sigpipe_rest.sh
@@ -15,7 +15,7 @@ export internal_exec_status=$?
 # This value may contains multiple pids as a whitespace-separated string, and
 # we must split it as multiple pids by shell's field splitting.
 # shellcheck disable=SC2086
-(> /dev/null 2>&1 kill -SIGPIPE $pids_to_kill || true)
+(unset IFS; > /dev/null 2>&1 kill -SIGPIPE $pids_to_kill || true)
 
 ##
 ## Old way of waiting, very inefficient.