[SOLVED] Bash Question?

Spazztic_Killer · 05-28-2013, 08:15 PM

I've got a script that tests whether or not a user and a group does exist. And I also got the input restricted pretty well. What I am running into is that Im testing the script and only alpha-numeric is allow for must input here, but when I do like this username joe$$ it expands to joe2566 or what evernumber I believe the PID. Question is how do I better or futher restrict the input to not allow $$. It will fail if I do joe$

Code:

if [ "${?}" -eq 0 ] && [ -n "${_usr}" ] && [ -n "${_full_name}" ] && [[ "${_usr}" =~ ^[[:alnum:]]*$ ]] && [[ "${_full_name}" =~ ^[[:alnum:]' '.]*$ ]]; then
     echo > /dev/null;
  elif [ "${?}" -eq 1 ] && [ -n "${_usr}" ] && [ -n "${_full_name}" ] && [[ ! "${_usr}" =~ ^[[:alnum:]]*$ ]] || [[ ! "${_full_name}" =~ ^[[:alnum:]' '.]*$ ]]; then
     printf "%s\n" "ERROR: Invaild user or full name, "${_usr}" and or "${_full_name}" may only be alpha-numeric " && exit 1;
   else
     printf "%s\n" "Arguments where not passed to script " && exit 1;
   fi

jpollard · 05-28-2013, 10:41 PM

The simple answer is don't use bash.

Another one is that a user name joe$$ will frequently cause problems since $ is a special metacharacter for all shells.

A complicated answer is that it means you have to scan the user name and escape the special characters... You would have to turn joe$$ into joe\$\$. (or turn joe$$ into 'joe$$').

Of course, this also causes problems because joe\$\$ (or 'joe$$') isn't the user name...

The problem is that it depends on how you are using a variable, and here, you are using it in two different ways. As a simple value, and as a double expansion (the conditional tests)

You could try making a test variable with the escape sequence just for use in the conditions.

millgates · 05-29-2013, 03:01 AM

The "$$" is expanded by the shell before it is even passed to the script, so the script has no chance of doing anything about it. You really have to quote the argument (or escape the $'s)

Code:

username 'joe$$'

David the H. · 05-30-2013, 03:58 PM

I hate to say it, but that's rather ugly code, too.

1) At the very least please take the time to properly format it for easy reading. Use more whitespace and properly indent your lines so that the sub-commands are visually distinct from the surrounding if statement (the keywords for which all line up).

And lose the extra {} brackets around your variables. They don't do anything useful most of the time, and only clutter up the code even more. Trust me, you'll be better without them.

Scripting With Style

2) Second, lets clean up the logic here a bit. Long lines of &&/|| tests are quite confusing to parse, and very easy to screw up. How about starting by splitting it up into separate tests?

In addition, when using advanced shells like bash or ksh, it's recommended to use [[..]] for string/file tests, and ((..)) for numerical tests. Avoid using the old [..] test unless you specifically need POSIX-style portability.

http://mywiki.wooledge.org/BashFAQ/031
http://mywiki.wooledge.org/ArithmeticExpression

They are also better for combining multiple complex tests like these.

3) Try moving long text strings into variables first, to keep the code itself relatively clear and readable. Or at least use printf the way it should. It's a good idea to store bash regex tests in variables first too, as you don't need to do any complex escaping that way.

4) What is "echo > /dev/null" supposed to be doing? If you just want the script to do nothing, simply use the ":" (true) command. Or even better restructure the if statement so that you can just drop that unnecessary test entirely.

So, ignoring the actual function of the code for the moment, this is how I would re-write what was posted in the OP:

Code:

<command to test>
status=$?

re1='[^[:alnum:]]'
re2='[^[:alnum:] .]'
warnfmt='ERROR: Invaild user or full name, "%s" and or "%s" may only be alpha-numeric \n'

