[PATCH] Added double quotes aound variables in bitbake/source to prevent gobbling or splitting

[PATCH] Added double quotes aound variables in bitbake/source to prevent gobbling or splitting

[Abongwa Bonalais Amahnui]

Signed-off-by: Abongwa Bonalais Amahnui <abongwabonalais@gmail.com>
---
 bitbake/bin/toaster | 65 +++++++++++++++++++++++----------------------
 1 file changed, 33 insertions(+), 32 deletions(-)

diff --git a/bitbake/bin/toaster b/bitbake/bin/toaster
index 558a819570..d98c4c69f0 100755
--- a/bitbake/bin/toaster
+++ b/bitbake/bin/toaster
@@ -1,4 +1,5 @@
-#!/bin/echo ERROR: This script needs to be sourced. Please run as .
+#!/bin/sh 
+echo ERROR: This script needs to be sourced. Please run as .
 
 # toaster - shell script to start Toaster
 
@@ -20,8 +21,8 @@ Usage 2: source toaster manage [createsuperuser|lsupdates|migrate|makemigrations
 custom_extention()
 {
     custom_extension=$BBBASEDIR/lib/toaster/orm/fixtures/custom_toaster_append.sh
-    if [ -f $custom_extension ] ; then
-        $custom_extension $*
+    if [ -f "$custom_extension" ] ; then
+        $custom_extension "$*"
     fi
 }
 
@@ -51,19 +52,19 @@ databaseCheck()
 webserverKillAll()
 {
     local pidfile
-    if [ -f ${BUILDDIR}/.toastermain.pid ] ; then
+    if [ -f "${BUILDDIR}"/.toastermain.pid ] ; then
         custom_extention web_stop_postpend
     else
         custom_extention noweb_stop_postpend
     fi
-    for pidfile in ${BUILDDIR}/.toastermain.pid ${BUILDDIR}/.runbuilds.pid; do
-        if [ -f ${pidfile} ]; then
-            pid=`cat ${pidfile}`
-            while kill -0 $pid 2>/dev/null; do
-                kill -SIGTERM $pid 2>/dev/null
+    for pidfile in "${BUILDDIR}"/.toastermain.pid "${BUILDDIR}"/.runbuilds.pid; do
+        if [ -f "${pidfile}" ]; then
+            pid=`cat "${pidfile}"`
+            while kill -0 "$pid" 2>/dev/null; do
+                kill -SIGTERM "$pid" 2>/dev/null
                 sleep 1
             done
-            rm  ${pidfile}
+            rm  "${pidfile}"
         fi
     done
 }
@@ -84,8 +85,8 @@ webserverStartAll()
     echo "Starting webserver..."
 
     $MANAGE runserver --noreload "$ADDR_PORT" \
-           </dev/null >>${BUILDDIR}/toaster_web.log 2>&1 \
-           & echo $! >${BUILDDIR}/.toastermain.pid
+           </dev/null >>"${BUILDDIR}"/toaster_web.log 2>&1 \
+           & echo "$!" >"${BUILDDIR}"/.toastermain.pid
 
     sleep 1
 
@@ -95,7 +96,7 @@ webserverStartAll()
     else
         echo "Toaster development webserver started at http://$ADDR_PORT"
         echo -e "\nYou can now run 'bitbake <target>' on the command line and monitor your build in Toaster.\nYou can also use a Toaster project to configure and run a build.\n"
-        custom_extention web_start_postpend $ADDR_PORT
+        custom_extention web_start_postpend "$ADDR_PORT"
     fi
 
     return $retval
@@ -131,8 +132,8 @@ verify_prereq() {
     exp=$exp'vmax=["%02d" % int(n) for n in "\4".split(".")];'
     exp=$exp'sys.exit(not (version \1 vmin and version \3 vmax))'
     exp=$exp'/p'
-    if ! sed -n "$exp" $reqfile | python3 - ; then
-        req=`grep ^Django $reqfile`
+    if ! sed -n "$exp" "$reqfile" | python3 - ; then
+        req=`grep ^Django "$reqfile"`
         echo "This program needs $req"
         echo "Please install with pip3 install -r $reqfile"
         return 2
@@ -150,10 +151,10 @@ else
     TOASTER=$0
 fi
 
-export BBBASEDIR=`dirname $TOASTER`/..
+export BBBASEDIR=`dirname "$TOASTER"`/..
 MANAGE="python3 $BBBASEDIR/lib/toaster/manage.py"
 if [ -z "$OE_ROOT" ]; then
-    OE_ROOT=`dirname $TOASTER`/../..
+    OE_ROOT=`dirname "$TOASTER"`/../..
 fi
 
 # this is the configuraton file we are using for toaster
@@ -164,7 +165,7 @@ fi
 # in the local layers that currently make using an arbitrary
 # toasterconf.json difficult.
 
-. $OE_ROOT/.templateconf
+. "$OE_ROOT"/.templateconf
 if [ -n "$TEMPLATECONF" ]; then
     if [ ! -d "$TEMPLATECONF" ]; then
         # Allow TEMPLATECONF=meta-xyz/conf as a shortcut
@@ -199,10 +200,10 @@ for param in $*; do
     webport=*)
             ADDR_PORT="${param#*=}"
             # Split the addr:port string
-            ADDR=`echo $ADDR_PORT | cut -f 1 -d ':'`
-            PORT=`echo $ADDR_PORT | cut -f 2 -d ':'`
+            ADDR=`echo "$ADDR_PORT" | cut -f 1 -d ':'`
+            PORT=`echo "$ADDR_PORT" | cut -f 2 -d ':'`
             # If only a port has been speified then set address to localhost.
-            if [ $ADDR = $PORT ] ; then
+            if [ "$ADDR" = "$PORT" ] ; then
                 ADDR_PORT="localhost:$PORT"
             fi
     ;;
@@ -229,7 +230,7 @@ for param in $*; do
     esac
 done
 
-if [ `basename \"$0\"` = `basename \"${TOASTER}\"` ]; then
+if [ `basename \""$0"\"` = `basename \""${TOASTER}"\"` ]; then
     echo "Error: This script needs to be sourced. Please run as . $TOASTER"
     return 1
 fi
@@ -265,7 +266,7 @@ fi
 echo "The system will $CMD."
 
 # Execute the commands
-custom_extention toaster_prepend $CMD $ADDR_PORT
+custom_extention toaster_prepend "$CMD" "$ADDR_PORT"
 
 case $CMD in
     start )
