Correctly handle get of an empty object in the wallet client. The

empty string is valid object content.

Add a full end-to-end test suite to catch protocol mismatches between
the client and server, such as the one fixed in this release.

8 files changed, 70 insertions, 42 deletions

diff --git a/NEWS b/NEWS
index 110ca37..d17e4ef 100644
--- a/NEWS
+++ b/NEWS
@@ -6,10 +6,16 @@ wallet 0.8 (unreleased)
 
     Add file object support to the wallet server.
 
+    Correctly handle get of an empty object in the wallet client.  The
+    empty string is valid object content.
+
     Wallet::Config and hence the wallet server now checks for the
     environment variable WALLET_CONFIG and loads configuration from the
     file specified there instead of /etc/wallet/wallet.conf if it is set.
 
+    Add a full end-to-end test suite to catch protocol mismatches between
+    the client and server, such as the one fixed in this release.
+
 wallet 0.7 (2008-02-08)
 
     Add new exists and autocreate wallet server interfaces.  The first
diff --git a/client/file.c b/client/file.c
index 17f0f23..10304e5 100644
--- a/client/file.c
+++ b/client/file.c
@@ -32,11 +32,13 @@ overwrite_file(const char *name, const void *data, size_t length)
     fd = open(name, O_WRONLY | O_CREAT | O_EXCL, 0600);
     if (fd < 0)
         sysdie("open of %s failed", name);
-    status = write(fd, data, length);
-    if (status < 0)
-        sysdie("write to %s failed", name);
-    else if (status != (ssize_t) length)
-        die("write to %s truncated", name);
+    if (length > 0) {
+        status = write(fd, data, length);
+        if (status < 0)
+            sysdie("write to %s failed", name);
+        else if (status != (ssize_t) length)
+            die("write to %s truncated", name);
+    }
     if (close(fd) < 0)
         sysdie("close of %s failed (file probably truncated)", name);
 }
