From 26d6b35a498843f66c66d1d1ed1a28189ef15dd2 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Mon, 21 Nov 2011 21:40:10 +0100 Subject: [PATCH] [leakcheck] fix leaks seen by the unit tests This commit also improved valgrind suppression file to hide static allocations done by the GLib type system. --- lasso/id-wsf-2.0/saml2_login.c | 2 +- lasso/saml-2.0/login.c | 3 ++- lasso/saml-2.0/provider.c | 2 +- lasso/saml-2.0/server.c | 4 +-- lasso/xml/xml.c | 1 + tests/basic_tests.c | 12 ++++++--- tests/non_regression_tests.c | 49 +++++++++++++++++++++++++--------- tests/valgrind/lasso.supp | 39 +++++++++++++++++++++++++++ 8 files changed, 91 insertions(+), 21 deletions(-) diff --git a/lasso/id-wsf-2.0/saml2_login.c b/lasso/id-wsf-2.0/saml2_login.c index fc0f074b..6f86ff8e 100644 --- a/lasso/id-wsf-2.0/saml2_login.c +++ b/lasso/id-wsf-2.0/saml2_login.c @@ -91,7 +91,7 @@ lasso_server_create_assertion_as_idwsf2_security_token(LassoServer *server, lasso_release_gobject(assertion); goto cleanup; } - lasso_assign_gobject(assertion->Subject->EncryptedID, encrypted_id); + lasso_assign_new_gobject(assertion->Subject->EncryptedID, encrypted_id); } else { lasso_assign_new_gobject(assertion->Subject->NameID, name_id); } diff --git a/lasso/saml-2.0/login.c b/lasso/saml-2.0/login.c index 42bdbba6..864750cb 100644 --- a/lasso/saml-2.0/login.c +++ b/lasso/saml-2.0/login.c @@ -1436,6 +1436,7 @@ lasso_saml20_login_build_authn_response_msg(LassoLogin *login) lasso_check_good_rc(lasso_saml20_profile_build_response_msg(profile, NULL, http_method, url)); cleanup: + lasso_release_string(url); return rc; } @@ -1493,7 +1494,7 @@ lasso_saml20_login_init_idp_initiated_authn_request(LassoLogin *login, return LASSO_SERVER_ERROR_PROVIDER_NOT_FOUND; lasso_assign_string(profile->remote_providerID, remote_providerID); - lasso_assign_gobject(profile->request, lasso_samlp2_authn_request_new()); + lasso_assign_new_gobject(profile->request, lasso_samlp2_authn_request_new()); lasso_assign_new_gobject(LASSO_SAMLP2_AUTHN_REQUEST(profile->request)->NameIDPolicy, lasso_samlp2_name_id_policy_new()); lasso_assign_new_gobject(LASSO_SAMLP2_REQUEST_ABSTRACT(profile->request)->Issuer, diff --git a/lasso/saml-2.0/provider.c b/lasso/saml-2.0/provider.c index 747ca2e5..66293c3f 100644 --- a/lasso/saml-2.0/provider.c +++ b/lasso/saml-2.0/provider.c @@ -287,7 +287,6 @@ load_endpoint_type(xmlNode *xmlnode, LassoProvider *provider, LassoProviderRole } else { name = g_strdup_printf("%s %s", xmlnode->name, binding_s); } - lasso_release_xml_string(binding); /* Response endpoint ? */ response_value = getSaml2MdProp(xmlnode, LASSO_SAML2_METADATA_ATTRIBUTE_RESPONSE_LOCATION); @@ -301,6 +300,7 @@ load_endpoint_type(xmlNode *xmlnode, LassoProvider *provider, LassoProviderRole _lasso_provider_add_metadata_value_for_role(provider, role, name, (char*)value); cleanup: + lasso_release_xml_string(binding); lasso_release_xml_string(value); lasso_release_xml_string(response_value); lasso_release_string(name); diff --git a/lasso/saml-2.0/server.c b/lasso/saml-2.0/server.c index f2dc8879..cac2d89b 100644 --- a/lasso/saml-2.0/server.c +++ b/lasso/saml-2.0/server.c @@ -139,7 +139,7 @@ lasso_saml20_server_load_metadata_entity(LassoServer *server, LassoProviderRole provider = lasso_provider_new_from_xmlnode(role, entity); if (provider) { - char *name = g_strdup(provider->ProviderID); + char *name = provider->ProviderID; if (g_list_find_custom(blacklisted_entity_ids, name, (GCompareFunc) g_strcmp0)) { @@ -153,7 +153,7 @@ lasso_saml20_server_load_metadata_entity(LassoServer *server, LassoProviderRole l->next->data = g_strdup(name); *loaded_end = l->next; } - g_hash_table_insert(server->providers, name, provider); + g_hash_table_insert(server->providers, g_strdup(name), provider); return 0; } else { return LASSO_SERVER_ERROR_NO_PROVIDER_LOADED; diff --git a/lasso/xml/xml.c b/lasso/xml/xml.c index 13480f33..dd51f22b 100644 --- a/lasso/xml/xml.c +++ b/lasso/xml/xml.c @@ -890,6 +890,7 @@ _lasso_node_free_custom_element(struct _CustomElement *custom_element) lasso_release_string(custom_element->private_key); lasso_release_string(custom_element->private_key_password); lasso_release_string(custom_element->certificate); + lasso_release_sec_key(custom_element->encryption_public_key); } lasso_release(custom_element); } diff --git a/tests/basic_tests.c b/tests/basic_tests.c index e0158055..69991d4b 100644 --- a/tests/basic_tests.c +++ b/tests/basic_tests.c @@ -1840,6 +1840,13 @@ START_TEST(test10_test_alldumps) lasso_release_string(node_dump); lasso_release_gobject(node2); lasso_release_gobject(node); + /* test serialization / deserialization of KeyInfoConfirmationDataType */ + node = LASSO_NODE(lasso_saml2_key_info_confirmation_data_type_new()); + node_dump = lasso_node_dump(node); + fail_unless((node2 = lasso_node_new_from_dump(node_dump)) != NULL, "restoring dump failed after lasso_saml2_key_info_confirmation_data_type_new"); + lasso_release_string(node_dump); + lasso_release_gobject(node2); + lasso_release_gobject(node); #endif /* test deserialization of saml2:EncryptedAssertion" */ const char *encrypted_element_xml[] = { @@ -1873,10 +1880,6 @@ START_TEST(test10_test_alldumps) lasso_release_doc(xmldoc); ++iter; } - /* test serialization / deserialization of KeyInfoConfirmationDataType */ - node = LASSO_NODE(lasso_saml2_key_info_confirmation_data_type_new()); - printf("%s\n", lasso_node_debug(node, 10)); - lasso_release_gobject(node); } END_TEST @@ -1976,6 +1979,7 @@ START_TEST(test13_test_lasso_server_load_metadata) check_equals(g_list_length(loaded_entity_ids), 283); check_equals(g_hash_table_size(server->providers), 393); #endif + lasso_release_list_of_strings(loaded_entity_ids); lasso_release_gobject(server); } diff --git a/tests/non_regression_tests.c b/tests/non_regression_tests.c index b266e9dd..03c11a35 100644 --- a/tests/non_regression_tests.c +++ b/tests/non_regression_tests.c @@ -88,6 +88,7 @@ END_TEST START_TEST(indexed_endpoints_20101008) { LassoProvider *provider = NULL; + char *str; char *meta01 = "\n\ \n\ \n\ @@ -115,27 +116,51 @@ START_TEST(indexed_endpoints_20101008) provider = lasso_provider_new_from_buffer(LASSO_PROVIDER_ROLE_SP, meta01, NULL, NULL); check_not_null(provider); - check_str_equals(lasso_provider_get_assertion_consumer_service_url(provider, NULL), "ok"); - check_str_equals(lasso_provider_get_assertion_consumer_service_url(provider, "0"), "ok"); - check_str_equals(lasso_provider_get_assertion_consumer_service_url(provider, "1"), "wrong"); + str = lasso_provider_get_assertion_consumer_service_url(provider, NULL); + check_str_equals(str, "ok"); + g_free(str); + str = lasso_provider_get_assertion_consumer_service_url(provider, "0"); + check_str_equals(str, "ok"); + g_free(str); + str = lasso_provider_get_assertion_consumer_service_url(provider, "1"); + check_str_equals(str, "wrong"); + g_free(str); lasso_release_gobject(provider); provider = lasso_provider_new_from_buffer(LASSO_PROVIDER_ROLE_SP, meta02, NULL, NULL); check_not_null(provider); - check_str_equals(lasso_provider_get_assertion_consumer_service_url(provider, NULL), "ok"); - check_str_equals(lasso_provider_get_assertion_consumer_service_url(provider, "0"), "wrong"); - check_str_equals(lasso_provider_get_assertion_consumer_service_url(provider, "1"), "ok"); + str = lasso_provider_get_assertion_consumer_service_url(provider, NULL); + check_str_equals(str, "ok"); + g_free(str); + str = lasso_provider_get_assertion_consumer_service_url(provider, "0"); + check_str_equals(str, "wrong"); + g_free(str); + str = lasso_provider_get_assertion_consumer_service_url(provider, "1"); + check_str_equals(str, "ok"); + g_free(str); lasso_release_gobject(provider); provider = lasso_provider_new_from_buffer(LASSO_PROVIDER_ROLE_SP, meta03, NULL, NULL); check_not_null(provider); - check_str_equals(lasso_provider_get_assertion_consumer_service_url(provider, NULL), "ok"); - check_str_equals(lasso_provider_get_assertion_consumer_service_url(provider, "0"), "wrong"); - check_str_equals(lasso_provider_get_assertion_consumer_service_url(provider, "1"), "ok"); + str = lasso_provider_get_assertion_consumer_service_url(provider, NULL); + check_str_equals(str, "ok"); + g_free(str); + str = lasso_provider_get_assertion_consumer_service_url(provider, "0"); + check_str_equals(str, "wrong"); + g_free(str); + str = lasso_provider_get_assertion_consumer_service_url(provider, "1"); + check_str_equals(str, "ok"); + g_free(str); lasso_release_gobject(provider); provider = lasso_provider_new_from_buffer(LASSO_PROVIDER_ROLE_SP, meta04, NULL, NULL); check_not_null(provider); - check_str_equals(lasso_provider_get_assertion_consumer_service_url(provider, NULL), "ok"); - check_str_equals(lasso_provider_get_assertion_consumer_service_url(provider, "0"), "wrong"); - check_str_equals(lasso_provider_get_assertion_consumer_service_url(provider, "1"), "ok"); + str = lasso_provider_get_assertion_consumer_service_url(provider, NULL); + check_str_equals(str, "ok"); + g_free(str); + str = lasso_provider_get_assertion_consumer_service_url(provider, "0"); + check_str_equals(str, "wrong"); + g_free(str); + str = lasso_provider_get_assertion_consumer_service_url(provider, "1"); + check_str_equals(str, "ok"); + g_free(str); lasso_release_gobject(provider); } END_TEST diff --git a/tests/valgrind/lasso.supp b/tests/valgrind/lasso.supp index b4d22161..4e9a80e2 100644 --- a/tests/valgrind/lasso.supp +++ b/tests/valgrind/lasso.supp @@ -165,3 +165,42 @@ fun:g_hash_table_new fun:g_quark_from_static_string } +{ + g_type_init + Memcheck:Leak + fun:malloc + ... + fun:g_type_init +} +{ + g_type_init + Memcheck:Leak + fun:calloc + ... + fun:g_type_init +} +{ + g_type_init + Memcheck:Leak + fun:realloc + ... + fun:g_type_init +} +{ + register type + Memcheck:Leak + fun:malloc + ... + fun:g_type_register_static + ... + fun:lasso_*get_type +} +{ + register type + Memcheck:Leak + fun:realloc + ... + fun:g_type_register_static + ... + fun:lasso_*get_type +}