Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[20732] Fix some leaks in XML DynamicTypes Parser (backport #4717) #4736

Closed
wants to merge 1 commit into from
Closed

[20732] Fix some leaks in XML DynamicTypes Parser (backport #4717) #4736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions include/fastrtps/xmlparser/XMLProfileManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ class XMLProfileManager
* FastDDS's Domain calls this method automatically on its destructor, but
* if using XMLProfileManager outside of FastDDS, it should be called manually.
*/
<<<<<<< HEAD
RTPS_DllAPI static void DeleteInstance()
{
participant_profiles_.clear();
Expand All @@ -237,6 +238,9 @@ class XMLProfileManager
xml_files_.clear();
transport_profiles_.clear();
}
=======
RTPS_DllAPI static void DeleteInstance();
>>>>>>> 8cb447bb2 (Fix some leaks in XML DynamicTypes Parser (#4717))

/**
* Retrieves a DynamicPubSubType for the given dynamic type name.
Expand Down
86 changes: 75 additions & 11 deletions src/cpp/rtps/xmlparser/XMLDynamicParser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,11 +341,23 @@ XMLP_ret XMLParser::parseXMLAliasDynamicType(
if (valueBuilder != nullptr)
{
const char* name = p_root->Attribute(NAME);
if (name != nullptr)
if (name != nullptr && name[0] != '\0')
{
p_dynamictypebuilder_t typeBuilder =
types::DynamicTypeBuilderFactory::get_instance()->create_alias_builder(valueBuilder, name);
XMLProfileManager::insertDynamicTypeByName(name, typeBuilder);
if (nullptr == XMLProfileManager::getDynamicTypeByName(name))
{
p_dynamictypebuilder_t typeBuilder =
types::DynamicTypeBuilderFactory::get_instance()->create_alias_builder(valueBuilder, name);
if (false == XMLProfileManager::insertDynamicTypeByName(name, typeBuilder))
{
types::DynamicTypeBuilderFactory::get_instance()->delete_builder(typeBuilder);
ret = XMLP_ret::XML_ERROR;
}
}
else
{
EPROSIMA_LOG_ERROR(XMLPARSER, "Error parsing alias type: Type '" << name << "' already defined.");
ret = XMLP_ret::XML_ERROR;
}
}
else
{
Expand Down Expand Up @@ -394,11 +406,16 @@ XMLP_ret XMLParser::parseXMLBitsetDynamicType(
uint32_t mId = 0;

const char* name = p_root->Attribute(NAME);
if (nullptr == name)
if (nullptr == name || name[0] == '\0')
{
logError(XMLPARSER, "Error parsing 'bitsetDcl' type. No name attribute given.");
return XMLP_ret::XML_ERROR;
}
if (nullptr != XMLProfileManager::getDynamicTypeByName(name))
{
EPROSIMA_LOG_ERROR(XMLPARSER, "Error parsing 'bitsetDcl' type: Type '" << name << "' already defined.");
return XMLP_ret::XML_ERROR;
}

const char* baseType = p_root->Attribute(BASE_TYPE);
if (baseType != nullptr)
Expand Down Expand Up @@ -441,7 +458,11 @@ XMLP_ret XMLParser::parseXMLBitsetDynamicType(
}
}

XMLProfileManager::insertDynamicTypeByName(name, typeBuilder);
if (false == XMLProfileManager::insertDynamicTypeByName(name, typeBuilder))
{
types::DynamicTypeBuilderFactory::get_instance()->delete_builder(typeBuilder);
ret = XMLP_ret::XML_ERROR;
}
return ret;
}

Expand Down Expand Up @@ -628,6 +649,12 @@ XMLP_ret XMLParser::parseXMLBitmaskDynamicType(
{
return XMLP_ret::XML_ERROR;
}
if (nullptr != XMLProfileManager::getDynamicTypeByName(name))
{
EPROSIMA_LOG_ERROR(XMLPARSER, "Error parsing 'bitmaskDcl' type: Type '" << name << "' already defined.");
return XMLP_ret::XML_ERROR;
}

p_dynamictypebuilder_t typeBuilder =
types::DynamicTypeBuilderFactory::get_instance()->create_bitmask_builder(bit_bound);
typeBuilder->set_name(name);
Expand All @@ -652,7 +679,11 @@ XMLP_ret XMLParser::parseXMLBitmaskDynamicType(
}
}

XMLProfileManager::insertDynamicTypeByName(name, typeBuilder);
if (false == XMLProfileManager::insertDynamicTypeByName(name, typeBuilder))
{
types::DynamicTypeBuilderFactory::get_instance()->delete_builder(typeBuilder);
ret = XMLP_ret::XML_ERROR;
}
return ret;
}

Expand All @@ -677,11 +708,16 @@ XMLP_ret XMLParser::parseXMLEnumDynamicType(
XMLP_ret ret = XMLP_ret::XML_OK;
const char* enumName = p_root->Attribute(NAME);

if (enumName == nullptr)
if (enumName == nullptr || enumName[0] == '\0')
{
logError(XMLPARSER, "Error parsing 'enum' type. No name attribute given.");
return XMLP_ret::XML_ERROR;
}
if (nullptr != XMLProfileManager::getDynamicTypeByName(enumName))
{
EPROSIMA_LOG_ERROR(XMLPARSER, "Error parsing 'enum' type: Type '" << enumName << "' already defined.");
return XMLP_ret::XML_ERROR;
}

p_dynamictypebuilder_t typeBuilder = types::DynamicTypeBuilderFactory::get_instance()->create_enum_builder();
uint32_t currValue = 0;
Expand All @@ -703,7 +739,11 @@ XMLP_ret XMLParser::parseXMLEnumDynamicType(
typeBuilder->add_empty_member(currValue++, name);
}

XMLProfileManager::insertDynamicTypeByName(enumName, typeBuilder);
if (false == XMLProfileManager::insertDynamicTypeByName(enumName, typeBuilder))
{
types::DynamicTypeBuilderFactory::get_instance()->delete_builder(typeBuilder);
ret = XMLP_ret::XML_ERROR;
}
return ret;
}

Expand All @@ -728,6 +768,11 @@ XMLP_ret XMLParser::parseXMLStructDynamicType(
logError(XMLPARSER, "Missing required attribute 'name' in 'structDcl'.");
return XMLP_ret::XML_ERROR;
}
if (nullptr != XMLProfileManager::getDynamicTypeByName(name))
{
EPROSIMA_LOG_ERROR(XMLPARSER, "Error parsing 'structDcl' type: Type '" << name << "' already defined.");
return XMLP_ret::XML_ERROR;
}

p_dynamictypebuilder_t typeBuilder; // = types::DynamicTypeBuilderFactory::get_instance()->create_struct_builder();
//typeBuilder->set_name(name);
Expand Down Expand Up @@ -773,7 +818,11 @@ XMLP_ret XMLParser::parseXMLStructDynamicType(
}
}

XMLProfileManager::insertDynamicTypeByName(name, typeBuilder);
if (false == XMLProfileManager::insertDynamicTypeByName(name, typeBuilder))
{
types::DynamicTypeBuilderFactory::get_instance()->delete_builder(typeBuilder);
ret = XMLP_ret::XML_ERROR;
}

return ret;
}
Expand Down Expand Up @@ -804,6 +853,17 @@ XMLP_ret XMLParser::parseXMLUnionDynamicType(

XMLP_ret ret = XMLP_ret::XML_OK;
const char* name = p_root->Attribute(NAME);
if (nullptr == name || name[0] == '\0')
{
EPROSIMA_LOG_ERROR(XMLPARSER, "Missing required attribute 'name' in 'unionDcl'.");
return XMLP_ret::XML_ERROR;
}
if (nullptr != XMLProfileManager::getDynamicTypeByName(name))
{
EPROSIMA_LOG_ERROR(XMLPARSER, "Error parsing 'unionDcl' type: Type '" << name << "' already defined.");
return XMLP_ret::XML_ERROR;
}

tinyxml2::XMLElement* p_element = p_root->FirstChildElement(DISCRIMINATOR);
if (p_element != nullptr)
{
Expand Down Expand Up @@ -867,7 +927,11 @@ XMLP_ret XMLParser::parseXMLUnionDynamicType(
}
}

XMLProfileManager::insertDynamicTypeByName(name, typeBuilder);
if (false == XMLProfileManager::insertDynamicTypeByName(name, typeBuilder))
{
types::DynamicTypeBuilderFactory::get_instance()->delete_builder(typeBuilder);
ret = XMLP_ret::XML_ERROR;
}
}
}
else
Expand Down
24 changes: 24 additions & 0 deletions src/cpp/rtps/xmlparser/XMLProfileManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -692,3 +692,27 @@ XMLP_ret XMLProfileManager::extractReplierProfile(

return XMLP_ret::XML_OK;
}

void XMLProfileManager::DeleteInstance()
{
participant_factory_profiles_.clear();
participant_profiles_.clear();
publisher_profiles_.clear();
subscriber_profiles_.clear();
requester_profiles_.clear();
replier_profiles_.clear();
topic_profiles_.clear();
xml_files_.clear();
transport_profiles_.clear();

// Delete the registered dynamic types builders
{
namespace dyn_types = eprosima::fastrtps::types;
auto factory = dyn_types::DynamicTypeBuilderFactory::get_instance();
for (auto&& type : dynamic_types_)
{
factory->delete_builder(type.second);
}
dynamic_types_.clear();
}
}
8 changes: 8 additions & 0 deletions test/unittest/xmlparser/XMLParserTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,14 @@ TEST_F(XMLParserTests, regressions)
EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/19851_profile_bin.xml", root));
EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/simple_participant_profiles_nok.xml", root));
EXPECT_EQ(XMLP_ret::XML_OK, XMLParser::loadXML("regressions/simple_participant_profiles_ok.xml", root));
<<<<<<< HEAD
=======
EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/20186_profile_bin.xml", root));
EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/20187_profile_bin.xml", root));
EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/20608_profile_bin.xml", root));
EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/20610_profile_bin.xml", root));
EXPECT_EQ(XMLP_ret::XML_ERROR, XMLParser::loadXML("regressions/20732_profile_bin.xml", root));
>>>>>>> 8cb447bb2 (Fix some leaks in XML DynamicTypes Parser (#4717))
}

TEST_F(XMLParserTests, NoFile)
Expand Down
13 changes: 13 additions & 0 deletions test/unittest/xmlparser/regressions/20732_profile_bin.xml
View file
Open in desktop

Large diffs are not rendered by default.