Bash scripting error (unexpected end of line)

Fraeco · 01-05-2012, 09:24 AM

I've been writing a script to maintain users on our new ftp server.

When I try to run it I'm getting a rather annoying "/addftp: line 65: syntax error: unexpected end of file" error.

I've been looking arround a bit and found some reference to not closing quotes etc, but I think I've been over the script a dozen times now, and haven't found any open quotes or anything else out of the ordinary.

Can you guys take a look at the script and see what I am missing?

Code:

#!/bin/bash

USERFILE="/etc/vsftpd/virtual-users.txt"
DBFILE="/etc/vsftpd/virtual-users.db"

function userdata {
        echo $username > $USERFILE
        echo $password >> $USERFILE
        db4.8_load -T -t hash -f $USERFILE $DBFILE
        rm $USERFILE
}

choice=4
echo "1. Create a new user"
echo "2. Change the password of an existing user"
echo "3. Disable an existing user"
echo -n "Choose wisely"
while [ $choice -eq 4 ] ; do
read choice

if [ $choice -eq 1 ] ; then
        echo "Starting FTP virtual user creation."
        echo "Please enter the new username"
        read username
        echo "Please choose a password for the new user"
        read -s password
        userdata
        echo "Creating user homefolder"
        mkdir /home/ftp/$username
        chown ftp:ftp /home/ftp/$username
        echo "User $username with homedirectory /home/ftp/$username created."

else

        if [ $choice -eq 2 ] ; then
        echo "Changing user password"
        echo "Please enter the username"
        read username
        echo "Please enter the new password"
        read -s password
        userdata
        echo "Password of $username changed"

        else
                if [ $choice -eq 3 ] ; then
                echo "Disabling FTP user"
                echo "Please enter the username"
                read username
#               echo "Delete existing user folder and contents? y/n"
#               read delete
#                       if [ $delete -eq y ] ; then
#                       echo "Removing /home/ftp/$USERNAME"
#                       rm -R /home/ftp/$USERNAME
#                       else
                password=accessnomoreyoucanhaz
                userdata
                else
                echo "Please choose a valid option"
                choice=4
                fi
        fi
fi

The commented part is still a work in progress, so don't really need to bother much with that

thesnow · 01-05-2012, 09:36 AM

You're missing the "done" after the "do" to end the while loop.

However, do you really need the while loop? The way it's constructed it's only going to run through once, then quit, because choice is only 4 initially, not selected as an option by the user. You could just take the whole "while" line out, read "choice" and the rest of the script should run based on the selection.

soppy · 01-05-2012, 09:52 AM

Here is the fixed code....There were some errors that I'd like to point out. This runs flawlessly now.

Code:

#!/bin/bash

USERFILE="/etc/vsftpd/virtual-users.txt"
DBFILE="/etc/vsftpd/virtual-users.db"

function userdata {
        echo $username > $USERFILE
        echo $password >> $USERFILE
        db4.8_load -T -t hash -f $USERFILE $DBFILE
        rm $USERFILE
}

choice=4
echo "1. Create a new user"
echo "2. Change the password of an existing user"
echo "3. Disable an existing user"
echo -n "Choose wisely: "

read choice

if [ $choice -eq 1 ] ; then
        echo "Starting FTP virtual user creation."
        echo "Please enter the new username"
        read username
        echo "Please choose a password for the new user"
        read -s password
        userdata
        echo "Creating user homefolder"
        mkdir /home/ftp/$username
        chown ftp:ftp /home/ftp/$username
        echo "User $username with homedirectory /home/ftp/$username created."

	elif [ $choice -eq 2 ] ; then
		echo "Changing user password"
		echo "Please enter the username"
		read username
		echo "Please enter the new password"
		read -s password
		userdata
		echo "Password of $username changed"

        elif [ $choice -eq 3 ] ; then
               echo "Disabling FTP user"
               echo "Please enter the username"
               read username
#           echo "Delete existing user folder and contents? y/n"
#               read delete
#                       if [ $delete -eq y ] ; then
#                       echo "Removing /home/ftp/$USERNAME"
#                       rm -R /home/ftp/$USERNAME
#                       else
            password=accessnomoreyoucanhaz
            userdata
         
         else
                echo "Please choose a valid option"
                choice=4
fi

Now for an explanation...

Code:

while [ $choice -eq 4 ] ; do

I take it you're a compiled language programmer (C/C++)? This is a common mistake. Read will already pause for input, we don't need to create this loop here. And if we did, we'd have to close it off with a "done".

Code:

if [ $choice -eq 1 ] ; then
        echo "Starting FTP virtual user creation."
        echo "Please enter the new username"
        read username
        echo "Please choose a password for the new user"
        read -s password
        userdata
        echo "Creating user homefolder"
        mkdir /home/ftp/$username
        chown ftp:ftp /home/ftp/$username
        echo "User $username with homedirectory /home/ftp/$username created."

