Merge lp:~mterry/snap-confine/tmpdir into lp:~snappy-dev/snap-confine/trunk

[Michael Terry]

Fix propagation of TMPDIR from the launcher to the command being run.

Fix propagation of TMPDIR from the launcher to the command being run.

Our logic for making sure that TMPDIR exists iterated over the return value of getenv("TMPDIR") with strtok. But strtok modifies its argument. And getenv returns the real environment memory used, so modifications there actually change your environment.

The end result is that we were emptying TMPDIR while making it.

I've also fixed a bug that caused us to stop trying to make TMPDIR if any piece of it (above /tmp) already existed (like, /tmp/snaps/!).

And of course, a test.

[John Lenton (chipaca)]

review: Approve

[lp:~mterry/snap-confine/tmpdir]

69. By Michael Terry on 2015-06-03

Check return of strdup, add comment about why we do it

70. By Michael Terry on 2015-06-03

Make it die instead of just returning

=== modified file 'src/main.c'
--- src/main.c	2015-05-29 20:20:46 +0000
+++ src/main.c	2015-06-03 13:12:41 +0000
@@ -245,17 +245,26 @@
         return;
     }
 
+    // strtok trashes its argument, so to avoid changing the TMPDIR variable,
+    // we make a copy.
+    dir = strdup(dir);
+    if (!dir) {
+        die("out of memory");
+    }
+
     int n = 4;
     char buf[MAX_BUF] = "/tmp";
     char *d = strtok(dir+4, "/");
     while (d) {
         n += must_snprintf(buf+n, MAX_BUF-n, "/%s", d);
-        if (mkdir(buf, 01777) < 0) {
-            return;
+        if (mkdir(buf, 01777) < 0 && errno != EEXIST) {
+            break;
         }
 
         d = strtok(NULL, "/");
     }
+
+    free(dir);
 }
 
 int main(int argc, char **argv)

=== added file 'tests/test_tmpdir'
--- tests/test_tmpdir	1970-01-01 00:00:00 +0000
+++ tests/test_tmpdir	2015-06-03 13:12:41 +0000
@@ -0,0 +1,34 @@
+#!/bin/sh
+
+set -e
+
+. $(pwd)/common.sh
+
+printf "Test that we create TMPDIR and propagate that variable intact"
+
+cat >$TMP/echotest <<EOF
+#!/bin/sh
+echo \$TMPDIR
+EOF
+chmod a+x $TMP/echotest
+
+cat >$TMP/unrestricted <<EOF
+@unrestricted
+EOF
+
+export TMPDIR="$(mktemp -du /tmp/snaps/XXXXXX)/foo/bar"
+test ! -e "$TMPDIR" # just sanity checking
+
+OUTPUT="$($L appname unrestricted $TMP/echotest)"
+
+if [ ! -d "$TMPDIR" ]; then
+    # we didn't create TMPDIR like we should have
+    FAIL
+fi
+
+if [ "$TMPDIR" != "$OUTPUT" ]; then
+    # we didn't propagate TMPDIR to the command
+    FAIL
+fi
+
+PASS