GNOME.org
 

[gnome-keyring] gcr: Fixes and debugging for GcrGnupgCollection.

commit 74a4ec824f826cb81e6bf6f8204d779f4341d0e2
Author: Stef Walter <stefw collabora co uk>
Date:   Sun May 15 10:45:02 2011 +0200

    gcr: Fixes and debugging for GcrGnupgCollection.
    
     * Comments and review changes.
     * Add tests for collection loading and reloading.
     * Debug logging
     * Remove racy pid tracking. Will solve this in another branch.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=648019

 gcr/gcr-debug.c                           |    1 +
 gcr/gcr-debug.h                           |    1 +
 gcr/gcr-gnupg-collection.c                |   61 +++++++------
 gcr/tests/Makefile.am                     |    1 +
 gcr/tests/files/gnupg-homedir/pubring.gpg |  Bin 19689 -> 21969 bytes
 gcr/tests/files/gnupg-homedir/secring.gpg |  Bin 0 -> 4398 bytes
 gcr/tests/files/gnupg-homedir/trustdb.gpg |  Bin 1200 -> 1400 bytes
 gcr/tests/test-gnupg-collection.c         |  136 ++++++++++++++++++++++++++---
 testing/gnupg-example/random_seed         |  Bin 600 -> 0 bytes
 9 files changed, 161 insertions(+), 39 deletions(-)
---
diff --git a/gcr/gcr-debug.c b/gcr/gcr-debug.c
index 6df2341..2ecbe4b 100644
--- a/gcr/gcr-debug.c
+++ b/gcr/gcr-debug.c
@@ -38,6 +38,7 @@ static GcrDebugFlags current_flags = 0;
 static GDebugKey keys[] = {
 	{ "certificate-chain", GCR_DEBUG_CERTIFICATE_CHAIN },
 	{ "parse", GCR_DEBUG_PARSE },
+	{ "gnupg", GCR_DEBUG_GNUPG },
 	{ 0, }
 };
 
diff --git a/gcr/gcr-debug.h b/gcr/gcr-debug.h
index 200c939..51ac345 100644
--- a/gcr/gcr-debug.h
+++ b/gcr/gcr-debug.h
@@ -31,6 +31,7 @@ typedef enum {
 	GCR_DEBUG_LIBRARY = 1 << 1,
 	GCR_DEBUG_CERTIFICATE_CHAIN = 1 << 2,
 	GCR_DEBUG_PARSE = 1 << 3,
+	GCR_DEBUG_GNUPG = 1 << 4
 } GcrDebugFlags;
 
 gboolean           _gcr_debug_flag_is_set              (GcrDebugFlags flag);
diff --git a/gcr/gcr-gnupg-collection.c b/gcr/gcr-gnupg-collection.c
index 454238b..5a68470 100644
--- a/gcr/gcr-gnupg-collection.c
+++ b/gcr/gcr-gnupg-collection.c
@@ -25,6 +25,8 @@
 
 #include "gcr-colons.h"
 #include "gcr-collection.h"
+#define DEBUG_FLAG GCR_DEBUG_GNUPG
+#include "gcr-debug.h"
 #include "gcr-gnupg-collection.h"
 #include "gcr-gnupg-key.h"
 #include "gcr-internal.h"
