Merge pull request #1358 from YunoHost/fix-eval-in-logging-helpers

Fix eval in logging helpers

data/helpers.d/logging

@@ -38,25 +38,6 @@ ynh_print_info() {
     echo "$message" >&$YNH_STDINFO
 }
 
-# Ignore the yunohost-cli log to prevent errors with conditional commands
-#
-# [internal]
-#
-# usage: ynh_no_log COMMAND
-#
-# Simply duplicate the log, execute the yunohost command and replace the log without the result of this command
-# It's a very badly hack...
-#
-# Requires YunoHost version 2.6.4 or higher.
-ynh_no_log() {
-    local ynh_cli_log=/var/log/yunohost/yunohost-cli.log
-    cp --archive ${ynh_cli_log} ${ynh_cli_log}-move
-    eval $@
-    local exit_code=$?
-    mv ${ynh_cli_log}-move ${ynh_cli_log}
-    return $exit_code
-}
-
 # Main printer, just in case in the future we have to change anything about that.
 #
 # [internal]
@@ -102,72 +83,107 @@ ynh_print_err() {
 
 # Execute a command and print the result as an error
 #
-# usage: ynh_exec_err "your_command [ | other_command ]"
+# usage: ynh_exec_err your command and args
 # | arg: command - command to execute
 #
-# When using pipes, double quotes are required - otherwise, this helper will run the first command, and the whole output will be sent through the next pipe.
+# Note that you should NOT quote the command but only prefix it with ynh_exec_err
-#
-# If the command to execute uses double quotes, they have to be escaped or they will be interpreted and removed.
 #
 # Requires YunoHost version 3.2.0 or higher.
 ynh_exec_err() {
+    # Boring legacy handling for when people calls ynh_exec_* wrapping the command in quotes,
+    # (because in the past eval was used) ...
+    # we detect this by checking that there's no 2nd arg, and $1 contains a space
+    if [[ "$#" -eq 1 ]] && [[ "$1" == *" "* ]]
+    then
         ynh_print_err "$(eval $@)"
+    else
+        # Note that "$@" is used and not $@, c.f. https://unix.stackexchange.com/a/129077
+        ynh_print_err "$("$@")"
+    fi
 }
 
 # Execute a command and print the result as a warning
 #
-# usage: ynh_exec_warn "your_command [ | other_command ]"
+# usage: ynh_exec_warn your command and args
 # | arg: command - command to execute
 #
-# When using pipes, double quotes are required - otherwise, this helper will run the first command, and the whole output will be sent through the next pipe.
+# Note that you should NOT quote the command but only prefix it with ynh_exec_warn
-#
-# If the command to execute uses double quotes, they have to be escaped or they will be interpreted and removed.
 #
 # Requires YunoHost version 3.2.0 or higher.
 ynh_exec_warn() {
+    # Boring legacy handling for when people calls ynh_exec_* wrapping the command in quotes,
+    # (because in the past eval was used) ...
+    # we detect this by checking that there's no 2nd arg, and $1 contains a space
+    if [[ "$#" -eq 1 ]] && [[ "$1" == *" "* ]]
+    then
         ynh_print_warn "$(eval $@)"
+    else
+        # Note that "$@" is used and not $@, c.f. https://unix.stackexchange.com/a/129077
+        ynh_print_warn "$("$@")"
+    fi
 }
 
 # Execute a command and force the result to be printed on stdout
 #
-# usage: ynh_exec_warn_less "your_command [ | other_command ]"
+# usage: ynh_exec_warn_less your command and args
 # | arg: command - command to execute
 #
-# When using pipes, double quotes are required - otherwise, this helper will run the first command, and the whole output will be sent through the next pipe.
+# Note that you should NOT quote the command but only prefix it with ynh_exec_warn
-#
-# If the command to execute uses double quotes, they have to be escaped or they will be interpreted and removed.
 #
 # Requires YunoHost version 3.2.0 or higher.
 ynh_exec_warn_less() {
+    # Boring legacy handling for when people calls ynh_exec_* wrapping the command in quotes,
+    # (because in the past eval was used) ...
+    # we detect this by checking that there's no 2nd arg, and $1 contains a space
+    if [[ "$#" -eq 1 ]] && [[ "$1" == *" "* ]]
+    then
         eval $@ 2>&1
+    else
+        # Note that "$@" is used and not $@, c.f. https://unix.stackexchange.com/a/129077
+        "$@" 2>&1
+    fi
 }
 
 # Execute a command and redirect stdout in /dev/null
 #
-# usage: ynh_exec_quiet "your_command [ | other_command ]"
+# usage: ynh_exec_quiet your command and args
 # | arg: command - command to execute
 #
-# When using pipes, double quotes are required - otherwise, this helper will run the first command, and the whole output will be sent through the next pipe.
+# Note that you should NOT quote the command but only prefix it with ynh_exec_warn
-#
-# If the command to execute uses double quotes, they have to be escaped or they will be interpreted and removed.
 #
 # Requires YunoHost version 3.2.0 or higher.
 ynh_exec_quiet() {
+    # Boring legacy handling for when people calls ynh_exec_* wrapping the command in quotes,
+    # (because in the past eval was used) ...
+    # we detect this by checking that there's no 2nd arg, and $1 contains a space
+    if [[ "$#" -eq 1 ]] && [[ "$1" == *" "* ]]
+    then
         eval $@ > /dev/null
+    else
+        # Note that "$@" is used and not $@, c.f. https://unix.stackexchange.com/a/129077
+        "$@" > /dev/null
+    fi
 }
 
 # Execute a command and redirect stdout and stderr in /dev/null
 #
-# usage: ynh_exec_fully_quiet "your_command [ | other_command ]"
+# usage: ynh_exec_quiet your command and args
 # | arg: command - command to execute
 #
-# When using pipes, double quotes are required - otherwise, this helper will run the first command, and the whole output will be sent through the next pipe.
+# Note that you should NOT quote the command but only prefix it with ynh_exec_quiet
-#
-# If the command to execute uses double quotes, they have to be escaped or they will be interpreted and removed.
 #
 # Requires YunoHost version 3.2.0 or higher.
 ynh_exec_fully_quiet() {
+    # Boring legacy handling for when people calls ynh_exec_* wrapping the command in quotes,
+    # (because in the past eval was used) ...
+    # we detect this by checking that there's no 2nd arg, and $1 contains a space
+    if [[ "$#" -eq 1 ]] && [[ "$1" == *" "* ]]
+    then
         eval $@ > /dev/null 2>&1
+    else
+        # Note that "$@" is used and not $@, c.f. https://unix.stackexchange.com/a/129077
+        "$@" > /dev/null 2>&1
+    fi
 }
 
 # Remove any logs for all the following commands.
@@ -302,67 +318,3 @@ ynh_script_progression() {
 ynh_return() {
     echo "$1" >>"$YNH_STDRETURN"
 }
-
-# Debugger for app packagers
-#
-# usage: ynh_debug [--message=message] [--trace=1/0]
-# | arg: -m, --message= - The text to print
-# | arg: -t, --trace=   - Turn on or off the trace of the script. Usefull to trace nonly a small part of a script.
-#
-# Requires YunoHost version 3.5.0 or higher.
-ynh_debug() {
-    # Disable set xtrace for the helper itself, to not pollute the debug log
-    set +o xtrace # set +x
-    # Declare an array to define the options of this helper.
-    local legacy_args=mt
-    local -A args_array=([m]=message= [t]=trace=)
-    local message
-    local trace
-    # Manage arguments with getopts
-    ynh_handle_getopts_args "$@"
-    # Re-disable xtrace, ynh_handle_getopts_args set it back
-    set +o xtrace # set +x
-    message=${message:-}
-    trace=${trace:-}
-
-    if [ -n "$message" ]; then
-        ynh_print_log "[Debug] ${message}" >&2
-    fi
-
-    if [ "$trace" == "1" ]; then
-        ynh_debug --message="Enable debugging"
-        set +o xtrace # set +x
-        # Get the current file descriptor of xtrace
-        old_bash_xtracefd=$BASH_XTRACEFD
-        # Add the current file name and the line number of any command currently running while tracing.
-        PS4='$(basename ${BASH_SOURCE[0]})-L${LINENO}: '
-        # Force xtrace to stderr
-        BASH_XTRACEFD=2
-        # Force stdout to stderr
-        exec 1>&2
-    fi
-    if [ "$trace" == "0" ]; then
-        ynh_debug --message="Disable debugging"
-        set +o xtrace # set +x
-        # Put xtrace back to its original fild descriptor
-        BASH_XTRACEFD=$old_bash_xtracefd
-        # Restore stdout
-        exec 1>&1
-    fi
-    # Renable set xtrace
-    set -o xtrace # set -x
-}
-
-# Execute a command and print the result as debug
-#
-# usage: ynh_debug_exec "your_command [ | other_command ]"
-# | arg: command - command to execute
-#
-# When using pipes, double quotes are required - otherwise, this helper will run the first command, and the whole output will be sent through the next pipe.
-#
-# If the command to execute uses double quotes, they have to be escaped or they will be interpreted and removed.
-#
-# Requires YunoHost version 3.5.0 or higher.
-ynh_debug_exec() {
-    ynh_debug --message="$(eval $@)"
-}

data/helpers.d/php

@@ -447,24 +447,24 @@ ynh_get_scalable_phpfpm() {
     fi
 
     if [ $print -eq 1 ]; then
-        ynh_debug --message="Footprint=${footprint}Mb by pool."
+        ynh_print_warn --message="Footprint=${footprint}Mb by pool."
-        ynh_debug --message="Process manager=$php_pm"
+        ynh_print_warn --message="Process manager=$php_pm"
-        ynh_debug --message="Max RAM=${max_ram}Mb"
+        ynh_print_warn --message="Max RAM=${max_ram}Mb"
         if [ "$php_pm" != "static" ]; then
-            ynh_debug --message="\nMax estimated footprint=$(($php_max_children * $footprint))"
+            ynh_print_warn --message="\nMax estimated footprint=$(($php_max_children * $footprint))"
-            ynh_debug --message="Min estimated footprint=$(($php_min_spare_servers * $footprint))"
+            ynh_print_warn --message="Min estimated footprint=$(($php_min_spare_servers * $footprint))"
         fi
         if [ "$php_pm" = "dynamic" ]; then
-            ynh_debug --message="Estimated average footprint=$(($php_max_spare_servers * $footprint))"
+            ynh_print_warn --message="Estimated average footprint=$(($php_max_spare_servers * $footprint))"
         elif [ "$php_pm" = "static" ]; then
-            ynh_debug --message="Estimated footprint=$(($php_max_children * $footprint))"
+            ynh_print_warn --message="Estimated footprint=$(($php_max_children * $footprint))"
         fi
-        ynh_debug --message="\nRaw php-fpm values:"
+        ynh_print_warn --message="\nRaw php-fpm values:"
-        ynh_debug --message="pm.max_children = $php_max_children"
+        ynh_print_warn --message="pm.max_children = $php_max_children"
         if [ "$php_pm" = "dynamic" ]; then
-            ynh_debug --message="pm.start_servers = $php_start_servers"
+            ynh_print_warn --message="pm.start_servers = $php_start_servers"
-            ynh_debug --message="pm.min_spare_servers = $php_min_spare_servers"
+            ynh_print_warn --message="pm.min_spare_servers = $php_min_spare_servers"
-            ynh_debug --message="pm.max_spare_servers = $php_max_spare_servers"
+            ynh_print_warn --message="pm.max_spare_servers = $php_max_spare_servers"
         fi
     fi
 }

tests/test_helpers.d/ynhtest_logging.sh

@@ -0,0 +1,92 @@
+ynhtest_exec_warn_less() {
+
+    FOO='foo'
+    bar=""
+    BAR='$bar'
+    FOOBAR="foo bar"
+
+    # These looks like stupid edge case
+    # but in fact happens when dealing with passwords
+    # (which could also contain bash chars like [], {}, ...)
+    # or urls containing &, ...
+    FOOANDBAR="foo&bar"
+    FOO1QUOTEBAR="foo'bar"
+    FOO2QUOTEBAR="foo\"bar"
+    
+    ynh_exec_warn_less uptime
+   
+    test ! -e $FOO
+    ynh_exec_warn_less touch $FOO
+    test -e $FOO
+    rm $FOO
+
+    test ! -e $FOO1QUOTEBAR
+    ynh_exec_warn_less touch $FOO1QUOTEBAR
+    test -e $FOO1QUOTEBAR
+    rm $FOO1QUOTEBAR
+
+    test ! -e $FOO2QUOTEBAR
+    ynh_exec_warn_less touch $FOO2QUOTEBAR
+    test -e $FOO2QUOTEBAR
+    rm $FOO2QUOTEBAR
+
+    test ! -e $BAR
+    ynh_exec_warn_less touch $BAR
+    test -e $BAR
+    rm $BAR
+
+    test ! -e "$FOOBAR"
+    ynh_exec_warn_less touch "$FOOBAR"
+    test -e "$FOOBAR"
+    rm "$FOOBAR"
+
+    test ! -e "$FOOANDBAR"
+    ynh_exec_warn_less touch $FOOANDBAR
+    test -e "$FOOANDBAR"
+    rm "$FOOANDBAR"
+
+    ###########################
+    # Legacy stuff using eval #
+    ###########################
+    
+    test ! -e $FOO
+    ynh_exec_warn_less "touch $FOO"
+    test -e $FOO
+    rm $FOO
+
+    test ! -e $FOO1QUOTEBAR
+    ynh_exec_warn_less "touch \"$FOO1QUOTEBAR\""
+    # (this works but expliciy *double* quotes have to be provided)
+    test -e $FOO1QUOTEBAR
+    rm $FOO1QUOTEBAR
+
+    #test ! -e $FOO2QUOTEBAR
+    #ynh_exec_warn_less "touch \'$FOO2QUOTEBAR\'"
+    ## (this doesn't work with simple or double quotes)
+    #test -e $FOO2QUOTEBAR
+    #rm $FOO2QUOTEBAR
+
+    test ! -e $BAR
+    ynh_exec_warn_less 'touch $BAR'
+    # That one works because $BAR is only interpreted during eval
+    test -e $BAR
+    rm $BAR
+
+    #test ! -e $BAR
+    #ynh_exec_warn_less "touch $BAR"
+    # That one doesn't work because $bar gets interpreted as empty var by eval...
+    #test -e $BAR
+    #rm $BAR
+
+    test ! -e "$FOOBAR"
+    ynh_exec_warn_less "touch \"$FOOBAR\""
+    # (works but requires explicit double quotes otherwise eval would interpret 'foo bar' as two separate args..)
+    test -e "$FOOBAR"
+    rm "$FOOBAR"
+
+    test ! -e "$FOOANDBAR"
+    ynh_exec_warn_less "touch \"$FOOANDBAR\""
+    # (works but requires explicit double quotes otherwise eval would interpret '&' as a "run command in background" and also bar is not a valid command)
+    test -e "$FOOANDBAR"
+    rm "$FOOANDBAR"
+}