From 13c679652834db45e2df4137f088ecd06351f017 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Sat, 3 Jul 2021 00:49:13 +0000 Subject: [PATCH] audio: Fix handling of relative XML include paths in VTS Pass "no fixup base dirs" flag to the XInclude processor to avoid modifications of the top-level elements from included XML files as a result of "fixup." Added tests to ensure that all relevant XInclude scenarios work. Bug: 192619060 Test: atest -host android.hardware.audio.common.test.utility_tests Change-Id: Id595c9fd30be378d76387ee55a8937e0bf28d1cd --- .../all-versions/test/utility/Android.bp | 31 ++- .../all-versions/test/utility/TEST_MAPPING | 7 + .../test/utility/src/ValidateXml.cpp | 3 +- .../test/utility/tests/utility_tests.cpp | 176 ++++++++++++++++++ 4 files changed, 214 insertions(+), 3 deletions(-) create mode 100644 audio/common/all-versions/test/utility/TEST_MAPPING create mode 100644 audio/common/all-versions/test/utility/tests/utility_tests.cpp diff --git a/audio/common/all-versions/test/utility/Android.bp b/audio/common/all-versions/test/utility/Android.bp index 1602d25b2f..757f8a853d 100644 --- a/audio/common/all-versions/test/utility/Android.bp +++ b/audio/common/all-versions/test/utility/Android.bp @@ -25,7 +25,7 @@ package { cc_library_static { name: "android.hardware.audio.common.test.utility", - defaults : ["hidl_defaults"], + defaults: ["hidl_defaults"], srcs: ["src/ValidateXml.cpp"], cflags: [ "-O0", @@ -34,7 +34,34 @@ cc_library_static { ], local_include_dirs: ["include/utility"], export_include_dirs: ["include"], - shared_libs: ["libxml2", "liblog"], + shared_libs: [ + "libxml2", + "liblog", + ], static_libs: ["libgtest"], export_static_lib_headers: ["libgtest"], } + +// Note: this isn't a VTS test, but rather a unit test +// to verify correctness of test utilities. +cc_test { + name: "android.hardware.audio.common.test.utility_tests", + host_supported: true, + local_include_dirs: ["include/utility"], + srcs: [ + "src/ValidateXml.cpp", + "tests/utility_tests.cpp", + ], + cflags: [ + "-Werror", + "-Wall", + "-g", + ], + shared_libs: [ + "libbase", + "libxml2", + "liblog", + ], + static_libs: ["libgtest"], + test_suites: ["general-tests"], +} diff --git a/audio/common/all-versions/test/utility/TEST_MAPPING b/audio/common/all-versions/test/utility/TEST_MAPPING new file mode 100644 index 0000000000..0bc187157a --- /dev/null +++ b/audio/common/all-versions/test/utility/TEST_MAPPING @@ -0,0 +1,7 @@ +{ + "presubmit": [ + { + "name": "android.hardware.audio.common.test.utility_tests" + } + ] +} diff --git a/audio/common/all-versions/test/utility/src/ValidateXml.cpp b/audio/common/all-versions/test/utility/src/ValidateXml.cpp index a866104b38..f111c011d9 100644 --- a/audio/common/all-versions/test/utility/src/ValidateXml.cpp +++ b/audio/common/all-versions/test/utility/src/ValidateXml.cpp @@ -112,7 +112,8 @@ struct Libxml2Global { return ::testing::AssertionFailure() << "Failed to parse xml\n" << context(); } - if (xmlXIncludeProcess(doc.get()) == -1) { + // Process 'include' directives w/o modifying elements loaded from included files. + if (xmlXIncludeProcessFlags(doc.get(), XML_PARSE_NOBASEFIX) == -1) { return ::testing::AssertionFailure() << "Failed to resolve xincludes in xml\n" << context(); } diff --git a/audio/common/all-versions/test/utility/tests/utility_tests.cpp b/audio/common/all-versions/test/utility/tests/utility_tests.cpp new file mode 100644 index 0000000000..c52306638f --- /dev/null +++ b/audio/common/all-versions/test/utility/tests/utility_tests.cpp @@ -0,0 +1,176 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include + +#include + +using ::android::hardware::audio::common::test::utility::validateXml; + +const char* XSD_SOURCE = + "" + "" + " " + " " + " " + " " + " " + " " + " " + " " + " " + " " + " " + " " + " " + " " + " " + " " + " " + ""; + +const char* INVALID_XML_SOURCE = + "" + ""; + +const char* VALID_XML_SOURCE = + "" + "" + " " + " " + " %s" + " " + ""; + +const char* MODULE_SOURCE = ""; + +const char* XI_INCLUDE = ""; + +const char* XML_INCLUDED_SOURCE = "%s"; + +namespace { + +std::string substitute(const char* fmt, const char* param) { + std::string buffer(static_cast(strlen(fmt) + strlen(param)), '\0'); + snprintf(buffer.data(), buffer.size(), fmt, param); + return buffer; +} + +std::string substitute(const char* fmt, const std::string& s) { + return substitute(fmt, s.c_str()); +} + +} // namespace + +TEST(ValidateXml, InvalidXml) { + TemporaryFile xml; + ASSERT_TRUE(android::base::WriteStringToFile(INVALID_XML_SOURCE, xml.path)) << strerror(errno); + TemporaryFile xsd; + ASSERT_TRUE(android::base::WriteStringToFile(XSD_SOURCE, xsd.path)) << strerror(errno); + EXPECT_FALSE(validateXml("xml", "xsd", xml.path, xsd.path)); +} + +TEST(ValidateXml, ValidXml) { + TemporaryFile xml; + ASSERT_TRUE( + android::base::WriteStringToFile(substitute(VALID_XML_SOURCE, MODULE_SOURCE), xml.path)) + << strerror(errno); + TemporaryFile xsd; + ASSERT_TRUE(android::base::WriteStringToFile(XSD_SOURCE, xsd.path)) << strerror(errno); + EXPECT_TRUE(validateXml("xml", "xsd", xml.path, xsd.path)); +} + +TEST(ValidateXml, IncludeAbsolutePath) { + TemporaryFile xmlInclude; + ASSERT_TRUE(android::base::WriteStringToFile(substitute(XML_INCLUDED_SOURCE, MODULE_SOURCE), + xmlInclude.path)) + << strerror(errno); + TemporaryFile xml; + ASSERT_TRUE(android::base::WriteStringToFile( + substitute(VALID_XML_SOURCE, substitute(XI_INCLUDE, xmlInclude.path)), xml.path)) + << strerror(errno); + TemporaryFile xsd; + ASSERT_TRUE(android::base::WriteStringToFile(XSD_SOURCE, xsd.path)) << strerror(errno); + EXPECT_TRUE(validateXml("xml", "xsd", xml.path, xsd.path)); +} + +TEST(ValidateXml, IncludeSameDirRelativePath) { + TemporaryFile xmlInclude; + ASSERT_TRUE(android::base::WriteStringToFile(substitute(XML_INCLUDED_SOURCE, MODULE_SOURCE), + xmlInclude.path)) + << strerror(errno); + TemporaryFile xml; + ASSERT_EQ(android::base::Dirname(xml.path), android::base::Dirname(xmlInclude.path)); + ASSERT_TRUE(android::base::WriteStringToFile( + substitute(VALID_XML_SOURCE, + substitute(XI_INCLUDE, android::base::Basename(xmlInclude.path))), + xml.path)) + << strerror(errno); + TemporaryFile xsd; + ASSERT_TRUE(android::base::WriteStringToFile(XSD_SOURCE, xsd.path)) << strerror(errno); + EXPECT_TRUE(validateXml("xml", "xsd", xml.path, xsd.path)); +} + +TEST(ValidateXml, IncludeSubdirRelativePath) { + TemporaryDir xmlIncludeDir; + TemporaryFile xmlInclude(xmlIncludeDir.path); + ASSERT_TRUE(android::base::WriteStringToFile(substitute(XML_INCLUDED_SOURCE, MODULE_SOURCE), + xmlInclude.path)) + << strerror(errno); + TemporaryFile xml; + ASSERT_EQ(android::base::Dirname(xml.path), android::base::Dirname(xmlIncludeDir.path)); + ASSERT_TRUE(android::base::WriteStringToFile( + substitute(VALID_XML_SOURCE, + substitute(XI_INCLUDE, android::base::Basename(xmlIncludeDir.path) + "/" + + android::base::Basename(xmlInclude.path))), + xml.path)) + << strerror(errno); + TemporaryFile xsd; + ASSERT_TRUE(android::base::WriteStringToFile(XSD_SOURCE, xsd.path)) << strerror(errno); + EXPECT_TRUE(validateXml("xml", "xsd", xml.path, xsd.path)); +} + +TEST(ValidateXml, IncludeParentDirRelativePath) { + // An XML file from a subdirectory includes a file from the parent directory using '..' syntax. + TemporaryFile xmlInclude; + ASSERT_TRUE(android::base::WriteStringToFile(substitute(XML_INCLUDED_SOURCE, MODULE_SOURCE), + xmlInclude.path)) + << strerror(errno); + TemporaryDir xmlIncludeDir; + TemporaryFile xmlParentInclude(xmlIncludeDir.path); + ASSERT_TRUE(android::base::WriteStringToFile( + substitute(XML_INCLUDED_SOURCE, + substitute(XI_INCLUDE, "../" + android::base::Basename(xmlInclude.path))), + xmlParentInclude.path)) + << strerror(errno); + TemporaryFile xml; + ASSERT_EQ(android::base::Dirname(xml.path), android::base::Dirname(xmlInclude.path)); + ASSERT_EQ(android::base::Dirname(xml.path), android::base::Dirname(xmlIncludeDir.path)); + ASSERT_TRUE(android::base::WriteStringToFile( + substitute( + VALID_XML_SOURCE, + substitute(XI_INCLUDE, android::base::Basename(xmlIncludeDir.path) + "/" + + android::base::Basename(xmlParentInclude.path))), + xml.path)) + << strerror(errno); + TemporaryFile xsd; + ASSERT_TRUE(android::base::WriteStringToFile(XSD_SOURCE, xsd.path)) << strerror(errno); + EXPECT_TRUE(validateXml("xml", "xsd", xml.path, xsd.path)); +}