0

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.

peak
  • 105,803
  • 17
  • 152
  • 177
Eric
  • 3
  • 2
  • 2
    In your curl command, you put single quotes around the `-H` string, which prevents variables from being expanded. Use double quotes. – Benjamin W. Oct 15 '17 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 Oct 15 '17 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 Oct 15 '17 at 19:22

1 Answers1

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 ...)

janos
  • 120,954
  • 29
  • 226
  • 236
  • 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. – Charles Duffy Oct 15 '17 at 19:15
  • 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! – Eric Oct 15 '17 at 19:58