Simple Bash Script Not Working

Question

I'm having issues with a shell script I'm working on that for whatever reason is not working as intended. The bulk of the script, and where things are going wrong, is below:

for username in $list
do
   sleep $sleep
   VAL=`curl -H 'Client-ID: $clientid' -s $host$username | jq -r .message | grep -E "unavailable|not found"`
   if [[ $VAL == *"not found"* ]]; then
     echo "$username is available"
     echo "$username" >> available.names
   else
     echo -e "$username is reserved"
   fi
done

I realize there are some variables such as sleep, host, clientid, and username but to give you an idea of what I'm working with, the result of the curl command ran at the VAL line (minus the grep) would be something like this:

User "username" was not found
or
User "username" is unavailable

So including the pipe to grep -E, the result of VAL would simply be something like:

not found

Now according to this StackOverflow answer, I should be able to wildcards so that, when using my script, if the VAL contains "not found" then it should echo that the username is available. As I've done here:

[[ $VAL == *"not found"* ]]

However what it's doing is the else statement for all usernames that are being checked. So even ones where the VAL contains "not found", it's still echoing that the username is taken.

Does anyone see anything wrong with my script that would cause this to happen? I've looked it over about 100 times and I'm getting no errors to help troubleshoot, only that it's not working as intended. Any help would be appreciated.

In your curl command, you put single quotes around the -H string, which prevents variables from being expanded. Use double quotes. — Benjamin W.
– Benjamin W., Commented Oct 15, 2017 at 19:05
BTW, for username in $list is a code smell -- it indicates that you're trying to store a list in a string rather than using a native array type. A string is the wrong tool for the job -- not only does it restrict what values you can store, but it also means that if you have a value of * it'll be replaced with a list of files in the current directory. Use an array instead: list=( allison bob charlie ); for username in "${list[@]}"; do ... — Charles Duffy
– Charles Duffy, Commented Oct 15, 2017 at 19:17
BTW, it would probably help you narrow down where issues are coming from (and thus ask more focused questions in the future) if you make a habit of debugging with bash -x yourscript to log every command run -- that way if (for example) an expansion isn't taking place somewhere you expect it to, you can see it in the log and ask a question about that one specific line/operation. — Charles Duffy
– Charles Duffy, Commented Oct 15, 2017 at 19:22

janos · Accepted Answer · 2017-10-15 19:48:41Z

1

The single-quote in -H 'Client-ID: $clientid' is clearly wrong, as the value of $clientid is not expanded that way. Use double-quotes instead.

And btw you don't need [[ $VAL == *"not found"* ]], you can write a conditional directly using the exit code of grep of the pipeline:

for username in $list
do
   sleep $sleep
   if curl -H "Client-ID: $clientid" -s "$host$username" | jq -r .message | grep -qE "unavailable|not found"; then
     echo "$username is available"
     echo "$username" >> available.names
   else
     echo "$username is reserved"
   fi
done

As @CharlesDuffy pointed out in a comment, the echo -e made no sense, so I removed that too. He also pointed out the code smell in writing for username in $list, instead of using proper arrays. (list=( allison bob charlie ); for username in "${list[@]}"; do ...)

edited Oct 15, 2017 at 19:48

answered Oct 15, 2017 at 19:10

janos

126k31 gold badges242 silver badges253 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Charles Duffy Over a year ago

Is echo -e ever the right thing here? Even if the username contains a literal \t or \n sequence, I'd think you'd want to print it as the two-character sequence it is.

Eric Over a year ago

Thanks to both of you for the help. I should have caught the single quotes so that was just me being irresponsible with my code. However I didn't realize that using for username in $list could potentially cause the mentioned issues, so I need to look into using an array instead as suggested. It's a simple script, but I still need to use best practice no matter the purpose of the script. So again I appreciate the both of you for the help!

Collectives™ on Stack Overflow

Simple Bash Script Not Working

1 Answer 1

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related