Shell POSIX OpenSSL file decryption script follow-up #1

Question

This question is the first follow-up of: Shell POSIX OpenSSL file decryption script

Please read it before you continue reading this. Thank you.

---

I only have one new problem solved and I would like you to have a look.

That problem being accepting one option -o output_directory.

I have little idea if I did this the best way, but it works.

The code follows:

---

#!/bin/sh bold=$(tput bold) red=$(tput setaf 1) yellow=$(tput setaf 3) nocolor=$(tput sgr0) bold_red=${bold}${red} bold_yellow=${bold}${yellow} print_usage_and_exit() { echo "Usage: $0 [-o output_directory] filename_to_decrypt" exit 1 } print_error_and_exit() { echo "${bold_red}$1 Exit code = $2.${nocolor}" 1>&2 exit "$2" } while getopts ":o:" option do case "${option}" in o) given_output_directory=${OPTARG} ;; h | *) print_usage_and_exit ;; esac done shift $((OPTIND - 1)) [ "$#" -eq 0 ] && print_usage_and_exit [ "$#" -gt 1 ] && print_error_and_exit "Multiple files are not supported." 2 [ ! -f "$1" ] && print_error_and_exit "The given argument is not an existing file." 3 input_filename="$1" [ ! -r "$input_filename" ] && print_error_and_exit "Input file is not readable by you." 4 input_filepath=$(dirname "$input_filename") if [ -z ${given_output_directory} ] then output_directory="$input_filepath" else output_directory="$given_output_directory" fi [ ! -w "$output_directory" ] && print_error_and_exit "Destination directory is not writable by you." 5 filename_extracted_from_path=$(basename "$input_filename") filename_without_enc_extension="${filename_extracted_from_path%.enc}" if [ "$filename_extracted_from_path" = "$filename_without_enc_extension" ] then # the file has a different than .enc extension or no extension at all # what we do now, is that we append .dec extention to the file name output_filename="$output_directory/$filename_extracted_from_path".dec else # the file has the .enc extension # what we do now, is that we use the file name without .enc extension output_filename="$output_directory/$filename_without_enc_extension" fi [ -f "$output_filename" ] && print_error_and_exit "Destination file exists." 6 if ! pv -W "$input_filename" | openssl enc -aes-256-cbc -md sha256 -salt -out "$output_filename" -d 2> /dev/null then [ -f "$output_filename" ] && rm "$output_filename" print_error_and_exit "Decryption failed." 7 else echo "${bold_yellow}Decryption successful.${nocolor}" exit 0 fi 

---

Uploaded on GitHub: https://git.io/fxslm

Answer 1

Although print_error_and_exit correctly sends output to standard error and exits with a non-zero status, print_usage_and_exit outputs to standard out. I recommend explicitly accepting -h (as your case implies, but getopts hasn't been instructed to do), and if the user asks for help, then writing to standard out and returning success, but if the help is provided due to error, then writing to standard error and returning failure:

print_usage() { echo "Usage: $0 [-o output_directory] filename_to_decrypt" } while getopts ":ho:" option do case "${option}" in o) given_output_directory=${OPTARG} ;; h) print_usage exit 0 ;; '?') print_usage >&2 exit 1 ;; esac done shift $((OPTIND - 1)) [ "$#" -eq 0 ] && print_usage && exit 1 

---

While we're on error reporting, you might want to consider making the exit code be the first argument to print_error_and_exit rather than the last. That can make the code easier to read, and it also allows you to pass multiple arguments, just like echo which it wraps:

print_error_and_exit() { value=$1; shift echo "${bold_red}$* Exit code = $value.${nocolor}" 1>&2 exit "$value" } 

---

It's good to see that you use tput to produce the terminal-specific escape codes (if they exist) for highlighting. But it's probably not worth putting them into variables for a single use each. I'd get rid of the variables and just write something like

print_error_and_exit() { value=$1; shift exec >&2 tput bold tput setaf 1 # red echo "$* Exit code = $value." tput sgr0 exit "$value" } 

and (in the success branch)

else tput bold tput setaf 3 # yellow echo "Decryption successful." tput sgr0 exit 0 fi 

---

These tests are probably over-doing it:

[ ! -f "$1" ] && print_error_and_exit 3 "The given argument is not an existing file." input_filename="$1" [ ! -r "$input_filename" ] && print_error_and_exit 4 "Input file is not readable by you." 

Unless there's a good reason we can't accept input from a pipe or other non-regular file, just skip the -f test: -r will fail if its argument doesn't exist. It's also worth including the filename in the message - this is really useful when the user expands a variable into the call, and it doesn't expand to what she expects:

[ -r "$1" ] || print_error_and_exit 3 "$1: not a readable file." 

If you really feel you need different exit status for non-existent and existing-but-unreadable files (why?), then consider -e as an alternative to -f.

---

We should check whether the supplied output directory is actually a directory, before trying to use it:

[ -d "$output_directory" ] || print_error_and_exit 4 "$output_directory: not a directory." [ -w "$output_directory" ] || print_error_and_exit 5 "Destination directory is not writeable by you." 

---

A shorter way to write this:

if [ -z ${given_output_directory} ] then output_directory="$input_filepath" else output_directory="$given_output_directory" fi 

is to use parameter substitution with - modifier:

output_directory="${given_output_directory:-$input_filepath}" 

---

I think a case helps with matching filename patterns here:

if [ "$filename_extracted_from_path" = "$filename_without_enc_extension" ] then 

I'd write that block as:

filename_extracted_from_path=$(basename "$input_filename") # Strip .enc suffix from name if it has one, else add .dec suffix case "$filename_extracted_from_path" in *.enc) output_filename="${filename_extracted_from_path%.enc}" ;; *) output_filename="$filename_extracted_from_path".dec ;; esac output_filename="$output_directory/$output_filename" 

Note that there's no longer a need for $filename_without_enc_extension.

---

You definitely need to replace this -f with -e:

[ -f "$output_filename" ] && print_error_and_exit 6 "Destination file exists."