Better critical preconditions for invalid attributes

 * When passing NULL for a string vararg attribute print
   a critical warning.
 * Make all attribute warnings critical
 * Add tests for the various critical warnings

https://bugzilla.gnome.org/show_bug.cgi?id=686015

diff --git a/libsecret/secret-attributes.c b/libsecret/secret-attributes.c
index f394408..00372e7 100644 (file)
--- a/libsecret/secret-attributes.c
+++ b/libsecret/secret-attributes.c
@@ -153,7 +153,7 @@ secret_attributes_buildv (const SecretSchema *schema,
                }
 
                if (!type_found) {
-                       g_warning ("The attribute '%s' was not found in the password schema.", attribute_name);
+                       g_critical ("The attribute '%s' was not found in the password schema.", attribute_name);
                        g_hash_table_unref (attributes);
                        return NULL;
                }
@@ -165,8 +165,12 @@ secret_attributes_buildv (const SecretSchema *schema,
                        break;
                case SECRET_SCHEMA_ATTRIBUTE_STRING:
                        string = va_arg (va, gchar *);
+                       if (string == NULL) {
+                               g_critical ("The value for attribute '%s' was NULL", attribute_name);
+                               return NULL;
+                       }
                        if (!g_utf8_validate (string, -1, NULL)) {
-                               g_warning ("The value for attribute '%s' was not a valid utf-8 string.", attribute_name);
+                               g_critical ("The value for attribute '%s' was not a valid UTF-8 string.", attribute_name);
                                g_hash_table_unref (attributes);
                                return NULL;
                        }
@@ -177,7 +181,7 @@ secret_attributes_buildv (const SecretSchema *schema,
                        value = g_strdup_printf ("%d", integer);
                        break;
                default:
-                       g_warning ("The password attribute '%s' has an invalid type in the password schema.", attribute_name);
+                       g_critical ("The password attribute '%s' has an invalid type in the password schema.", attribute_name);
                        g_hash_table_unref (attributes);
                        return NULL;
                }
@@ -220,7 +224,7 @@ _secret_attributes_validate (const SecretSchema *schema,
                }
 
                if (attribute == NULL) {
-                       g_critical ("%s: invalid %s attribute in for %s schema",
+                       g_critical ("%s: invalid %s attribute for %s schema",
                                    pretty_function, key, schema->name);
                        return FALSE;
                }

diff --git a/libsecret/secret-password.c b/libsecret/secret-password.c
index 0969e42..d0df50f 100644 (file)
--- a/libsecret/secret-password.c
+++ b/libsecret/secret-password.c
@@ -94,6 +94,10 @@ secret_password_store (const SecretSchema *schema,
        attributes = secret_attributes_buildv (schema, va);
        va_end (va);
 
+       /* Precondition failed, already warned */
+       if (!attributes)
+               return;
+
        secret_password_storev (schema, attributes, collection, label, password,
                                cancellable, callback, user_data);
 
@@ -225,6 +229,10 @@ secret_password_store_sync (const SecretSchema *schema,
        attributes = secret_attributes_buildv (schema, va);
        va_end (va);
 
+       /* Precondition failed, already warned */
+       if (!attributes)
+               return FALSE;
+
        ret = secret_password_storev_sync (schema, attributes, collection,
                                           label, password, cancellable, error);
 
@@ -335,6 +343,10 @@ secret_password_lookup (const SecretSchema *schema,
        attributes = secret_attributes_buildv (schema, va);
        va_end (va);
 
+       /* Precondition failed, already warned */
+       if (!attributes)
+               return;
+
        secret_password_lookupv (schema, attributes, cancellable,
                                 callback, user_data);
 
@@ -468,6 +480,10 @@ secret_password_lookup_sync (const SecretSchema *schema,
        attributes = secret_attributes_buildv (schema, va);
        va_end (va);
 
+       /* Precondition failed, already warned */
+       if (!attributes)
+               return NULL;
+
        password = secret_password_lookupv_sync (schema, attributes,
                                                 cancellable, error);
 
@@ -516,6 +532,10 @@ secret_password_lookup_nonpageable_sync (const SecretSchema *schema,
        attributes = secret_attributes_buildv (schema, va);
        va_end (va);
 
+       /* Precondition failed, already warned */
+       if (!attributes)
+               return NULL;
+
        password = secret_password_lookupv_nonpageable_sync (schema, attributes,
                                                             cancellable, error);
 
@@ -668,6 +688,10 @@ secret_password_clear (const SecretSchema *schema,
        attributes = secret_attributes_buildv (schema, va);
        va_end (va);
 
+       /* Precondition failed, already warned */
+       if (!attributes)
+               return;
+
        secret_password_clearv (schema, attributes, cancellable,
                                callback, user_data);
 
@@ -769,6 +793,10 @@ secret_password_clear_sync (const SecretSchema* schema,
        attributes = secret_attributes_buildv (schema, va);
        va_end (va);
 
+       /* Precondition failed, already warned */
+       if (!attributes)
+               return FALSE;
+
        result = secret_password_clearv_sync (schema, attributes,
                                              cancellable, error);
 

diff --git a/libsecret/tests/Makefile.am b/libsecret/tests/Makefile.am
index ad380ca..8293edb 100644 (file)
--- a/libsecret/tests/Makefile.am
+++ b/libsecret/tests/Makefile.am
@@ -33,6 +33,7 @@ LDADD =  \
        $(NULL)
 
 C_TESTS = \
+       test-attributes \
        test-value \
        test-prompt \
        test-service \

diff --git a/libsecret/tests/test-attributes.c b/libsecret/tests/test-attributes.c
new file mode 100644 (file)
index 0000000..eaf18f2
--- /dev/null
+++ b/libsecret/tests/test-attributes.c
@@ -0,0 +1,140 @@
+/* libsecret - GLib wrapper for Secret Service
+ *
+ * Copyright 2012 Red Hat Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; either version 2 of the licence or (at
+ * your option) any later version.
+ *
+ * See the included COPYING file for more information.
+ *
+ * Author: Stef Walter <stefw@gnome.org>
+ */
+
+
+#include "config.h"
+
+#include "secret-attributes.h"
+
+#include "egg/egg-testing.h"
+
+#include <glib.h>
+
+#include <errno.h>
+#include <stdlib.h>
+
+static const SecretSchema MOCK_SCHEMA = {
+       "org.mock.Schema",
+       SECRET_SCHEMA_NONE,
+       {
+               { "number", SECRET_SCHEMA_ATTRIBUTE_INTEGER },
+               { "string", SECRET_SCHEMA_ATTRIBUTE_STRING },
+               { "even", SECRET_SCHEMA_ATTRIBUTE_BOOLEAN },
+               { "bad-type", -1 },
+       }
+};
+
+static void
+test_build (void)
+{
+       GHashTable *attributes;
+
+       attributes = secret_attributes_build (&MOCK_SCHEMA,
+                                             "number", 4,
+                                             "string", "four",
+                                             "even", TRUE,
+                                             NULL);
+
+       g_assert_cmpstr (g_hash_table_lookup (attributes, "number"), ==, "4");
+       g_assert_cmpstr (g_hash_table_lookup (attributes, "string"), ==, "four");
+       g_assert_cmpstr (g_hash_table_lookup (attributes, "even"), ==, "true");
+
+       g_hash_table_unref (attributes);
+}
+
+static void
+test_build_unknown (void)
+{
+       GHashTable *attributes;
+
+       if (g_test_trap_fork (0, G_TEST_TRAP_SILENCE_STDERR)) {
+               attributes = secret_attributes_build (&MOCK_SCHEMA,
+                                                     "invalid", "whee",
+                                                     "string", "four",
+                                                     "even", TRUE,
+                                                     NULL);
+               g_assert (attributes == NULL);
+       }
+
+       g_test_trap_assert_failed ();
+       g_test_trap_assert_stderr ("*was not found in*");
+}
+
+static void
+test_build_null_string (void)
+{
+       GHashTable *attributes;
+
+       if (g_test_trap_fork (0, G_TEST_TRAP_SILENCE_STDERR)) {
+               attributes = secret_attributes_build (&MOCK_SCHEMA,
+                                                     "number", 4,
+                                                     "string", NULL,
+                                                     "even", TRUE,
+                                                     NULL);
+               g_assert (attributes == NULL);
+       }
+
+       g_test_trap_assert_failed ();
+       g_test_trap_assert_stderr ("*attribute*NULL*");
+}
+
+static void
+test_build_non_utf8_string (void)
+{
+       GHashTable *attributes;
+
+       if (g_test_trap_fork (0, G_TEST_TRAP_SILENCE_STDERR)) {
+               attributes = secret_attributes_build (&MOCK_SCHEMA,
+                                                     "number", 4,
+                                                     "string", "\xfftest",
+                                                     "even", TRUE,
+                                                     NULL);
+               g_assert (attributes == NULL);
+       }
+
+       g_test_trap_assert_failed ();
+       g_test_trap_assert_stderr ("*attribute*UTF-8*");
+}
+
+static void
+test_build_bad_type (void)
+{
+       GHashTable *attributes;
+
+       if (g_test_trap_fork (0, G_TEST_TRAP_SILENCE_STDERR)) {
+               attributes = secret_attributes_build (&MOCK_SCHEMA,
+                                                     "bad-type", "test",
+                                                     NULL);
+               g_assert (attributes == NULL);
+       }
+
+       g_test_trap_assert_failed ();
+       g_test_trap_assert_stderr ("*invalid type*");
+}
+
+int
+main (int argc, char **argv)
+{
+       g_test_init (&argc, &argv, NULL);
+       g_set_prgname ("test-attributes");
+       g_type_init ();
+
+       g_test_add_func ("/attributes/build", test_build);
+       g_test_add_func ("/attributes/build-unknown", test_build_unknown);
+       g_test_add_func ("/attributes/build-null-string", test_build_null_string);
+       g_test_add_func ("/attributes/build-non-utf8-string", test_build_non_utf8_string);
+       g_test_add_func ("/attributes/build-bad-type", test_build_bad_type);
+
+       return g_test_run ();
+}