else

        if [ $choice -eq 2 ] ; then
        echo "Changing user password"
        echo "Please enter the username"
        read username
        echo "Please enter the new password"
        read -s password
        userdata
        echo "Password of $username changed"

        else
                if [ $choice -eq 3 ] ; then
                echo "Disabling FTP user"
                echo "Please enter the username"
                read username
#               echo "Delete existing user folder and contents? y/n"
#               read delete
#                       if [ $delete -eq y ] ; then
#                       echo "Removing /home/ftp/$USERNAME"
#                       rm -R /home/ftp/$USERNAME
#                       else
                password=accessnomoreyoucanhaz
                userdata
                else
                echo "Please choose a valid option"
                choice=4
                fi
        fi
fi

You have NO IDEA how close you were here...except these else and if statements won't work in eachother. Instead, you need to use the "elif" command (else if) to test the number. So if it's 1, do this. Else if it's 2, do this. Else if it's 3...etc etc. Bash simply doesn't like the way you did it and it got lost. It's very picky like that...

To be honest, I wouldn't even use all those else if statements, I would've used a switch case. It's a lot cleaner and less code.

Hope I helped out!!!

David the H. · 01-05-2012, 05:22 PM

I second that recommendation. Use a case statement instead (shells don't use the word "switch"). It's almost always better for evaluating the contents of a single variable for a set list of values. Use if/elif/else when the tests you need are more complex and varied (i.e. if $a = foo, elif $b != bar, elif $c = baz && $d != bum...).

Two more general points about scripting in bash:

1) It's recommended to use ((..)) for numerical tests, and [[..]] for string tests and other complex expressions. Don't use the old [..] test unless you specifically need POSIX-style portability.

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

2) QUOTE ALL OF YOUR VARIABLE SUBSTITUTIONS. You should never leave the quotes off a variable expansion unless you explicitly want the resulting string to be word-broken by the shell. This is a vitally important concept in scripting, so train yourself to do it correctly now. You can learn about the exceptions later.

http://mywiki.wooledge.org/Arguments
http://mywiki.wooledge.org/WordSplitting
http://mywiki.wooledge.org/Quotes

Fraeco · 01-06-2012, 04:29 AM

The entire "case" idea is great!

I've rewritten to this

Code:

#!/bin/bash
USERFILE="/etc/vsftpd/virtual-users.txt"
DBFILE="/etc/vsftpd/virtual-users.db"

function userdata {
        echo "$username" > "$USERFILE"
        echo "$password" >> "$USERFILE"
        db4.8_load -T -t hash -f "$USERFILE" "$DBFILE"
        rm "$USERFILE"
}

echo "1. Create a new user"
echo "2. Change the password of an existing user"
echo "3. Disable an existing user"
echo -n "Choose wisely: "
read choice

case "$choice" in
1)
        echo "Starting FTP virtual user creation."
        echo "Please enter the new username"
        read username
        echo "Please choose a password for the new user"
        read -s password
        userdata
        echo "Creating user homefolder"
        mkdir /home/ftp/$username
        chown ftp:ftp /home/ftp/$username
        echo "User $username with homedirectory /home/ftp/$username created."
        ;;
2)
        echo "Changing user password"
        echo "Please enter the username"
        read username
        echo "Please enter the new password"
        read -s password
        userdata
        echo "Password of $username changed"
        ;;

3)
        echo "Disabling FTP user"
        echo "Please enter the username"
        read username
                echo -n "Delete existing user folder and contents? y/n"
                read delete
                case "$delete" in
                [yY] | [yY][Ee][Ss] )
                        echo "Removing /home/ftp/$username"
                        rm -R /home/ftp/$username
                        ;;
                [nN] | [n|N][O|o] )
                        echo "Not removing /home/ftp/$username"
                        ;;
                *)
                        echo "You failed! Not removing anything now. Take the next few seconds to think about yourself"
                        sleep 10
                        ;;
                esac
        password=accessnomoreyoucanhaz
        userdata

        ;;
*)
        echo "Please choose a valid option. 1, 2 or 3"
esac

Alltough, there's one minor problem. Is there a possibility to let the script go back to top if the case is "*"?

David the H. · 01-06-2012, 07:52 PM

Sure! Just embed it inside a while true loop!

Then use a break command to exit the loop when you want it to. Make sure the read is part of the loop, of course, so you have new input to test each time.

It's the traditional way to create a repeating menu in scripting.

You can also choose to use a select loop instead, which is a built-in that provides a pre-configured looping menu. It pretty much just does the same thing in the end though.

http://wiki.bash-hackers.org/syntax/ccmd/user_select
http://www.tldp.org/LDP/abs/html/testbranch.html