@@ -279,7 +280,7 @@ case $CMD in
         # Create configuration file
         conf=${BUILDDIR}/conf/local.conf
         line='INHERIT+="toaster buildhistory"'
-        grep -q "$line" $conf || echo $line >> $conf
+        grep -q "$line" "$conf" || echo "$line" >> "$conf"
 
         if [ $WEBSERVER -eq 0 ] ; then
             # Do not update the database for "noweb" unless
@@ -290,17 +291,17 @@ case $CMD in
                     return 4
                 fi
             fi
-            custom_extention noweb_start_postpend $ADDR_PORT
+            custom_extention noweb_start_postpend "$ADDR_PORT"
         fi
-        if [ $WEBSERVER -gt 0 ] && ! webserverStartAll; then
+        if [ "$WEBSERVER" -gt 0 ] && ! webserverStartAll; then
             echo "Failed ${CMD}."
             return 4
         fi
         export BITBAKE_UI='toasterui'
         if [ $TOASTER_BUILDSERVER -eq 1 ] ; then
             $MANAGE runbuilds \
-               </dev/null >>${BUILDDIR}/toaster_runbuilds.log 2>&1 \
-               & echo $! >${BUILDDIR}/.runbuilds.pid
+               </dev/null >>"${BUILDDIR}"/toaster_runbuilds.log 2>&1 \
+               & echo $! >"${BUILDDIR}"/.runbuilds.pid
         else
             echo "Toaster build server not started."
         fi
@@ -308,7 +309,7 @@ case $CMD in
         # set fail safe stop system on terminal exit
         trap stop_system SIGHUP
         echo "Successful ${CMD}."
-        custom_extention toaster_postpend $CMD $ADDR_PORT
+        custom_extention toaster_postpend "$CMD" "$ADDR_PORT"
         return 0
     ;;
     stop )
@@ -316,9 +317,9 @@ case $CMD in
         echo "Successful ${CMD}."
     ;;
     manage )
-        cd $BBBASEDIR/lib/toaster
-        $MANAGE $manage_cmd
+        cd "$BBBASEDIR"/lib/toaster
+        "$MANAGE $manage_cmd"
     ;;
 esac
-custom_extention toaster_postpend $CMD $ADDR_PORT
+custom_extention toaster_postpend "$CMD" "$ADDR_PORT"
 
-- 
2.25.1

Re: [bitbake-devel] [PATCH] Added double quotes aound variables in bitbake/source to prevent gobbling or splitting

[Richard Purdie]

Hi,

Thanks for the patch, I have a question below.

