search

zakjan / cert-chain-resolver

SSL certificate chain resolver
MIT License
807 stars 85 forks source link

Get rid of rm -rf? #1

Closed erkie closed 9 years ago

erkie commented 9 years ago

This script is freakin awesome. I always encounter these problems when working SSL certs.

However, looking through script, the rm -rf $TMP_DIR phase is a bit scary. Could this be changed to something less dangerous? I'm no bash pro, so I might just be overly cautious. If something goes wrong in the script or $TMP_DIR is accidentally set to / somebody might have a very bad time.

Sorry that I can't provide you with a suggested pull request, just wanted to start a discussion. What do you think?

zakjan commented 9 years ago

TMP_DIR is path to a fresh temporary directory created with mktemp -d and is never changed. I wanted to avoid conflicts with files in current directory. But yes, I agree with you, on the first sight it doesn't look good.

erkie commented 9 years ago

So I tried mktemp -d on Mac OS X, and it only prints:

usage: mktemp [-d] [-q] [-t prefix] [-u] template ...
       mktemp [-d] [-q] [-u] -t prefix

So I'm not sure if the following will break the script prematurely but this is what might happen if it runs on Mac:

$ TMP_DIR=$(mktemp -d)
  usage: mktemp [-d] [-q] [-t prefix] [-u] template ...
         mktemp [-d] [-q] [-u] -t prefix
$ echo "rm -rf $TMP_DIR"
rm -rf

I'm terribly sorry if this is totally wrong. :) And again, I wish I could come with some more constructive ideas!

zakjan commented 9 years ago

What if you run mktemp -d XXXXX? Apparently OSX requires the template argument.

But I'm already thinking about how to get rid of it :)

zakjan commented 9 years ago

mkdir issue fixed in https://github.com/zakjan/cert-chain-resolver/commit/3f4a5770c0c6e589aa8a994c362c57de76f0aebc

zakjan commented 9 years ago

rm -rf issue fixed, closing.