[Toybox] [PATCH] mktemp: yet more tests, and yet more fixes.

[Rob Landley]

On 12/6/18 10:52 AM, enh via Toybox wrote:
> We can't reuse the password.c code for random ASCII salts because that
> allows '/' (plus it seems to generate sequences of trailing '.'s for
> some reason). Do the simplest thing that could possibly work instead.

I need a week sometime to properly put the user account management stuff into
mkroot and get it all promoted.

Part of the reason it's further down my todo list is it's no use to Android
because PIDs mean something else there and login data would go in your version
of the registry instead of being unix-style text files anyway...

> ---
>  tests/mktemp.test |  6 ++++++
>  toys/lsb/mktemp.c | 34 ++++++++++++++++++++++++++++------
>  2 files changed, 34 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/mktemp.test b/tests/mktemp.test
> index ee023d6b..0c235469 100755
> --- a/tests/mktemp.test
> +++ b/tests/mktemp.test
> @@ -37,3 +37,9 @@ testing "-p DIR -t TEMPLATE but no TMPDIR" "TMPDIR=
> mktemp -u -p DIR -t hello.XX
> 
>  # mktemp -u doesn't need to be able to write to the directory.
>  testing "-u" "mktemp -u -p /proc | grep -q '^/proc/tmp\...........$'
> && echo yes" "yes\n" "" ""
> +
> +# mktemp needs at least XX in the template.
> +testing "bad template" "mktemp -u helloX || echo error" "error\n" "" ""

The one on ubuntu 14.04 required three XXX, so that's what I checked for...

> +# mktemp -q shouldn't print the path.
> +testing "-q" "mktemp -p /proc -q || echo only-failure" "only-failure\n" "" ""
> diff --git a/toys/lsb/mktemp.c b/toys/lsb/mktemp.c
> index 57d1d118..55ab1aff 100644
> --- a/toys/lsb/mktemp.c
> +++ b/toys/lsb/mktemp.c
> @@ -37,6 +37,8 @@ void mktemp_main(void)
>    int template_dir = template && !!strchr(template, '/');
>    int flags_dir = (toys.optflags & (FLAG_p|FLAG_t));
>    int use_dir = flags_dir && !template_dir;
> +  char *s, *e;
> +  int len;
> 
>    if (template_dir && flags_dir) error_exit("conflicting directories given");
> 
> @@ -61,12 +63,32 @@ void mktemp_main(void)
>    // TODO: coreutils cleans paths, so -p /t/// would result in /t/xxx...
>    template = use_dir ? xmprintf("%s/%s", TT.p, template) : xstrdup(template);
> 
> -  if (toys.optflags & FLAG_u) {
> -    template = mktemp(template);
> -  } else if (toys.optflags & FLAG_d ? !mkdtemp(template) :
> mkstemp(template) == -1) {
> -    if (toys.optflags & FLAG_q) toys.exitval = 1;
> -    else perror_exit("Failed to create %s %s/%s",
> -        toys.optflags & FLAG_d ? "directory" : "file", TT.p, template);
> +  // Point `s` and `e` to the start and end of the last region of XXXXXXes.
> +  s = e = strrchr(template, 'X');
> +  if (!e || e == template || *(e-1) != 'X') error_exit("need XX in template");
> +  while (s >= template && *(s-1) == 'X') --s;
> +  len = (e-s+1);
> +
> +  while (1) {
> +    struct stat sb;
> +    int i;
> +
> +    xgetrandom(toybuf, len, 0);
> +    for (i = 0; i < len; ++i) {
> +      // mktemp randomness is only from "A-Za-z0-9".
> +      s[i] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
> +      "abcdefghijklmnopqrstuvwxyz"
> +      "0123456789"[toybuf[i] % (26*2+10)];
> +    }

I wanted to avoid a "long long" division on 32 bit systems pulling in the
function unnecessarily, and you only need 2 more chars for 64, and the "posix
portable file character set" thing has 3 more (- . and _).

> +    if ((FLAG(u) && lstat(template, &sb) == -1 && errno == ENOENT) ||
> +        (FLAG(d) && mkdir(template, 0700) != -1) ||
> +        (open(template, O_CREAT|O_CLOEXEC, 0500) != -1)) break;
> +    if (errno == EEXIST) continue;
> +    if (FLAG(q)) {
> +      toys.exitval = 1;
> +      return;
> +    } else perror_exit("%s", template);

I didn't see this until just now (see "list mass unsubscribe again"), but I'll
try to take a proper look this weekend and see what I missed.

Rob