From 7e02a594a77c2730efcc13dd141cc78e201cd7e3 Mon Sep 17 00:00:00 2001 From: Stef Walter Date: Fri, 12 Oct 2012 11:27:34 +0200 Subject: [PATCH] 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 --- libsecret/secret-attributes.c | 12 ++-- libsecret/secret-password.c | 28 ++++++++ libsecret/tests/Makefile.am | 1 + libsecret/tests/test-attributes.c | 140 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 177 insertions(+), 4 deletions(-) create mode 100644 libsecret/tests/test-attributes.c diff --git a/libsecret/secret-attributes.c b/libsecret/secret-attributes.c index f394408..00372e7 100644 --- 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 --- 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 --- 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 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 + */ + + +#include "config.h" + +#include "secret-attributes.h" + +#include "egg/egg-testing.h" + +#include + +#include +#include + +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 (); +} -- 2.7.4