On Tue, 2022-04-05 at 19:29 +0100, Abongwa Amahnui Bonalais wrote:
> Signed-off-by: Abongwa Bonalais Amahnui <abongwabonalais@gmail.com>
> ---
>  bitbake/bin/toaster | 65 +++++++++++++++++++++++----------------------
>  1 file changed, 33 insertions(+), 32 deletions(-)
> 
> diff --git a/bitbake/bin/toaster b/bitbake/bin/toaster
> index 558a819570..d98c4c69f0 100755
> --- a/bitbake/bin/toaster
> +++ b/bitbake/bin/toaster
> @@ -1,4 +1,5 @@
> -#!/bin/echo ERROR: This script needs to be sourced. Please run as .
> +#!/bin/sh 
> +echo ERROR: This script needs to be sourced. Please run as .
>  
>  # toaster - shell script to start Toaster
> 

Changing /bin/echo to /bin/sh probably isn't intentional? We do intend the
script to be sourced rather than run directly. How did you test the changes?

Cheers,

Richard

Re: [PATCH] Added double quotes aound variables in bitbake/source to prevent gobbling or splitting

[Abongwa Amahnui Bonalais]

[-- Attachment #1.1: Type: text/plain, Size: 327 bytes --]

I tested the changes by running shellcheck toaster.
I did so the first time and it returned
"^-- SC1008: This shebang was unrecognized. ShellCheck only supports sh/bash/dash/ksh. Add a 'shell' directive to specify.
^-- SC2096: On most OS, shebangs can only specify a single parameter.
"
as shown in the attachment below.

[-- Attachment #1.2: Type: text/html, Size: 363 bytes --]

[-- Attachment #2: Screenshot from 2022-04-05 20-58-32.png --]
[-- Type: image/png, Size: 252206 bytes --]

Re: [PATCH] Added double quotes aound variables in bitbake/source to prevent gobbling or splitting

[Abongwa Amahnui Bonalais]

[-- Attachment #1: Type: text/plain, Size: 361 bytes --]

Hi Richard
I'm sorry, I just saw the first email now. I wish to ask if the I could revert the change on the first line, Allowing just the changes to the spaces quoting. I actually intended to send that as a different patch but u I untintentionally included it this path.
If the quoting is fine, I will send a patch that fixes just the quoting issue.
Thanks

[-- Attachment #2: Type: text/html, Size: 373 bytes --]

RE: [bitbake-devel] [PATCH] Added double quotes aound variables in bitbake/source to prevent gobbling or splitting

[Peter Kjellerstedt]

[ Since you seem new to the process of reviewing via mail, please 
  note that I have added comments throughout the entire patch below. ]

> -----Original Message-----
> From: bitbake-devel@lists.openembedded.org <bitbake-
> devel@lists.openembedded.org> On Behalf Of Abongwa Amahnui Bonalais
> Sent: den 5 april 2022 20:29
> To: bitbake-devel@lists.openembedded.org
> Cc: Abongwa Bonalais Amahnui <abongwabonalais@gmail.com>
> Subject: [bitbake-devel] [PATCH] Added double quotes aound variables in bitbake/source to prevent gobbling or splitting

Always prefix the subject line with the affected recipe name or similar. 
Also keep the subject short, typically shorter than 72 characters.
A better subject in this case would be:

  toaster: Add quoting of variables to avoid splitting

Then there should be a description that gives a motivation for 
why this is needed and any other relevant information.

> 
> Signed-off-by: Abongwa Bonalais Amahnui <abongwabonalais@gmail.com>
> ---
>  bitbake/bin/toaster | 65 +++++++++++++++++++++++----------------------
>  1 file changed, 33 insertions(+), 32 deletions(-)
> 
> diff --git a/bitbake/bin/toaster b/bitbake/bin/toaster
> index 558a819570..d98c4c69f0 100755
> --- a/bitbake/bin/toaster
> +++ b/bitbake/bin/toaster
> @@ -1,4 +1,5 @@
> -#!/bin/echo ERROR: This script needs to be sourced. Please run as .
> +#!/bin/sh
> +echo ERROR: This script needs to be sourced. Please run as .

Leave as it was. It is explicitly _not_ using #!/bin/sh as shebang line 
to catch misuse.

> 
>  # toaster - shell script to start Toaster
> 
> @@ -20,8 +21,8 @@ Usage 2: source toaster manage [createsuperuser|lsupdates|migrate|makemigrations
>  custom_extention()
>  {
> 
> custom_extension=$BBBASEDIR/lib/toaster/orm/fixtures/custom_toaster_append.sh
> -    if [ -f $custom_extension ] ; then
> -        $custom_extension $*
> +    if [ -f "$custom_extension" ] ; then
> +        $custom_extension "$*"
>      fi
>  }
> 
> @@ -51,19 +52,19 @@ databaseCheck()
>  webserverKillAll()
>  {
>      local pidfile
> -    if [ -f ${BUILDDIR}/.toastermain.pid ] ; then
> +    if [ -f "${BUILDDIR}"/.toastermain.pid ] ; then

While it is technically correct, please put the quotes around the 
entire string to make it more readable, i.e.:

    if [ -f "${BUILDDIR}/.toastermain.pid" ] ; then

This applies to the rest of the changes in this file.

>          custom_extention web_stop_postpend
>      else
>          custom_extention noweb_stop_postpend
>      fi
> -    for pidfile in ${BUILDDIR}/.toastermain.pid ${BUILDDIR}/.runbuilds.pid; do
> -        if [ -f ${pidfile} ]; then
> -            pid=`cat ${pidfile}`
> -            while kill -0 $pid 2>/dev/null; do
> -                kill -SIGTERM $pid 2>/dev/null
> +    for pidfile in "${BUILDDIR}"/.toastermain.pid "${BUILDDIR}"/.runbuilds.pid; do
> +        if [ -f "${pidfile}" ]; then
> +            pid=`cat "${pidfile}"`
> +            while kill -0 "$pid" 2>/dev/null; do
> +                kill -SIGTERM "$pid" 2>/dev/null
>                  sleep 1
>              done
> -            rm  ${pidfile}
> +            rm  "${pidfile}"
>          fi
>      done
>  }
> @@ -84,8 +85,8 @@ webserverStartAll()
>      echo "Starting webserver..."
> 
>      $MANAGE runserver --noreload "$ADDR_PORT" \
> -           </dev/null >>${BUILDDIR}/toaster_web.log 2>&1 \
> -           & echo $! >${BUILDDIR}/.toastermain.pid
> +           </dev/null >>"${BUILDDIR}"/toaster_web.log 2>&1 \
> +           & echo "$!" >"${BUILDDIR}"/.toastermain.pid
> 
>      sleep 1
> 
> @@ -95,7 +96,7 @@ webserverStartAll()
>      else
>          echo "Toaster development webserver started at http://$ADDR_PORT"
>          echo -e "\nYou can now run 'bitbake <target>' on the command line and monitor your build in Toaster.\nYou can also use a Toaster project to configure and run a build.\n"
> -        custom_extention web_start_postpend $ADDR_PORT
> +        custom_extention web_start_postpend "$ADDR_PORT"
>      fi
> 
>      return $retval
> @@ -131,8 +132,8 @@ verify_prereq() {
>      exp=$exp'vmax=["%02d" % int(n) for n in "\4".split(".")];'
>      exp=$exp'sys.exit(not (version \1 vmin and version \3 vmax))'
>      exp=$exp'/p'
> -    if ! sed -n "$exp" $reqfile | python3 - ; then
> -        req=`grep ^Django $reqfile`
> +    if ! sed -n "$exp" "$reqfile" | python3 - ; then
> +        req=`grep ^Django "$reqfile"`
>          echo "This program needs $req"
>          echo "Please install with pip3 install -r $reqfile"
>          return 2
> @@ -150,10 +151,10 @@ else
>      TOASTER=$0
>  fi
> 
> -export BBBASEDIR=`dirname $TOASTER`/..
> +export BBBASEDIR=`dirname "$TOASTER"`/..
>  MANAGE="python3 $BBBASEDIR/lib/toaster/manage.py"
>  if [ -z "$OE_ROOT" ]; then
> -    OE_ROOT=`dirname $TOASTER`/../..
> +    OE_ROOT=`dirname "$TOASTER"`/../..
>  fi
> 
>  # this is the configuraton file we are using for toaster
> @@ -164,7 +165,7 @@ fi
>  # in the local layers that currently make using an arbitrary
>  # toasterconf.json difficult.
> 
> -. $OE_ROOT/.templateconf
> +. "$OE_ROOT"/.templateconf
>  if [ -n "$TEMPLATECONF" ]; then
>      if [ ! -d "$TEMPLATECONF" ]; then
>          # Allow TEMPLATECONF=meta-xyz/conf as a shortcut
> @@ -199,10 +200,10 @@ for param in $*; do
>      webport=*)
>              ADDR_PORT="${param#*=}"
>              # Split the addr:port string
> -            ADDR=`echo $ADDR_PORT | cut -f 1 -d ':'`
> -            PORT=`echo $ADDR_PORT | cut -f 2 -d ':'`
> +            ADDR=`echo "$ADDR_PORT" | cut -f 1 -d ':'`
> +            PORT=`echo "$ADDR_PORT" | cut -f 2 -d ':'`
>              # If only a port has been speified then set address to localhost.
> -            if [ $ADDR = $PORT ] ; then
> +            if [ "$ADDR" = "$PORT" ] ; then
>                  ADDR_PORT="localhost:$PORT"
>              fi
>      ;;
> @@ -229,7 +230,7 @@ for param in $*; do
>      esac
>  done
> 
> -if [ `basename \"$0\"` = `basename \"${TOASTER}\"` ]; then
> +if [ `basename \""$0"\"` = `basename \""${TOASTER}"\"` ]; then

That is wrong (as the original code was wrong). Change it to:

if [ `basename "$0"` = `basename "${TOASTER}"` ]; then

>      echo "Error: This script needs to be sourced. Please run as . $TOASTER"
>      return 1
>  fi
> @@ -265,7 +266,7 @@ fi
>  echo "The system will $CMD."
> 
>  # Execute the commands
> -custom_extention toaster_prepend $CMD $ADDR_PORT
> +custom_extention toaster_prepend "$CMD" "$ADDR_PORT"
> 
>  case $CMD in
>      start )
> @@ -279,7 +280,7 @@ case $CMD in
>          # Create configuration file
>          conf=${BUILDDIR}/conf/local.conf
>          line='INHERIT+="toaster buildhistory"'
> -        grep -q "$line" $conf || echo $line >> $conf
> +        grep -q "$line" "$conf" || echo "$line" >> "$conf"
> 
>          if [ $WEBSERVER -eq 0 ] ; then
>              # Do not update the database for "noweb" unless
> @@ -290,17 +291,17 @@ case $CMD in
>                      return 4
>                  fi
>              fi
> -            custom_extention noweb_start_postpend $ADDR_PORT
> +            custom_extention noweb_start_postpend "$ADDR_PORT"
>          fi
> -        if [ $WEBSERVER -gt 0 ] && ! webserverStartAll; then
> +        if [ "$WEBSERVER" -gt 0 ] && ! webserverStartAll; then
>              echo "Failed ${CMD}."
>              return 4
>          fi
>          export BITBAKE_UI='toasterui'
>          if [ $TOASTER_BUILDSERVER -eq 1 ] ; then
>              $MANAGE runbuilds \
> -               </dev/null >>${BUILDDIR}/toaster_runbuilds.log 2>&1 \
> -               & echo $! >${BUILDDIR}/.runbuilds.pid
> +               </dev/null >>"${BUILDDIR}"/toaster_runbuilds.log 2>&1 \
> +               & echo $! >"${BUILDDIR}"/.runbuilds.pid
>          else
>              echo "Toaster build server not started."
>          fi
> @@ -308,7 +309,7 @@ case $CMD in
>          # set fail safe stop system on terminal exit
>          trap stop_system SIGHUP
>          echo "Successful ${CMD}."
> -        custom_extention toaster_postpend $CMD $ADDR_PORT
> +        custom_extention toaster_postpend "$CMD" "$ADDR_PORT"
>          return 0
>      ;;
>      stop )
> @@ -316,9 +317,9 @@ case $CMD in
>          echo "Successful ${CMD}."
>      ;;
>      manage )
> -        cd $BBBASEDIR/lib/toaster
> -        $MANAGE $manage_cmd
> +        cd "$BBBASEDIR"/lib/toaster
> +        "$MANAGE $manage_cmd"
>      ;;
>  esac
> -custom_extention toaster_postpend $CMD $ADDR_PORT
> +custom_extention toaster_postpend "$CMD" "$ADDR_PORT"
> 
> --
> 2.25.1

//Peter

Re: [PATCH] Added double quotes aound variables in bitbake/source to prevent gobbling or splitting

[Abongwa Amahnui Bonalais]

[-- Attachment #1: Type: text/plain, Size: 87 bytes --]

Hi Peter,
Thanks alot for the review, I will do the changes and send a patch for it.

[-- Attachment #2: Type: text/html, Size: 91 bytes --]

Re: [PATCH] Added double quotes aound variables in bitbake/source to prevent gobbling or splitting

[Abongwa Amahnui Bonalais]

[-- Attachment #1: Type: text/plain, Size: 118 bytes --]

Hi Peter
this is the link to the new patch I created.
https://lists.openembedded.org/g/bitbake-devel/message/13591

[-- Attachment #2: Type: text/html, Size: 238 bytes --]