1
\$\begingroup\$

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

\$\endgroup\$
0

1 Answer 1

1
\$\begingroup\$

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."
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.