@@ -93,15 +95,17 @@ get_file(struct remctl *r, const char *prefix, const char *type,
     status = run_command(r, command, &data, &length);
     if (status != 0)
         return status;
-    if (data == NULL) {
-        warn("no data returned by wallet server");
-        return 255;
-    }
+
+    /* The empty string is valid data. */
+    if (data == NULL)
+        length = 0;
     if (file != NULL)
         write_file(file, data, length);
-    else {
+    else if (length > 0) {
         if (fwrite(data, length, 1, stdout) != 1)
             sysdie("cannot write to standard output");
     }
+    if (data != NULL)
+        free(data);
     return 0;
 }
diff --git a/perl/t/lib/Util.pm b/perl/t/lib/Util.pm
index 481b7d9..7b41e53 100644
--- a/perl/t/lib/Util.pm
+++ b/perl/t/lib/Util.pm
@@ -97,14 +97,15 @@ sub getcreds {
 sub remctld_spawn {
     my ($path, $principal, $keytab, $config) = @_;
     unlink 'test-pid';
+    my @command = ($path, '-m', '-p', 14373, '-s', $principal, '-P',
+                   'test-pid', '-f', $config, '-S', '-F', '-k', $keytab);
+    print "Starting remctld: @command\n";
     my $pid = fork;
     if (not defined $pid) {
         die "cannot fork: $!\n";
     } elsif ($pid == 0) {
         open (STDERR, '>&STDOUT') or die "cannot redirect stderr: $!\n";
-        exec ($path, '-m', '-p', 14373, '-s', $principal, '-P', 'test-pid',
-              '-f', $config, '-S', '-F', '-k', $keytab) == 0
-            or die "cannot exec $path: $!\n";
+        exec (@command) or die "cannot exec $path: $!\n";
     } else {
         my $tries = 0;
         while ($tries < 10 && ! -f 'test-pid') {
diff --git a/tests/TESTS b/tests/TESTS
index e19f290..5bae099 100644
--- a/tests/TESTS
+++ b/tests/TESTS
@@ -1,4 +1,5 @@
 client/basic
+client/full
 client/pod
 portable/asprintf
 portable/snprintf
diff --git a/tests/client/basic-t.in b/tests/client/basic-t.in
index 67d7a3a..26a020e 100644
--- a/tests/client/basic-t.in
+++ b/tests/client/basic-t.in
@@ -27,7 +27,7 @@ wallet='../client/wallet'
 # Start the remctld daemon and wait for it to start.
 principal=`cat data/test.principal`
 rm -f data/pid
-( @REMCTLD@ -m -p 14373 -s "$principal" -P data/pid -f data/wallet.conf \
+( @REMCTLD@ -m -p 14373 -s "$principal" -P data/pid -f data/basic.conf \
   -S -F -k data/test.keytab &)
 KRB5CCNAME=data/test.cache; export KRB5CCNAME
 kinit -k -t data/test.keytab "$principal" > /dev/null 2>&1
diff --git a/tests/client/full-t.in b/tests/client/full-t.in
index bae7aa3..83c0ce9 100644
--- a/tests/client/full-t.in
+++ b/tests/client/full-t.in
@@ -12,7 +12,7 @@
 # is loaded, and it's pulled in as a prerequisite for Wallet::Admin.
 BEGIN { $ENV{WALLET_CONFIG} = '@abs_top_srcdir@/tests/data/wallet.conf' }
 
-BEGIN { our $total = 1 }
+BEGIN { our $total = 47 }
 use Test::More tests => $total;
 
 use lib '@abs_top_srcdir@/perl';
@@ -52,10 +52,6 @@ sub wallet {
     return ($output, $error, $status);
 }
 
-# Use this to accumulate the history traces so that we can check history.
-my $history = '';
-my $date = strftime ('%Y-%m-%d %H:%M:%S', localtime $trace[2]);
-
 SKIP: {
     skip 'no keytab configuration', $total
         unless -f '@abs_top_builddir@/tests/data/test.keytab';
@@ -68,7 +64,7 @@ SKIP: {
     my $principal = contents ('@abs_top_builddir@/tests/data/test.principal');
     remctld_spawn ($remctld, $principal,
                    '@abs_top_builddir@/tests/data/test.keytab',
-                   '@abs_top_builddir@/tests/data/full.conf.in');
+                   '@abs_top_builddir@/tests/data/full.conf');
     $ENV{KRB5CCNAME} = 'krb5cc_test';
     getcreds ('@abs_top_builddir@/tests/data/test.keytab', $principal);
 
@@ -81,74 +77,80 @@ SKIP: {
     my $dbh = $admin->dbh;
 
     # Create the file bucket.
-    system ('rm', '-r', 'test-files');
+    if (-d 'test-files') {
+        system ('rm', '-r', 'test-files');
+    }
     mkdir ('test-files', 0777) or die "cannot create file-bucket: $!\n";
 
     # Now, start testing.  First, create an object, create an ACL, assign an
     # owner, store the contents, and then get the contents and make sure all
     # of that works as expected.
-    my ($out, $err, $status) = wallet ('create', 'file', 'test');
+    my ($out, $err, $status) = wallet ($principal, 'create', 'file', 'test');
     is ($status, 0, 'Object creation succeeds');
     is ($out, '', ' with no output');
     is ($err, '', ' and no error');
-    ($out, $err, $status) = wallet ('acl', 'create', 'user/test');
+    ($out, $err, $status) = wallet ($principal, 'acl', 'create', 'user/test');
     is ($status, 0, 'ACL creation succeeds');
     is ($out, '', ' with no output');
     is ($err, '', ' and no error');
-    ($out, $err, $status) = wallet ('acl', 'add', 'user/test', 'krb5',
-                                    $principal);
+    ($out, $err, $status) = wallet ($principal, 'acl', 'add', 'user/test',
+                                    'krb5', $principal);
     is ($status, 0, 'ACL population succeeds');
     is ($out, '', ' with no output');
     is ($err, '', ' and no error');
-    ($out, $err, $status) = wallet ('acl', 'show', 'user/test');
+    ($out, $err, $status) = wallet ($principal, 'acl', 'show', 'user/test');
     is ($status, 0, 'ACL show succeeds');
-    is ($out, "Members of ACL user/test (id: 2) are:\n  krb5 $principal",
+    is ($out, "Members of ACL user/test (id: 2) are:\n  krb5 $principal\n",
         ' with the right output');
     is ($err, '', ' and no error');
-    ($out, $err, $status) = wallet ('owner', 'file', 'test', 'user/test');
+    ($out, $err, $status) = wallet ($principal, 'owner', 'file', 'test',
+                                    'user/test');
     is ($status, 0, 'Object owner succeeds');
     is ($out, '', ' with no output');
     is ($err, '', ' and no error');
-    ($out, $err, $status) = wallet ('store', 'file', 'test', "foo\nbar\n");
+    ($out, $err, $status) = wallet ($principal, 'store', 'file', 'test',
+                                    "foo\nbar\n");
     is ($status, 0, 'Object store succeeds');
     is ($out, '', ' with no output');
     is ($err, '', ' and no error');
-    ($out, $err, $status) = wallet ('get', 'file', 'test');
+    ($out, $err, $status) = wallet ($principal, 'get', 'file', 'test');
     is ($status, 0, 'Object get succeeds');
     is ($out, "foo\nbar\n", ' and returns the right data');
     is ($err, '', ' and no error');
-    ($out, $err, $status) = wallet ('store', 'file', 'test', '');
+    ($out, $err, $status) = wallet ($principal, 'store', 'file', 'test', '');
     is ($status, 0, 'Store of empty object succeeds');
     is ($out, '', ' with no output');
     is ($err, '', ' and no error');
-    ($out, $err, $status) = wallet ('get', 'file', 'test');
+    ($out, $err, $status) = wallet ($principal, 'get', 'file', 'test');
     is ($status, 0, 'Get of empty object succeeds');
     is ($out, '', ' and returns an empty object');
     is ($err, '', ' with no error');
-    ($out, $err, $status) = wallet ('destroy', 'file', 'test');
+    ($out, $err, $status) = wallet ($principal, 'destroy', 'file', 'test');
     is ($status, 0, 'Object destroy succeeds');
     is ($out, '', ' with no output');
     is ($err, '', ' and no error');
-    ($out, $err, $status) = wallet ('get', 'file', 'test');
+    ($out, $err, $status) = wallet ($principal, 'get', 'file', 'test');
     isnt ($status, 0, 'Object get now fails');
     is ($out, '', ' with no output');
-    is ($err, "wallet: cannot find file:test\n", ' and the right error');
+    is ($err, "wallet: $principal not authorized to create file:test\n",
+        ' and the right error');
 
     # Try auto-creation.
-    ($out, $err, $status) = wallet ('get', 'file', 'auto');
+    ($out, $err, $status) = wallet ($principal, 'get', 'file', 'auto');
     isnt ($status, 0, 'Object autocreation get fails');
     is ($out, '', ' with no output');
     is ($err, "wallet: cannot get file:auto: object has not been stored\n",
         ' and the right error');
-    ($out, $err, $status) = wallet ('destroy', 'file', 'auto');
+    ($out, $err, $status) = wallet ($principal, 'destroy', 'file', 'auto');
     is ($status, 0, 'Object destroy succeeds');
     is ($out, '', ' with no output');
     is ($err, '', ' and no error');
-    ($out, $err, $status) = wallet ('store', 'file', 'auto', "baz\nboo\n");
+    ($out, $err, $status) = wallet ($principal, 'store', 'file', 'auto',
+                                    "baz\nboo\n");
     is ($status, 0, 'Object autocreation store succeeds');
     is ($out, '', ' with no output');
     is ($err, '', ' and no error');
-    ($out, $err, $status) = wallet ('get', 'file', 'auto');
+    ($out, $err, $status) = wallet ($principal, 'get', 'file', 'auto');
     is ($status, 0, 'Object get now succeeds');
     is ($out, "baz\nboo\n", ' with the right output');
     is ($err, '', ' and no error');
@@ -157,5 +159,7 @@ SKIP: {
     remctld_stop;
     $admin->destroy;
     unlink ('wallet-db', 'krb5cc_test', 'test-pid');
-    system ('rm', '-r', 'test-files');
+    if (-d 'test-files') {
+        system ('rm', '-r', 'test-files');
+    }
 }
diff --git a/tests/data/cmd-wrapper.in b/tests/data/cmd-wrapper.in
index d50816c..a45edc8 100644
--- a/tests/data/cmd-wrapper.in
+++ b/tests/data/cmd-wrapper.in
@@ -6,4 +6,5 @@
 
 WALLET_CONFIG='@abs_top_srcdir@/tests/data/wallet.conf'
 export WALLET_CONFIG
-exec perl -I'@abs_top_srcdir@/perl' '@abs_top_srcdir@/server/wallet-backend'
+exec perl -I'@abs_top_srcdir@/perl' '@abs_top_srcdir@/server/wallet-backend' \
+    "$@"
diff --git a/tests/data/wallet.conf b/tests/data/wallet.conf
index 7900d03..b864e5e 100644
--- a/tests/data/wallet.conf
+++ b/tests/data/wallet.conf
@@ -1,4 +1,4 @@
-# wallet.conf -- Test wallet server configuration.
+# wallet.conf -- Test wallet server configuration.  -*- perl -*-
 # $Id$
 
 # Always test with SQLite.
@@ -8,4 +8,15 @@ $DB_INFO = 'wallet-db';
 # Set up a file bucket.
 $FILE_BUCKET = 'test-files';
 
+# Simple auto-creation rules.
+sub default_owner {
+    my ($type, $name) = @_;
+    my $principal = $ENV{REMOTE_USER};
+    if ($type eq 'file' and $name eq 'auto') {
+        return ('auto', [ 'krb5', $principal ]);
+    } else {
+        return;
+    }
+}
+
 1;