diff --git a/src/xml/xml_util.cc b/src/xml/xml_util.cc index be15ee95..b706a48a 100644 --- a/src/xml/xml_util.cc +++ b/src/xml/xml_util.cc @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -549,8 +550,9 @@ string mjXUtil::FindValue(const mjMap* map, int mapsz, int value) { // "len" is the number of floats or doubles to be read // the content is returned in "text", the numeric data in "data" // return true if attribute found, false if not found and not required -int mjXUtil::ReadAttr(XMLElement* elem, const char* attr, const int len, - double* data, string& text, bool required, bool exact) { +template +int mjXUtil::ReadAttrCpp(XMLElement* elem, const char* attr, const int len, + T* data, string& text, bool required, bool exact) { const char* pstr = elem->Attribute(attr); // check if attribute exists @@ -567,34 +569,27 @@ int mjXUtil::ReadAttr(XMLElement* elem, const char* attr, const int len, // get input stream istringstream strm(text); + std::string token; // read numbers - int i; - for (i=0; i> data[i]; - if (strm.eof()) { - i++; - break; - } else if (strm.bad()) { + int i = 0; + while (!strm.eof() && i < len) { + strm >> token; + istringstream token_strm(token); + token_strm >> data[i++]; + if (token_strm.fail() || !token_strm.eof()) { throw mjXError(elem, "problem reading attribute '%s'", attr); } } + strm >> std::ws; - // determine available length - int available = i; - if (strm.good()) { - double dummy; - strm >> dummy; - if (!strm.bad() && !strm.fail()) { - available++; - } - } - - // check - if (exact && availablelen) { + + // check if there is too much data + if (!strm.eof()) { throw mjXError(elem, "attribute '%s' has too much data", attr); } @@ -603,58 +598,17 @@ int mjXUtil::ReadAttr(XMLElement* elem, const char* attr, const int len, +int mjXUtil::ReadAttr(XMLElement* elem, const char* attr, const int len, + double* data, string& text, bool required, bool exact) { + return ReadAttrCpp(elem, attr, len, data, text, required, exact); +} + + + // float version int mjXUtil::ReadAttr(XMLElement* elem, const char* attr, const int len, float* data, string& text, bool required, bool exact) { - const char* pstr = elem->Attribute(attr); - - // check if attribute exists - if (!pstr) { - if (required) { - throw mjXError(elem, "required attribute missing: '%s'", attr); - } else { - return 0; - } - } - - // convert to string, remove trailing white space - text = string(pstr); - text.erase(text.find_last_not_of(" \t\n\r\f\v") + 1); - - // get input stream - istringstream strm(text); - - // read numbers - int i; - for (i=0; i> data[i]; - if (strm.eof()) { - i++; - break; - } else if (strm.bad()) { - throw mjXError(elem, "problem reading attribute '%s'", attr); - } - } - - // determine available length - int available = i; - if (strm.good()) { - float dummy; - strm >> dummy; - if (!strm.bad() && !strm.fail()) { - available++; - } - } - - // check - if (exact && availablelen) { - throw mjXError(elem, "attribute '%s' has too much data", attr); - } - - return i; + return ReadAttrCpp(elem, attr, len, data, text, required, exact); } @@ -662,55 +616,7 @@ int mjXUtil::ReadAttr(XMLElement* elem, const char* attr, const int len, // int version int mjXUtil::ReadAttr(XMLElement* elem, const char* attr, const int len, int* data, string& text, bool required, bool exact) { - const char* pstr = elem->Attribute(attr); - - // check if attribute exists - if (!pstr) { - if (required) { - throw mjXError(elem, "required attribute missing: '%s'", attr); - } else { - return 0; - } - } - - // convert to string, remove trailing white space - text = string(pstr); - text.erase(text.find_last_not_of(" \t\n\r\f\v") + 1); - - // get input stream - istringstream strm(text); - - // read numbers - int i; - for (i=0; i> data[i]; - if (strm.eof()) { - i++; - break; - } else if (strm.bad()) { - throw mjXError(elem, "problem reading attribute '%s'", attr); - } - } - - // determine available length - int available = i; - if (strm.good()) { - mjtByte dummy; - strm >> dummy; - if (!strm.bad() && !strm.fail()) { - available++; - } - } - - // check - if (exact && availablelen) { - throw mjXError(elem, "attribute '%s' has too much data", attr); - } - - return i; + return ReadAttrCpp(elem, attr, len, data, text, required, exact); } @@ -718,56 +624,7 @@ int mjXUtil::ReadAttr(XMLElement* elem, const char* attr, const int len, // byte version int mjXUtil::ReadAttr(XMLElement* elem, const char* attr, const int len, mjtByte* data, string& text, bool required, bool exact) { - const char* pstr = elem->Attribute(attr); - - // check if attribute exists - if (!pstr) { - if (required) { - throw mjXError(elem, "required attribute missing: '%s'", attr); - } else { - return 0; - } - } - - // convert to string, remove trailing white space - text = string(pstr); - text.erase(text.find_last_not_of(" \t\n\r\f\v") + 1); - - // get input stream - istringstream strm(text); - - // read numbers - int i, tmp; - for (i=0; i> tmp; - data[i] = (mjtByte)(tmp & 0xFF); - if (strm.eof()) { - i++; - break; - } else if (strm.bad()) { - throw mjXError(elem, "problem reading attribute '%s'", attr); - } - } - - // determine available length - int available = i; - if (strm.good()) { - mjtByte dummy; - strm >> dummy; - if (!strm.bad() && !strm.fail()) { - available++; - } - } - - // check - if (exact && availablelen) { - throw mjXError(elem, "attribute '%s' has too much data", attr); - } - - return i; + return ReadAttrCpp(elem, attr, len, data, text, required, exact); } diff --git a/src/xml/xml_util.h b/src/xml/xml_util.h index 0022bc7b..aac25447 100644 --- a/src/xml/xml_util.h +++ b/src/xml/xml_util.h @@ -98,6 +98,12 @@ class mjXUtil { // find value in map, return key ("": not found) static std::string FindValue(const mjMap* map, int mapsz, int value); + // read any type from attribute, return number read + template + static int ReadAttrCpp(tinyxml2::XMLElement* elem, const char* attr, const int len, + T* data, std::string& text, + bool required = false, bool exact = true); + // read DOUBLE array from attribute, return number read static int ReadAttr(tinyxml2::XMLElement* elem, const char* attr, const int len, double* data, std::string& text, diff --git a/test/engine/testdata/core_smooth/rne_post/connect/multiple_constraints.xml b/test/engine/testdata/core_smooth/rne_post/connect/multiple_constraints.xml index 4ac22633..34ef0551 100644 --- a/test/engine/testdata/core_smooth/rne_post/connect/multiple_constraints.xml +++ b/test/engine/testdata/core_smooth/rne_post/connect/multiple_constraints.xml @@ -22,7 +22,7 @@ - + diff --git a/test/xml/xml_native_reader_test.cc b/test/xml/xml_native_reader_test.cc index cbb7d457..7af9394d 100644 --- a/test/xml/xml_native_reader_test.cc +++ b/test/xml/xml_native_reader_test.cc @@ -128,6 +128,70 @@ TEST_F(UserDataTest, InvalidNUserSensor) { EXPECT_THAT(error.data(), HasSubstr("nuser_sensor")); } +TEST_F(UserDataTest, InvalidArrayElement) { + static constexpr char xml[] = R"( + + + + + + + + )"; + std::array error; + mjModel* model = LoadModelFromString(xml, error.data(), error.size()); + ASSERT_THAT(model, IsNull()); + EXPECT_THAT(error.data(), HasSubstr("problem reading attribute 'axisangle'")); +} + +TEST_F(UserDataTest, InvalidArrayLength) { + static constexpr char xml[] = R"( + + + + + + + + )"; + std::array error; + mjModel* model = LoadModelFromString(xml, error.data(), error.size()); + ASSERT_THAT(model, IsNull()); + EXPECT_THAT(error.data(), HasSubstr("has too much data")); +} + +TEST_F(UserDataTest, InvalidNumber) { + static constexpr char xml[] = R"( + + + + + + + + )"; + std::array error; + mjModel* model = LoadModelFromString(xml, error.data(), error.size()); + ASSERT_THAT(model, IsNull()); + EXPECT_THAT(error.data(), HasSubstr("problem reading attribute")); +} + +TEST_F(UserDataTest, AllowsSpaces) { + static constexpr char xml[] = R"( + + + + + + + + )"; + std::array error; + mjModel* model = LoadModelFromString(xml, error.data(), error.size()); + ASSERT_THAT(model, Not(IsNull())); + mj_deleteModel(model); +} + // ------------- test relative frame sensor parsing ---------------------------- using RelativeFrameSensorParsingTest = MujocoTest; @@ -207,7 +271,28 @@ TEST_F(ActuatorTest, IncompleteActlimited) { std::array error; mjModel* model = LoadModelFromString(xml, error.data(), error.size()); ASSERT_THAT(model, IsNull()); - EXPECT_THAT(error.data(), HasSubstr("actrange")); + EXPECT_THAT(error.data(), HasSubstr("attribute 'actrange' does not have enough data")); +} + +TEST_F(ActuatorTest, ReadsByte) { + static constexpr char xml[] = R"( + + + + + + + + + + + + )"; + std::array error; + mjModel* model = LoadModelFromString(xml, error.data(), error.size()); + ASSERT_THAT(model, Not(IsNull())); + EXPECT_EQ(*(model->actuator_actlimited), (mjtByte)(1 & 0xFF)); + mj_deleteModel(model); } } // namespace