@@ -76,6 +78,10 @@ _gcr_gnupg_collection_set_property (GObject *obj, guint prop_id, const GValue *v
 	case PROP_DIRECTORY:
 		g_return_if_fail (!self->pv->directory);
 		self->pv->directory = g_value_dup_string (value);
+		if (self->pv->directory && !g_path_is_absolute (self->pv->directory)) {
+			g_warning ("gnupg collection directory path should be absolute: %s",
+			           self->pv->directory);
+		}
 		break;
 	default:
 		G_OBJECT_WARN_INVALID_PROPERTY_ID (obj, prop_id, pspec);
@@ -210,8 +216,6 @@ typedef struct {
 	GcrLoadingPhase loading_phase;        /* Whether loading public or private */
 	GPtrArray *dataset;                   /* GcrColons* not yet made into a key */
 	guint spawn_sig;
-	guint child_sig;
-	GPid gnupg_pid;
 	GString *out_data;                    /* Pending output not yet parsed into colons */
 	GString *err_data;                    /* Pending errors not yet printed */
 	GHashTable *difference;               /* Hashset gchar *keyid -> gchar *keyid */
@@ -234,12 +238,6 @@ _gcr_gnupg_collection_load_free (gpointer data)
 
 	if (load->spawn_sig)
 		g_source_remove (load->spawn_sig);
-	if (load->child_sig)
-		g_source_remove (load->child_sig);
-	if (load->gnupg_pid) {
-		kill (load->gnupg_pid, SIGTERM);
-		g_spawn_close_pid (load->gnupg_pid);
-	}
 	g_slice_free (GcrGnupgCollectionLoad, load);
 }
 
@@ -256,11 +254,13 @@ process_dataset_as_public_key (GcrGnupgCollectionLoad *load, GPtrArray *dataset,
 
 	/* Already have this key, just update */
 	if (key) {
+		_gcr_debug ("updating public key: %s", keyid);
 		_gcr_gnupg_key_set_public_dataset (key, dataset);
 
 	/* Add a new key */
 	} else {
 		key = _gcr_gnupg_key_new (dataset, NULL);
+		_gcr_debug ("creating public key: %s", keyid);
 		g_hash_table_insert (load->collection->pv->items, g_strdup (keyid), key);
 		gcr_collection_emit_added (GCR_COLLECTION (load->collection), G_OBJECT (key));
 	}
@@ -275,12 +275,14 @@ process_dataset_as_secret_key (GcrGnupgCollectionLoad *load, GPtrArray *dataset,
 	key = g_hash_table_lookup (load->collection->pv->items, keyid);
 
 	/* Don't have this key */
-	if (key == NULL)
+	if (key == NULL) {
 		g_message ("Secret key seen but no public key for: %s", keyid);
 
 	/* Tell the private key that it's a secret one */
-	else
+	} else {
+		_gcr_debug ("adding secret dataset to key: %s", keyid);
 		_gcr_gnupg_key_set_secret_dataset (key, dataset);
+	}
 }
 
 static void
@@ -324,6 +326,8 @@ on_line_parse_output (const gchar *line, gpointer user_data)
 	GcrColons *colons;
 	GQuark schema;
 
+	_gcr_debug ("output: %s", line);
+
 	colons = _gcr_colons_parse (line, -1);
 	if (!colons) {
 		g_warning ("invalid gnupg output line: %s", line);
@@ -337,6 +341,7 @@ on_line_parse_output (const gchar *line, gpointer user_data)
 	 * it's a new key being listed.
 	 */
 	if (schema == GCR_COLONS_SCHEMA_PUB || schema == GCR_COLONS_SCHEMA_SEC) {
+		_gcr_debug ("start of new key");
 		if (load->dataset->len)
 			process_dataset_as_key (load);
 		g_assert (!load->dataset->len);
@@ -430,10 +435,12 @@ on_spawn_completed (gpointer user_data)
 	/* If we completed loading public keys, then go and load secret */
 	switch (load->loading_phase) {
 	case GCR_LOADING_PHASE_PUBLIC:
+		_gcr_debug ("public load phase completed");
 		load->loading_phase = GCR_LOADING_PHASE_SECRET;
 		spawn_gnupg_list_process (load, res);
 		return;
 	case GCR_LOADING_PHASE_SECRET:
+		_gcr_debug ("secret load phase completed");
 		/* continue below */
 		break;
 	default:
@@ -446,6 +453,7 @@ on_spawn_completed (gpointer user_data)
 		object = g_hash_table_lookup (load->collection->pv->items, keyid);
 		if (object != NULL) {
 			g_object_ref (object);
+			_gcr_debug ("removing key no longer present in keyring: %s", (gchar*)keyid);
 			g_hash_table_remove (load->collection->pv->items, keyid);
 			gcr_collection_emit_removed (GCR_COLLECTION (load->collection), object);
 			g_object_unref (object);
@@ -456,17 +464,12 @@ on_spawn_completed (gpointer user_data)
 }
 
 static void
-on_child_exited (GPid pid, gint status, gpointer user_data)
+on_child_exited (GPid pid, gint status, gpointer unused)
 {
-	GSimpleAsyncResult *res = G_SIMPLE_ASYNC_RESULT (user_data);
-	GcrGnupgCollectionLoad *load = g_simple_async_result_get_op_res_gpointer (res);
 	gint code;
 
-	g_return_if_fail (pid == load->gnupg_pid);
-
-	g_spawn_close_pid (load->gnupg_pid);
-	load->gnupg_pid = 0;
-	load->child_sig = 0;
+	_gcr_debug ("process exited: %d", (gint)pid);
+	g_spawn_close_pid (pid);
 
 	if (WIFEXITED (status)) {
 		code = WEXITSTATUS (status);
@@ -493,15 +496,18 @@ spawn_gnupg_list_process (GcrGnupgCollectionLoad *load, GSimpleAsyncResult *res)
 {
 	GError *error = NULL;
 	GPtrArray *argv;
+	GPid pid;
 
 	argv = g_ptr_array_new ();
 	g_ptr_array_add (argv, (gpointer)GPG_EXECUTABLE);
 
 	switch (load->loading_phase) {
 	case GCR_LOADING_PHASE_PUBLIC:
+		_gcr_debug ("starting public load phase");
 		g_ptr_array_add (argv, (gpointer)"--list-keys");
 		break;
 	case GCR_LOADING_PHASE_SECRET:
+		_gcr_debug ("starting secret load phase");
 		g_ptr_array_add (argv, (gpointer)"--list-secret-keys");
 		break;
 	default:
@@ -517,27 +523,28 @@ spawn_gnupg_list_process (GcrGnupgCollectionLoad *load, GSimpleAsyncResult *res)
 	g_ptr_array_add (argv, NULL);
 
 	g_assert (!load->spawn_sig);
-	g_assert (!load->gnupg_pid);
+
+	if (_gcr_debugging) {
+		gchar *command = g_strjoinv (" ", (gchar**)argv->pdata);
+		_gcr_debug ("spawning gnupg process: %s", command);
+		g_free (command);
+	}
 
 	load->spawn_sig = egg_spawn_async_with_callbacks (load->collection->pv->directory,
 	                                                  (gchar**)argv->pdata, NULL,
 	                                                  G_SPAWN_DO_NOT_REAP_CHILD,
-	                                                  &load->gnupg_pid,
-	                                                  &spawn_callbacks,
+	                                                  &pid, &spawn_callbacks,
 	                                                  g_object_ref (res),
 	                                                  NULL, &error);
 
 	if (error) {
+		_gcr_debug ("spawning process failed: %s", error->message);
 		g_simple_async_result_set_from_error (res, error);
 		g_simple_async_result_complete_in_idle (res);
 		g_clear_error (&error);
 	} else {
-		g_assert (!load->child_sig);
-		load->child_sig = g_child_watch_add_full (G_PRIORITY_DEFAULT,
-		                                          load->gnupg_pid,
-		                                          on_child_exited,
-		                                          g_object_ref (res),
-		                                          g_object_unref);
+		_gcr_debug ("process spawned: %d", (gint)pid);
+		g_child_watch_add (pid, on_child_exited, NULL);
 	}
 
 	g_ptr_array_unref (argv);
diff --git a/gcr/tests/Makefile.am b/gcr/tests/Makefile.am
index e01cd15..bf3237e 100644
--- a/gcr/tests/Makefile.am
+++ b/gcr/tests/Makefile.am
@@ -34,6 +34,7 @@ TEST_PROGS = \
 check_PROGRAMS = $(TEST_PROGS)
 
 test: $(TEST_PROGS)
+	chmod -f 700 files/gnupg-homedir
 	gtester -k --verbose -m $(TEST_MODE) --g-fatal-warnings $(TEST_PROGS)
 
 check-local: test
diff --git a/gcr/tests/files/gnupg-homedir/pubring.gpg b/gcr/tests/files/gnupg-homedir/pubring.gpg
index a642536..10b1372 100644
Binary files a/gcr/tests/files/gnupg-homedir/pubring.gpg and b/gcr/tests/files/gnupg-homedir/pubring.gpg differ
diff --git a/gcr/tests/files/gnupg-homedir/secring.gpg b/gcr/tests/files/gnupg-homedir/secring.gpg
index e69de29..4a21e26 100644
Binary files a/gcr/tests/files/gnupg-homedir/secring.gpg and b/gcr/tests/files/gnupg-homedir/secring.gpg differ
diff --git a/gcr/tests/files/gnupg-homedir/trustdb.gpg b/gcr/tests/files/gnupg-homedir/trustdb.gpg
index 0377c97..3824098 100644
Binary files a/gcr/tests/files/gnupg-homedir/trustdb.gpg and b/gcr/tests/files/gnupg-homedir/trustdb.gpg differ
diff --git a/gcr/tests/test-gnupg-collection.c b/gcr/tests/test-gnupg-collection.c
index d156a82..0761463 100644
--- a/gcr/tests/test-gnupg-collection.c
+++ b/gcr/tests/test-gnupg-collection.c
@@ -24,6 +24,7 @@
 
 #include "gcr/gcr.h"
 #include "gcr/gcr-gnupg-collection.h"
+#include "gcr/gcr-gnupg-key.h"
 
 #include "egg/egg-testing.h"
 
@@ -32,34 +33,146 @@
 #include <errno.h>
 #include <string.h>
 
-#if 0
 typedef struct {
-
+	GcrGnupgCollection *collection;
+	GHashTable *keys;
+	GAsyncResult *result;
 } Test;
 
 static void
+on_collection_added (GcrCollection *collection, GObject *object, gpointer user_data)
+{
+	Test *test = user_data;
+	GcrGnupgKey *key;
+	const gchar *keyid;
+
+	g_assert (GCR_COLLECTION (test->collection) == collection);
+
+	g_assert (GCR_IS_GNUPG_KEY (object));
+	key = GCR_GNUPG_KEY (object);
+
+	keyid = _gcr_gnupg_key_get_keyid (key);
+	g_assert (keyid);
+	g_assert (!g_hash_table_lookup (test->keys, keyid));
+
+	g_hash_table_insert (test->keys, g_strdup (keyid), key);
+}
+
+static void
+on_collection_removed (GcrCollection *collection, GObject *object, gpointer user_data)
+{
+	Test *test = user_data;
+	GcrGnupgKey *key;
+	const gchar *keyid;
+
+	g_assert (GCR_COLLECTION (test->collection) == collection);
+	g_assert (GCR_IS_GNUPG_KEY (object));
+
+	keyid = _gcr_gnupg_key_get_keyid (GCR_GNUPG_KEY (object));
+	key = g_hash_table_lookup (test->keys, keyid);
+	g_assert (key == GCR_GNUPG_KEY (object));
+
+	if (!g_hash_table_remove (test->keys, keyid))
+		g_assert_not_reached ();
+}
+
+static void
 setup (Test *test, gconstpointer unused)
 {
+	GcrCollection *collection;
+	gchar *directory;
+	gchar *path;
+
+	directory = g_get_current_dir ();
+	path = g_build_filename (directory, "files", "gnupg-homedir", NULL);
+
+	collection = _gcr_gnupg_collection_new (path);
+	test->collection = GCR_GNUPG_COLLECTION (collection);
+
+	test->keys = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
+	g_signal_connect (collection, "added", G_CALLBACK (on_collection_added), test);
+	g_signal_connect (collection, "removed", G_CALLBACK (on_collection_removed), test);
 
+	g_free (path);
+	g_free (directory);
 }
 
 static void
 teardown (Test *test, gconstpointer unused)
 {
+	g_hash_table_destroy (test->keys);
 
+	if (test->result)
+		g_object_unref (test->result);
+
+	g_object_unref (test->collection);
+	g_assert (!GCR_IS_GNUPG_COLLECTION (test->collection));
 }
-#endif
 
 static void
-test_create (void)
+on_async_ready (GObject *source, GAsyncResult *res, gpointer user_data)
 {
-	GcrCollection *collection;
+	Test *test = user_data;
+	g_assert (G_OBJECT (test->collection) == source);
+	g_assert (test->result == NULL);
+	test->result = g_object_ref (res);
+}
 
-	collection = _gcr_gnupg_collection_new ("files/gnupg-homedir/");
+static void
+test_load (Test *test, gconstpointer unused)
+{
+	GError *error = NULL;
+	GcrGnupgKey *key;
+
+	_gcr_gnupg_collection_load_async (test->collection, NULL, on_async_ready, test);
+	egg_test_wait_until (500);
+
+	g_assert (test->result);
+	_gcr_gnupg_collection_load_finish (test->collection, test->result, &error);
+	g_assert_no_error (error);
+
+	/* Werner Koch (a public key) */
+	key = g_hash_table_lookup (test->keys, "5DE249965B0358A2");
+	g_assert (GCR_IS_GNUPG_KEY (key));
+	g_assert (_gcr_gnupg_key_get_secret_dataset (key) == NULL);
 
-	g_object_unref (collection);
+	/* Test Number 2 (a secret key)*/
+	key = g_hash_table_lookup (test->keys, "268FEE686262C395");
+	g_assert (GCR_IS_GNUPG_KEY (key));
+	g_assert (_gcr_gnupg_key_get_secret_dataset (key));
 }
 
+static void
+test_reload (Test *test, gconstpointer unused)
+{
+	GError *error = NULL;
+	GcrGnupgKey *key;
+
+	_gcr_gnupg_collection_load_async (test->collection, NULL, on_async_ready, test);
+	egg_test_wait_until (500);
+	g_assert (test->result);
+	_gcr_gnupg_collection_load_finish (test->collection, test->result, &error);
+	g_assert_no_error (error);
+
+	g_object_unref (test->result);
+	test->result = NULL;
+
+	_gcr_gnupg_collection_load_async (test->collection, NULL, on_async_ready, test);
+	egg_test_wait_until (500);
+	g_assert (test->result);
+	_gcr_gnupg_collection_load_finish (test->collection, test->result, &error);
+	g_assert_no_error (error);
+
+	/* Werner Koch (a public key) */
+	key = g_hash_table_lookup (test->keys, "5DE249965B0358A2");
+	g_assert (GCR_IS_GNUPG_KEY (key));
+	g_assert (_gcr_gnupg_key_get_secret_dataset (key) == NULL);
+
+	/* Test Number 2 (a secret key)*/
+	key = g_hash_table_lookup (test->keys, "268FEE686262C395");
+	g_assert (GCR_IS_GNUPG_KEY (key));
+	g_assert (_gcr_gnupg_key_get_secret_dataset (key));
+}
 
 int
 main (int argc, char **argv)
@@ -68,15 +181,14 @@ main (int argc, char **argv)
 
 	g_type_init ();
 	g_test_init (&argc, &argv, NULL);
+	g_set_prgname ("test-gnupg-collection");
 
 	srcdir = g_getenv ("SRCDIR");
 	if (srcdir && chdir (srcdir) < 0)
 		g_error ("couldn't change directory to: %s: %s", srcdir, g_strerror (errno));
 
-	g_test_add_func ("/gcr/gnupg-collection/create", test_create);
-#if 0
-	g_test_add ("/gcr/certificate/issuer_dn", Test, NULL, setup, test_issuer_dn, teardown);
-#endif
+	g_test_add ("/gcr/gnupg-collection/load", Test, NULL, setup, test_load, teardown);
+	g_test_add ("/gcr/gnupg-collection/reload", Test, NULL, setup, test_reload, teardown);
 
-	return g_test_run ();
+	return egg_tests_run_in_thread_with_loop ();
 }
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]