https://bugzilla.redhat.com/show_bug.cgi?id=1107420 Add a new define/create flag VIR_STORAGE_POOL_DEF_PARSE_VALIDATE_NAME to disallow new storage pools to be defined/created using a name comprised entirely of spaces. Alter the storagepoolxml2xmltest to add a parse failure scenario and a test in order to prove the failure occurs. Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> --- src/conf/storage_conf.c | 9 +++- src/conf/storage_conf.h | 7 +++ src/storage/storage_driver.c | 6 ++- .../pool-dir-whitespace-name.xml | 18 ++++++++ tests/storagepoolxml2xmltest.c | 45 +++++++++++++++---- 5 files changed, 73 insertions(+), 12 deletions(-) create mode 100644 tests/storagepoolxml2xmlin/pool-dir-whitespace-name.xml diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index e5d35cd254..e8bbe4ea54 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -681,7 +681,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt, char *uuid = NULL; char *target_path = NULL; - virCheckFlags(0, NULL); + virCheckFlags(VIR_STORAGE_POOL_DEF_PARSE_VALIDATE_NAME, NULL); if (VIR_ALLOC(ret) < 0) return NULL; @@ -729,6 +729,13 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt, goto error; } + if ((flags & VIR_STORAGE_POOL_DEF_PARSE_VALIDATE_NAME) && + virStringIsEmpty(ret->name)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("name must contain at least one non blank character")); + goto error; + } + uuid = virXPathString("string(./uuid)", ctxt); if (uuid == NULL) { if (virUUIDGenerate(ret->uuid) < 0) { diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index d6886ad6ca..31851643e9 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -235,6 +235,13 @@ struct _virStoragePoolSourceList { virStoragePoolSourcePtr sources; }; +typedef enum { + /* Perform extra name validation on new storage pool names which + * will cause failure to parse the XML. Initially just that a + * name cannot be all white space. */ + VIR_STORAGE_POOL_DEF_PARSE_VALIDATE_NAME = 1 << 0, +} virStoragePoolDefParseFlags; + virStoragePoolDefPtr virStoragePoolDefParseXML(xmlXPathContextPtr ctxt, unsigned int flags); diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 491c4fab9b..8d7a7b399c 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -690,6 +690,7 @@ storagePoolCreateXML(virConnectPtr conn, virStorageBackendPtr backend; virObjectEventPtr event = NULL; char *stateFile = NULL; + unsigned int parse_flags = VIR_STORAGE_POOL_DEF_PARSE_VALIDATE_NAME; unsigned int build_flags = 0; virCheckFlags(VIR_STORAGE_POOL_CREATE_WITH_BUILD | @@ -699,7 +700,7 @@ storagePoolCreateXML(virConnectPtr conn, VIR_EXCLUSIVE_FLAGS_RET(VIR_STORAGE_POOL_BUILD_OVERWRITE, VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, NULL); - if (!(newDef = virStoragePoolDefParseString(xml, 0))) + if (!(newDef = virStoragePoolDefParseString(xml, parse_flags))) goto cleanup; if (virStoragePoolCreateXMLEnsureACL(conn, newDef) < 0) @@ -787,10 +788,11 @@ storagePoolDefineXML(virConnectPtr conn, virStoragePoolDefPtr def; virStoragePoolPtr pool = NULL; virObjectEventPtr event = NULL; + unsigned int parse_flags = VIR_STORAGE_POOL_DEF_PARSE_VALIDATE_NAME; virCheckFlags(0, NULL); - if (!(newDef = virStoragePoolDefParseString(xml, 0))) + if (!(newDef = virStoragePoolDefParseString(xml, parse_flags))) goto cleanup; if (virXMLCheckIllegalChars("name", newDef->name, "\n") < 0) diff --git a/tests/storagepoolxml2xmlin/pool-dir-whitespace-name.xml b/tests/storagepoolxml2xmlin/pool-dir-whitespace-name.xml new file mode 100644 index 0000000000..024505df03 --- /dev/null +++ b/tests/storagepoolxml2xmlin/pool-dir-whitespace-name.xml @@ -0,0 +1,18 @@ +<pool type='dir'> + <name> </name> + <uuid>70a7eb15-6c34-ee9c-bf57-69e8e5ff3fb2</uuid> + <capacity>0</capacity> + <allocation>0</allocation> + <available>0</available> + <source> + </source> + <target> + <path>///var/////lib/libvirt/images//</path> + <permissions> + <mode>0700</mode> + <owner>-1</owner> + <group>-1</group> + <label>some_label_t</label> + </permissions> + </target> +</pool> diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c index 84f2bfb9ec..7893e79da8 100644 --- a/tests/storagepoolxml2xmltest.c +++ b/tests/storagepoolxml2xmltest.c @@ -17,14 +17,24 @@ #define VIR_FROM_THIS VIR_FROM_NONE static int -testCompareXMLToXMLFiles(const char *inxml, const char *outxml) +testCompareXMLToXMLFiles(const char *inxml, + const char *outxml, + bool expect_parse_fail) { char *actual = NULL; int ret = -1; virStoragePoolDefPtr dev = NULL; - - if (!(dev = virStoragePoolDefParseFile(inxml, 0))) + unsigned int parse_flags = VIR_STORAGE_POOL_DEF_PARSE_VALIDATE_NAME; + + if (!(dev = virStoragePoolDefParseFile(inxml, parse_flags))) { + if (expect_parse_fail) { + VIR_TEST_DEBUG("Got expected parse failure msg='%s'", + virGetLastErrorMessage()); + virResetLastError(); + ret = 0; + } goto fail; + } if (!(actual = virStoragePoolDefFormat(dev))) goto fail; @@ -40,21 +50,28 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml) return ret; } + +typedef struct test_params { + const char *name; + bool expect_parse_fail; +} test_params; + static int testCompareXMLToXMLHelper(const void *data) { int result = -1; char *inxml = NULL; char *outxml = NULL; + const test_params *tp = data; if (virAsprintf(&inxml, "%s/storagepoolxml2xmlin/%s.xml", - abs_srcdir, (const char*)data) < 0 || + abs_srcdir, tp->name) < 0 || virAsprintf(&outxml, "%s/storagepoolxml2xmlout/%s.xml", - abs_srcdir, (const char*)data) < 0) { + abs_srcdir, tp->name) < 0) { goto cleanup; } - result = testCompareXMLToXMLFiles(inxml, outxml); + result = testCompareXMLToXMLFiles(inxml, outxml, tp->expect_parse_fail); cleanup: VIR_FREE(inxml); @@ -68,13 +85,23 @@ mymain(void) { int ret = 0; +#define DO_TEST_FULL(name, expect_parse_fail) \ + do { \ + test_params tp = {name, expect_parse_fail}; \ + if (virTestRun("Storage Pool XML-2-XML " name, \ + testCompareXMLToXMLHelper, &tp) < 0) \ + ret = -1; \ + } while (0) + #define DO_TEST(name) \ - if (virTestRun("Storage Pool XML-2-XML " name, \ - testCompareXMLToXMLHelper, (name)) < 0) \ - ret = -1 + DO_TEST_FULL(name, false); + +#define DO_TEST_PARSE_FAIL(name) \ + DO_TEST_FULL(name, true); DO_TEST("pool-dir"); DO_TEST("pool-dir-naming"); + DO_TEST_PARSE_FAIL("pool-dir-whitespace-name"); DO_TEST("pool-fs"); DO_TEST("pool-logical"); DO_TEST("pool-logical-nopath"); -- 2.17.1 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list