Public secret_attributes_validate method

This makes the internal logic of _secret_attributes_validate public,
so applications can check and recover when an invalid attributes table
is passed to other libsecret API, such as secret_service_clear.

libsecret/secret-attributes.c

@@ -179,11 +179,23 @@ secret_attributes_buildv (const SecretSchema *schema,
 	return attributes;
 }
 
+/**
+ * secret_attributes_validate:
+ * @schema: the schema for the attributes
+ * @attributes: the attributes to be validated
+ * @error: place to report errors encountered
+ *
+ * Check if attributes are valid according to the provided schema.
+ *
+ * Verifies schema name if available, attribute names and parsing
+ * of attribute values.
+ *
+ * Returns: whether or not the given attributes table is valid
+ */
 gboolean
-_secret_attributes_validate (const SecretSchema *schema,
+secret_attributes_validate (const SecretSchema *schema,
-                             GHashTable *attributes,
+                            GHashTable *attributes,
-                             const char *pretty_function,
+			    GError **error)
-                             gboolean matching)
 {
 	const SecretSchemaAttribute *attribute;
 	GHashTableIter iter;
@@ -204,8 +216,10 @@ _secret_attributes_validate (const SecretSchema *schema,
 		   name. */
 		if (g_str_equal (key, "xdg:schema")) {
 			if (!g_str_equal (value, schema->name)) {
-				g_critical ("%s: xdg:schema value %s differs from schema %s:",
+				g_set_error_literal (error,
-					    pretty_function, value, schema->name);
+						     SECRET_ERROR,
+						     SECRET_ERROR_MISMATCHED_SCHEMA,
+						     "Schema attribute doesn't match schema name");
 				return FALSE;
 			}
 			continue;
@@ -227,16 +241,22 @@ _secret_attributes_validate (const SecretSchema *schema,
 		}
 
 		if (attribute == NULL) {
-			g_critical ("%s: invalid %s attribute for %s schema",
+			g_set_error (error,
-			            pretty_function, key, schema->name);
+				     SECRET_ERROR,
+				     SECRET_ERROR_NO_MATCHING_ATTRIBUTE,
+				     "Schema does not contain any attributes matching %s",
+				     key);
 			return FALSE;
 		}
 
 		switch (attribute->type) {
 		case SECRET_SCHEMA_ATTRIBUTE_BOOLEAN:
 			if (!g_str_equal (value, "true") && !g_str_equal (value, "false")) {
-				g_critical ("%s: invalid %s boolean value for %s schema: %s",
+				g_set_error (error,
-				            pretty_function, key, schema->name, value);
+					     SECRET_ERROR,
+					     SECRET_ERROR_WRONG_TYPE,
+					     "Attribute %s could not be parsed into a boolean",
+					     key);
 				return FALSE;
 			}
 			break;
@@ -244,35 +264,70 @@ _secret_attributes_validate (const SecretSchema *schema,
 			end = NULL;
 			g_ascii_strtoll (value, &end, 10);
 			if (!end || end[0] != '\0') {
-				g_warning ("%s: invalid %s integer value for %s schema: %s",
+				g_set_error (error,
-				           pretty_function, key, schema->name, value);
+					     SECRET_ERROR,
+					     SECRET_ERROR_WRONG_TYPE,
+					     "Attribute %s could not be parsed into an integer",
+					     key);
 				return FALSE;
 			}
 			break;
 		case SECRET_SCHEMA_ATTRIBUTE_STRING:
 			if (!g_utf8_validate (value, -1, NULL)) {
-				g_warning ("%s: invalid %s string value for %s schema: %s",
+				g_set_error (error,
-				           pretty_function, key, schema->name, value);
+					     SECRET_ERROR,
+					     SECRET_ERROR_WRONG_TYPE,
+					     "Attribute %s could not be parsed into a string",
+					     key);
 				return FALSE;
 			}
 			break;
 		default:
-			g_warning ("%s: invalid %s value type in %s schema",
+			g_set_error (error,
-			           pretty_function, key, schema->name);
+				     SECRET_ERROR,
+				     SECRET_ERROR_WRONG_TYPE,
+				     "%s: Invalid attribute type",
+				     key);
 			return FALSE;
 		}
 	}
 
 	/* Nothing to match on, resulting search would match everything :S */
-	if (matching && !any && schema->flags & SECRET_SCHEMA_DONT_MATCH_NAME) {
+	if (!any && schema->flags & SECRET_SCHEMA_DONT_MATCH_NAME) {
-		g_warning ("%s: must specify at least one attribute to match",
+		g_set_error_literal (error,
-		           pretty_function);
+				     SECRET_ERROR,
+				     SECRET_ERROR_EMPTY_TABLE,
+				     "Must have at least one attribute to check");
 		return FALSE;
 	}
 
 	return TRUE;
 }
 
+// Private function to be used internally
+gboolean
+_secret_attributes_validate (const SecretSchema *schema,
+                             GHashTable *attributes,
+                             const char *pretty_function,
+                             gboolean matching)
+{
+	GError *error = NULL;
+
+	if (!secret_attributes_validate (schema, attributes, &error)) {
+		// if matching is false, an empty table is fine
+		if ((!matching) && (error->code == SECRET_ERROR_EMPTY_TABLE)) {
+			g_error_free (error);
+			return TRUE;
+		}
+		
+		g_warning ("%s: error validating schema: %s", pretty_function, error->message);
+		g_error_free (error);
+		return FALSE;
+	}
+	return TRUE;
+}
+
+
 GHashTable *
 _secret_attributes_copy (GHashTable *attributes)
 {

libsecret/secret-attributes.h

@@ -32,6 +32,10 @@ GHashTable *         secret_attributes_build         (const SecretSchema *schema
 GHashTable *         secret_attributes_buildv        (const SecretSchema *schema,
                                                       va_list va);
 
+gboolean             secret_attributes_validate      (const SecretSchema *schema,
+						      GHashTable *attributes,
+						      GError **error);
+
 
 G_END_DECLS
 

libsecret/secret-types.h

@@ -33,6 +33,10 @@ typedef enum {
 	SECRET_ERROR_NO_SUCH_OBJECT = 3,
 	SECRET_ERROR_ALREADY_EXISTS = 4,
 	SECRET_ERROR_INVALID_FILE_FORMAT = 5,
+	SECRET_ERROR_MISMATCHED_SCHEMA = 6,
+	SECRET_ERROR_NO_MATCHING_ATTRIBUTE = 7,
+	SECRET_ERROR_WRONG_TYPE = 8,
+	SECRET_ERROR_EMPTY_TABLE = 9,
 } SecretError;
 
 #define SECRET_COLLECTION_DEFAULT "default"

libsecret/secret-util.c

@@ -29,6 +29,14 @@
  *   Service
  * @SECRET_ERROR_ALREADY_EXISTS: a relevant item or collection already exists
  * @SECRET_ERROR_INVALID_FILE_FORMAT: the file format is not valid
+ * @SECRET_ERROR_MISMATCHED_SCHEMA: the xdg:schema attribute of the table does
+ * not match the schema name
+ * @SECRET_ERROR_NO_MATCHING_ATTRIBUTE: attribute contained in table not found
+ * in corresponding schema
+ * @SECRET_ERROR_WRONG_TYPE: attribute could not be parsed according to its type
+ * reported in the table's schema
+ * @SECRET_ERROR_EMPTY_TABLE: attribute list passed to secret_attributes_validate
+ * has no elements to validate
  *
  * Errors returned by the Secret Service.
  *

libsecret/test-attributes.c

@@ -29,7 +29,7 @@
 
 static const SecretSchema MOCK_SCHEMA = {
 	"org.mock.Schema",
-	SECRET_SCHEMA_NONE,
+	SECRET_SCHEMA_DONT_MATCH_NAME,
 	{
 		{ "number", SECRET_SCHEMA_ATTRIBUTE_INTEGER },
 		{ "string", SECRET_SCHEMA_ATTRIBUTE_STRING },
@@ -152,7 +152,41 @@ test_validate_schema (void)
 }
 
 static void
-test_validate_schema_bad (void)
+test_validate_schema_empty_ok (void)
+{
+	GHashTable *attributes;
+	gboolean ret;
+
+	attributes = g_hash_table_new (g_str_hash, g_str_equal);
+	
+	ret = _secret_attributes_validate (&MOCK_SCHEMA, attributes, G_STRFUNC, FALSE);
+	g_assert_true (ret);
+
+	g_hash_table_unref (attributes);
+}
+
+static void
+test_validate_schema_bad_empty_not_ok (void)
+{
+	GHashTable *attributes;
+	gboolean ret;
+
+	if (g_test_subprocess ()) {
+		attributes = g_hash_table_new (g_str_hash, g_str_equal);
+		
+		ret = _secret_attributes_validate (&MOCK_SCHEMA, attributes, G_STRFUNC, TRUE);
+		g_assert_false (ret);
+
+		g_hash_table_unref (attributes);
+		return;
+	}
+
+	g_test_trap_subprocess ("/attributes/validate-schema-bad-empty-not-ok", 0, G_TEST_SUBPROCESS_INHERIT_STDOUT);
+	g_test_trap_assert_failed ();
+}
+
+static void
+test_validate_schema_bad_mismatched_schema (void)
 {
 	GHashTable *attributes;
 	gboolean ret;
@@ -170,7 +204,57 @@ test_validate_schema_bad (void)
 		return;
 	}
 
-	g_test_trap_subprocess ("/attributes/validate-schema-bad", 0, G_TEST_SUBPROCESS_INHERIT_STDOUT);
+	g_test_trap_subprocess ("/attributes/validate-schema-bad-mismatched-schema", 0, G_TEST_SUBPROCESS_INHERIT_STDOUT);
+	g_test_trap_assert_failed ();
+}
+
+static void
+test_validate_schema_bad_wrong_type (void)
+{
+	GHashTable *attributes;
+	gboolean ret;
+	char non_utf8_string[] = {(char) 128, '\0'};
+
+	if (g_test_subprocess ()) {
+		attributes = g_hash_table_new (g_str_hash, g_str_equal);
+		g_hash_table_replace (attributes, "number", "string_in_wrong_place");
+		g_hash_table_replace (attributes, "string", non_utf8_string);
+		g_hash_table_replace (attributes, "even", "neither_true_nor_false");
+		g_hash_table_replace (attributes, "xdg:schema", "org.mock.Schema");
+
+		ret = _secret_attributes_validate (&MOCK_SCHEMA, attributes, G_STRFUNC, TRUE);
+		g_assert_false (ret);
+
+		g_hash_table_unref (attributes);
+		return;
+	}
+
+	g_test_trap_subprocess ("/attributes/validate-schema-bad-wrong-type", 0, G_TEST_SUBPROCESS_INHERIT_STDOUT);
+	g_test_trap_assert_failed ();
+}
+
+static void
+test_validate_schema_bad_fake_key (void)
+{
+	GHashTable *attributes;
+	gboolean ret;
+
+	if (g_test_subprocess ()) {
+		attributes = g_hash_table_new (g_str_hash, g_str_equal);
+		g_hash_table_replace (attributes, "number", "1");
+		g_hash_table_replace (attributes, "string", "test");
+		g_hash_table_replace (attributes, "xdg:schema", "org.mock.Schema");
+		g_hash_table_replace (attributes, "made_up_key", "not_valid");
+
+		ret = _secret_attributes_validate (&MOCK_SCHEMA, attributes, G_STRFUNC, TRUE);
+		g_assert_false (ret);
+
+		g_hash_table_unref (attributes);
+		return;
+	}
+
+	g_test_trap_subprocess ("/attributes/validate-schema-bad-fake-key", 0, G_TEST_SUBPROCESS_INHERIT_STDOUT);
+	g_test_trap_assert_failed ();
 }
 
 static void
@@ -203,7 +287,11 @@ main (int argc, char **argv)
 	g_test_add_func ("/attributes/build-bad-type", test_build_bad_type);
 
 	g_test_add_func ("/attributes/validate-schema", test_validate_schema);
-	g_test_add_func ("/attributes/validate-schema-bad", test_validate_schema_bad);
+	g_test_add_func ("/attributes/validate-schema-empty-ok", test_validate_schema_empty_ok);
+	g_test_add_func ("/attributes/validate-schema-bad-empty-not-ok", test_validate_schema_bad_empty_not_ok);
+	g_test_add_func ("/attributes/validate-schema-bad-mismatched-schema", test_validate_schema_bad_mismatched_schema);
+	g_test_add_func ("/attributes/validate-schema-bad-wrong-type", test_validate_schema_bad_wrong_type);
+	g_test_add_func ("/attributes/validate-schema-bad-fake-key", test_validate_schema_bad_fake_key);
 	g_test_add_func ("/attributes/validate-libgnomekeyring", test_validate_libgnomekeyring);
 
 	return g_test_run ();

libsecret/test-vala-lang.vala

@@ -12,6 +12,20 @@
 Secret.Schema schema;
 Secret.Schema no_name_schema;
 
+private void test_attributes_validate () {
+  try {
+    var attributes = new GLib.HashTable<string,string> (GLib.str_hash, GLib.str_equal);
+    attributes["even"] = "false";
+    attributes["string"] = "one";
+    attributes["number"] = "1";
+    
+    bool valid = Secret.attributes_validate (schema, attributes);
+    GLib.assert (valid = true);
+  } catch ( GLib.Error e ) {
+    GLib.error (e.message);
+  }
+}
+
 private void test_lookup_sync () {
   try {
     string? password = Secret.password_lookup_sync (schema, null, "even", false, "string", "one", "number", 1);
@@ -166,6 +180,7 @@ private static int main (string[] args) {
                                         "string", Secret.SchemaAttributeType.STRING);
   }
 
+  GLib.Test.add_data_func ("/vala/attributes/validate", test_attributes_validate);
   GLib.Test.add_data_func ("/vala/lookup/sync", test_lookup_sync);
   GLib.Test.add_data_func ("/vala/lookup/async", test_lookup_async);
   GLib.Test.add_data_func ("/vala/lookup/no-name", test_lookup_no_name);