if [[ -n $_usr && -n $_full_name ]]; then

    if [[ $status -eq 1 && ( $_usr =~ $re1 || "$_full_name =~ $re2 ) ]]; then

        printf "$warnfmt" "$_usr" "$_full_name" && exit 1

    fi

else

    echo "Arguments were not passed to script " && exit 1

fi

Note that I had to move the $? into a permanent variable in order to ensure that the nested test checks the proper command.

BTW, simple globbing can be used instead of regex to test for unwanted characters. It's more efficient. Another good option is to just remove all the characters you want and see if anything remains.

Code:

string='foo@bar'

[[ $string == *[^[:alnum:]]* ]] && echo "Bad string!" || echo "Good string!"

[[ -n ${string//[[:alnum:]]} ]] && echo "Bad string!" || echo "Good string!"

Spazztic_Killer · 05-31-2013, 08:55 AM

Thank you very much for helping me!! The scriping guide looks awesome ill have to do more reading.

I've personally only been practicing linux, for just over a year now. I'm a college student trying to learn as much as I can as fast has I can. I've always new to turn to linux for backup when windows fails. And I lack the structuring part. The code is ugly I agree I will try like I do and understand what you all have giving me here and test and learn more and better layout my code. Jpollard you said dont use bash. I've been wanting to learn programming but I havnt really had time. I've have been looking at perl, php, and python? Been making very simple scripts with perl (nothing really useful yet).

How would I do the true test ':' just completly replace the "echo > /dev/null" with just ":"?

And what about the globbing instead? I am a very visual learner so if anything I will just have to trail and error and learn what does what.

So this is really what the whole thing looks like, without me touching it the whole picture. Which this is only the top part of another script, but ill clean it all up and redo a lot. Pretty much broken down into different functions. The re-written code you did, would replace the code in the clean_input_func, correct or must of it?

Code:

#!/bin/bash
#set -xv
function print_usage()
  {
   clear;
     cat<<_USAGE_
NOTE: Only the full name may contain a space, and you must quote that input to get it to take !
NOTE: The user is checked for in both the passwd and group files under /etc 

      Usage: ${0##*/} { username } { fullname } { groupname }
             ${0##*/} { username } { "full name" } { groupname }
             ${0##*/} John J.Doe Admins
             ${0##*/} John "John Doe" Admins
             ${0##*/} John5 "J. Doe" John

_USAGE_
  }
function clean_input_func()
  {
   if [ "${?}" -eq 0 ] && [ -n "${_usr}" ] && [ -n "${_full_name}" ] && [[ "${_usr}" =~ ^[[:alnum:]]*$ ]] && [[ "${_full_name}" =~ ^[[:alnum:]' '.]*$ ]]; then
     echo > /dev/null;
     if [ "${?}" -eq 0 ] && [ -n "${_grp}" ] && [[ "${_grp}" =~ ^[[:alnum:]]*$ ]]; then
       echo > /dev/null;
     elif [ "${?}" -eq 1 ] && [ -n "${_grp}" ] && [[ ! "${_grp}" =~ ^[[:alnum:]]*$ ]]; then
       printf "%s\n" "ERROR: Invaild Group name, "${_grp}" may only be alpha-numeric " && exit 1;
     else
       printf "%s\n" "Arguments where not passed to script " && exit 1;
     fi
   elif [ "${?}" -eq 1 ] && [ -n "${_usr}" ] && [ -n "${_full_name}" ] && [[ ! "${_usr}" =~ ^[[:alnum:]]*$ ]] || [[ ! "${_full_name}" =~ ^[[:alnum:]' '.]*$ ]]; then
     printf "%s\n" "ERROR: Invaild user or full name, "${_usr}" and or "${_full_name}" may only be alpha-numeric " && exit 1;
   else
     printf "%s\n" "Arguments where not passed to script " && exit 1;
   fi
   if [ "${?}" -eq 0 ]; then
     user_exist_func "${_usr}" "${_full_name}";
     group_exist_func "${_grp}";
   else
     print_usage >&2 && exit 1;
   fi
  }
function user_exist_func()
  {
   if [ "${?}" -eq 0 ]; then
     grep -Gwe "^${_usr}" /etc/passwd > /dev/null;
     if [ "${?}" -eq 0 ]; then
       printf "%s\n" "Warning: User "${_usr}" already exists " && unset "${_usr}";
       elif [ "${?}" -eq 1 ]; then
         echo > /dev/null;
       else
         printf "%s\n" "Return value greater than 1 ";
     fi
   fi
  }
function group_exist_func()
  {
   if [ "${_grp}" != "${_usr}" ]; then 
     for i in "${_grp}" "${_usr}"; do
       grep -Gwe "^${i}" /etc/group > /dev/null;
       if [ "$?" -eq 0 ]; then
         printf "%s\n" "Warning: Group "${i}" already exists " && unset "${i}";
       elif [ "${?}" -eq 1 ]; then
         echo > /dev/null;
       else
         printf "%s\n" "Return value greater than 1 ";
       fi
     done
   else
     for x in "${_grp}"; do
       grep -Gwe "^${x}" /etc/group > /dev/null;
       if [ "$?" -eq 0 ]; then
         printf "%s\n" "Warning: Group "${x}" already exists " && unset "${x}";
       elif [ "${?}" -eq 1 ]; then
         echo > /dev/null;
       else
         printf "%s\n" "Return value greater than 1 ";
       fi
     done
   fi
  }
if [ "${?}" -eq 0 ] && [ "${#}" = 3 ] && [ -n "${1}" ] && [ -n "${2}" ] && [ -n "${3}" ]; then
  _usr="${1}"
  _full_name="${2}"
  _grp="${3}"
   clean_input_func "${_usr}" "${_full_name}" "${_grp}";
elif [ "${?}" -eq 1 ] && [ -z "${*}" ]; then
  print_usage >&2 && exit 1;
fi

Thank you everyone very much for your time

rtmistler · 05-31-2013, 11:42 AM

David the H said it first, and looks like you rose up to the challenge perfectly.

Scripting or programming does not have to be "cute". So instead of writing it in that highly complicated form you started with, you have it in functions, as well as composed of re-usable steps which you can then use in other program logic should you move forwards with another script which does similar or more things.

I wish on a different thread, that the originator of the thread could understand this and probably will reference this thread over on that one.

David the H. · 05-31-2013, 11:42 AM

Yes. The true command is synonymed to ":", so either that or the full name can be used. true does nothing except instantly terminate with a successful exit code, so it's often used as a kind of stub action. There's also false, if you need a command that necessarily fails, but it doesn't have any synonyms.

globbing, a.k.a. wildcards, is a simplified pattern-matching system that has two uses in the shell. It's used to expand into lists of filenames to operate on, and, in case statements and [[..]] tests and the like, it's used for testing matching strings. It's not as powerful as regex, but it's usually faster and more efficient. Some other programs like find also use globbing patterns, so it's one of those basic things everyone should learn.

Being a bash lover myself, I don't necessarily agree with jpollard.

I think the shell is under-rated by many, and an experienced hand can do some amazing things with it. I will agree that it's not always the best choice, however, particularly if performance is an issue. perl/python/ruby are certainly faster and more powerful, if you know how to use them. In any case learning the shell can be a good stepping point towards learning other languages, as many of the basic concepts are transferable, and you can become productive in it relatively quickly.

Glancing through your code above, I don't see much more in particular to comment on right now that I haven't mentioned already. I applaud your use of functions, which many new coders tend to avoid, and your variable names are generally well chosen, although the use of underscores in everything seems a bit superfluous to me.

I'm only going to mention one more tip right now, concerning lines like this:

Code:

grep -Gwe "^${_usr}" /etc/passwd > /dev/null;
if [ "${?}" -eq 0 ]; then

The "[..]" test is actually a stand-alone command, not part of the if statement itself, and outputs an exit code. And the only thing structures like if/for/while etc. are concerned about is the exit code of the command that runs after the initial keyword. So you can simplify the above like this:

Code:

if grep -wq "^$_usr" /etc/passwd; then

Note too the use of -q to silence grep's output instead of redirecting it to /dev/null. The -G option isn't needed, since it's the default, and neither is -e, since there's only one expression to process. A large part of shell shell scripting is understanding how to use the individual programs available to you.

Spazztic_Killer · 06-02-2013, 12:44 PM

All I can say is thank you very much, could you show an example of a regex vs. globbing. And I redid the script. David, I've got a question why you choose to use echo, in the code in the arguments part instead of printf or even print or is there even one?

And code you example this the part I guess that confuses me is this, partially the ($usr =~ ||). I would think that it would have to be && and I tried it and it didn't work.

Code:

if [[ $status -eq 0 && ( $usr =~ $re1 || $full_name =~ $re2 || $grp =~ $re1 ) ]]

I think it looks a lot cleaner now, and the if grep is much cleaner and useful than the way I had it. Thank you again everyone!!

Code:

#!/bin/bash
#set -xv
set -e
status=$?

function print_usage()
  {
   clear;
     cat<<_USAGE_
NOTE: Only the full name may contain a space, and you must quote that input to get it to take !
NOTE: The user is checked for in both the passwd and group files under /etc 

      Usage: ${0##*/} { username } { fullname } { groupname }
             ${0##*/} { username } { "full name" } { groupname }
             ${0##*/} John J.Doe Admins
             ${0##*/} John "John Doe" Admins
             ${0##*/} John5 "J. Doe" John

_USAGE_
  }
function clean_input_func()
  {
   local re1='[^[:alnum:]]'
   local re2='[^[:alnum:] .]'
   local warnfmt='ERROR: Invaild user or full name, "%s" and or "%s" and or "%s" may only be alpha-numeric \n'

   if [[ -n $usr && -n $full_name && -n $grp ]]; then

      if [[ $status -eq 0 && ( $usr =~ $re1 || $full_name =~ $re2 || $grp =~ $re1 ) ]]; then

         printf "$warnfmt" "$usr" "$full_name" "$grp" && exit 1

      fi

   else

      echo "Arguments were not passed to script " && exit 1

   fi 

   if [[ $status -eq 0 ]]; then

      user_exist_func "$usr" "$full_name" && group_exist_func "$grp";
   
   else

      print_usage >&2 && exit 1;
   
   fi
   unset re1 re2 warnfmt
  }

function user_exist_func()
  {
   local warnfmt='Warning: User "%s" already exists \n'

   if grep -wq "^$usr" /etc/passwd; then

      if [[ $status -eq 0 ]]; then

         printf "$warnfmt" "$usr" && unset $usr;

      else

         :

      fi

   fi
   unset warnfmt
  }

function group_exist_func()
  {
   local warnfmt='Warning: Group "%s" already exists \n'

   if [[ $grp != $usr ]]; then

      for i in $grp $usr; do

          if grep -wq "^$i" /etc/passwd; then

                if [[ $status -eq 0 ]]; then

                   printf "$warnfmt" "$i" && unset $i;

                else

                   :

                fi

          fi
       
      done

   else

      for x in $grp; do

          if grep -wq "^$x" /etc/passwd; then

                if [[ $status -eq 0 ]]; then

                   printf "$warnfmt" "$x" && unset $x;

                else

                   :

                fi

          fi

      done

   fi
   unset warnfmt
  }

if [[ $# = 3 && -n $1  && -n $2 && -n $3 ]]; then

   usr=$1 && full_name=$2 && grp=$3

   clean_input_func $_usr $_full_name $_grp;

elif [[ $# != 3 || -z $1 || -z $2 || -z $3 ]]; then
  
     print_usage >&2 && exit 1;

fi

And I was wondering how you would clean this bloody mess up. Its a little piece of my "find old files" scipt. Would the scipt does it test whether the user passed any arguments to it and if so then it does the code below. If not then it runs through a menu driven part of the script I created through a series of case, select statements.

Code:

if [ "${?}" -eq 0 ] && [ "${#}" = 5 ] && [ -d "${1}" ] && [ "${2}" = 'days' ] && [ "${3}" = '+' ] && [[ "${4}" =~ ^[0-9]+$ ]] && [ "${5}" == 'access' ]; then
  _dir="${1}"; time_scale='time'; _opt='+'; _time_value="${4}"; amc='access';
  search_type_func "${_dir}" "${time_scale}" "${_opt}" "${_time_value}" "${amc}";
  elif [ "${?}" -eq 1 ] && [ "${#}" = 5 ] && [ -d "${1}" ] && [ "${2}" = 'days' ] && [ "${3}" = '+' ] && [[ "${4}" =~ ^[0-9]+$ ]] && [ "${5}" == 'modification' ]; then
  _dir="${1}"; time_scale='time'; _opt='+'; _time_value="${4}"; amc='modification';
  search_type_func "${_dir}" "${time_scale}" "${_opt}" "${_time_value}" "${amc}";
  elif [ "${?}" -eq 1 ] && [ "${#}" = 5 ] && [ -d "${1}" ] && [ "${2}" = 'days' ] && [ "${3}" = '+' ] && [[ "${4}" =~ ^[0-9]+$ ]] && [ "${5}" == 'change' ]; then
  _dir="${1}"; time_scale='time'; _opt='+'; _time_value="${4}"; amc='change';
  search_type_func "${_dir}" "${time_scale}" "${_opt}" "${_time_value}" "${amc}"

David the H. · 06-02-2013, 04:01 PM

You can find globbing tutorials all over the net, starting with the link I gave before. Perhaps the biggest difference between it and regex is that a globbing pattern must always match the entire string. It can't do substring matching.

Code:

foo*    #any string starting with "foo"  regex: '^foo'
*foo    #any string ending with "foo"    regex: 'foo$'
*foo*   #any string that contains "foo"  regex: 'foo'

*[abc]*   #any string containing "a", "b", or "c"  regex: '[abc]'
*[^abc]*  #any string containing at least one character that's not a-c  regex: '[^abc]'

echo is a simple command that prints the given arguments, followed by a newline. If you don't need anything fancier, use it. KISS.

But do use printf, OTOH, when you have more complex output to deal with. It's in-built looping ability in particular can come in handy when you want to output lists of things.

There is no print command in bourne-type shells.

next:

Code:

if [[ $status -eq 0 && ( $usr =~ $re1 || $full_name =~ $re2 || $grp =~ $re1 ) ]]

Notice the use of (..) grouping brackets. The test will return true if ( $status is 0 ) AND ( $usr matches regex1 OR $group matches regex1 OR $full_name matches regex2 ). Only one of the regex matches needs to be true for the whole expression to be true, provided the status part is also successful. If you used && then all three regex tests must match before the test will succeed.

I'm sorry, but I don't really have the time or energy right now to look at your last code block, so why don't you take a stab at it yourself first? Start by cleaning up the formatting and replace the individual single-bracket tests with combined double-bracket ones. Then you can work from there to analyze and improve upon the logic. Post what you come up with and I or another regular can help you from there.

As a general suggestion though, stop trying to do too much in a single step. I'd probably try to limit any single test to three or four sub-expressions, max; anything more than that tends to get confusing. Start by running stand-alone sanity tests on arguments that absolutely have to be set in a particular way, then follow up with the tests that depend on specific input values.

You might also consider using case statements instead for some of your string matches. (I'm thinking of your $5 match specifically.)

Good luck!

Spazztic_Killer · 06-02-2013, 04:33 PM

Thank you very much