How can I refactor this bash script?

Question

exportsData=$(curl -X GET \
 -H 'Accept: application/vnd.xxxxx+json; version=3.0' \
 -H 'Authorization: Bearer RCexxxxxxxxxxxxxxxxxxxxxxxxxcVefI5mWy' \
 -H 'Content-Type: application/json' \
https://api.xxxx.com/apps/9xxxxxxxxx15f96fe/exports)
status=$(echo $exportsData | jq '.[0].status')
downloadURL=$(echo $exportsData | jq '.[0].download_url') 
export ENCRYPTED_AES_KEY=$(echo $exportsData | jq '.[0].encrypted_aes_key')
export AES_IV=$(echo $exportsData | jq '.[0].aes_iv')

export ENCRYPTED_TARBALL=encryptedChatDump.tar.gz.enc
$(curl -X GET -o ENCRYPTED_TARBALL \
 -H 'Accept: application/vnd.xxxxx+json; version=3.0' \
 -H 'Authorization: Bearer RCexxxxxxxxxxxxxxxxxxxxxxxxxcVefI5mWy' \
 -H 'Content-Type: application/json' \
https://storage.googleapis.com/someUrlWhereTheFileIsBeingDownloadedFrom)

export OUTPUT_TAR=finalChatDumpUnencrypted.tar.gz
export PRIVATE_KEY_PATH=~/.ssh/id_rsa


openssl enc -in $ENCRYPTED_TARBALL -out $OUTPUT_TAR -d -aes-256-cbc |   base64 --decode | openssl rsautl -decrypt -inkey $PRIVATE_KEY_PATH | base64   --decode

This is my first script and I am having a hard time writing the shortest possible code.

You should double-quote all variable references (e.g. `echo "$exportsData"`) to avoid unexpected parsing problems. — Gordon Davisson, Dec 16 '17 at 19:42
Welcome to StackOverflow! If you posted credentials in this question, please be aware that even if you have since edited them, those of us with high enough rep can still see the old version. You should rotate those credentials. IDK if there's anything moderators can do, but I will flag this for review. — kojiro, Dec 16 '17 at 20:27
If you have working code and want to improve it, consider asking in https://codereview.stackexchange.com/ — kojiro, Dec 16 '17 at 20:30
Oh, and as always, consider running your shell scripts through [shellcheck](https://github.com/koalaman/shellcheck) — kojiro, Dec 16 '17 at 20:42
In general, by the way, this kind of question is broader than we consider on-topic here; a good StackOverflow question isolates a single, very specific problem and is amenable to a canonical answer. The pointer to [codereview.se] is likely a good one -- see https://codereview.meta.stackexchange.com/a/5778/155649 describing the difference in scope between the two StackExchange sites, and how questions that are on-topic on CR are typically too broad to be allowed on SO. — Charles Duffy, Dec 16 '17 at 20:48
(You might also make a habit of running your code through http://shellcheck.net/ and fixing what it finds before asking questions about shell scripts on *either* CR or SO). — Charles Duffy, Dec 16 '17 at 20:49
BTW -- in general, you should only `export` variables when you have a good reason to do so, rather than just assigning them as regular shell variables scoped to the local process and its subshells. Environment variables live in the same limited/restricted amount of space used for command-line arguments, so the more content you `export`, the shorter the longest permitted set of command-line arguments for any program started by the same shell or its children is. — Charles Duffy, Dec 16 '17 at 21:13

ewcz · Answer 1 · 2017-12-16T20:59:47.690

3

you could process the variables in a loop:

for key in status download_url encrypted_aes_key aes_iv
do
    val=$(echo "${exportsData}" | jq ".[0].${key}")
    echo "${val}"
done

EDIT:

in order to store the variables for further "processing", you might create them explicitly in the do loop as:

for key in status download_url encrypted_aes_key aes_iv
do
    val=$(echo "${exportsData}" | jq ".[0].${key}")
    declare -x "$(echo ${key} | tr '[a-z]' '[A-Z]')"="${val}"
done

this will create (and export) upper-cased variables STATUS, DOWNLOAD_URL, etc.

edited Dec 16 '17 at 20:59

answered Dec 16 '17 at 19:35

ewcz

12,819
1
25
47

Thanks a ton @ewcz !! Could you also tell me how to put an if condition as if (status == "pending") – frigocat Dec 16 '17 at 20:31
Good answer. In addition I'd suggest that `jq` can do more of the work. Something like `jq '.[0]|(.status, .download_url, .encrypted_aes_key, .aes_iv)' <<< "$exportsData"`. You can go further using the `@sh` option and `ascii_upcase` to output the entire `declare` expression from `jq`. – kojiro Dec 16 '17 at 20:34
@frigocat my pleasure :) - for string comparison, see for example this [answer](https://stackoverflow.com/a/4277753/5351549) or the examples [here](http://tldp.org/LDP/abs/html/comparison-ops.html) – ewcz Dec 16 '17 at 20:37
@kojiro thanks for the suggestion, that looks like a much more elegant approach indeed! – ewcz Dec 16 '17 at 20:41
2

Using all-upper-case names for user-defined variables is actually bad practice -- such names are used for variables with meaning to the operating system or shell. See relevant standard at http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html, fourth paragraph, keeping in mind that environment variables and shell variables share a single namespace. – Charles Duffy Dec 16 '17 at 20:46
@ewcz My condition if [ "completed" == $STATUS ]; is failing .. even when echo $STATUS prints completed! Any clue why? – frigocat Dec 16 '17 at 20:49
1

@frigocat, several things broken about that. `==` isn't a valid string comparison operator in all shells -- the standard operator (which will work when your script is run with ash or dash, not just bash) is `=`. And you need to quote the `"$STATUS"` (to prevent it from being string-split and glob-expanded), not the `completed` (which will always parse to exactly one word, whether it's quoted or not). – Charles Duffy Dec 16 '17 at 20:50
1

Similarly, the answer here is presently buggy in that it uses `echo $exportsData` instead of `echo "$exportsData"` or just `<<<"$exportsData"`-- if your `exportsData` contains a whitespace-surrounded `*` in it somewhere, that will be replaced with a list of filenames; similarly, tabs and newlines can be changed to single spaces, etc. See also [BashPitfalls #14](http://mywiki.wooledge.org/BashPitfalls#echo_.24foo). – Charles Duffy Dec 16 '17 at 20:53
BTW, if you want to generate a mapping between keys and values, if you're guaranteed that the only shell you need to support is bash 4.0 or newer, consider an associative array instead. Much safer -- code that sets arbitrary environment variables can be used as part of an attack on your system's security (ie. `LD_LIBRARY_PATH` can be pointed to an executable library with hostile intent), but an associative array is effectively its own namespace, avoiding side effects. – Charles Duffy Dec 16 '17 at 20:54
1

@frigocat, ...btw, to get the *exact* value of a variable in unambiguous form, consider `printf '%q=%q\n' STATUS "$STATUS"` -- that'll show you if your `STATUS` variable has trailing whitespace, unprintable characters, or other surprising/interesting things going on. – Charles Duffy Dec 16 '17 at 20:57
@CharlesDuffy thank you for the useful comments! I will include the quotes in the `echo` statement, as for the arrays, I used the `declare` approach in case the OP would indeed need to export those variables... – ewcz Dec 16 '17 at 20:58
BTW, running a separate `jq` once per key to be extracted seems rather unfortunate -- no reason you couldn't have just one `jq` instance extract all the values you need in a single pass. A set of NUL-delimited key/value pairs is an entirely unambiguous output form (at least if you strip any NULs from the values, since JSON strings can contain `\u0000`); see several patterns suggested in comments responding to the ticket I filed at https://github.com/stedolan/jq/issues/1271 – Charles Duffy Dec 16 '17 at 21:01
@CharlesDuffy I am in love with you!! Thanks!! – frigocat Dec 16 '17 at 21:01

How can I refactor this bash script?

1 Answers1