https://bugzilla.redhat.com/show_bug.cgi?id=1107420 Add a new define/create flag VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE_NAME to disallow new snapshots to be defined/created using a name comprised entirely of spaces. Alter the domainsnapshotxml2xmltest to add a test in order to prove the failure occurs. Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> --- Looks like the original send had a network glitch right at this patch so just making sure 11/11 makes it to the list (and hopefully as a reply to the 0/11 cover). src/conf/snapshot_conf.c | 7 ++++ src/conf/snapshot_conf.h | 1 + src/qemu/qemu_driver.c | 3 +- .../name_whitespace.xml | 3 ++ tests/domainsnapshotxml2xmltest.c | 38 ++++++++++++++----- 5 files changed, 42 insertions(+), 10 deletions(-) create mode 100644 tests/domainsnapshotxml2xmlin/name_whitespace.xml diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index adba149241..2897a7edf5 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -235,6 +235,13 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt, goto cleanup; } + if ((flags & VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE_NAME) && + virStringIsEmpty(def->name)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + ("name must contain at least one non blank character")); + goto cleanup; + } + def->description = virXPathString("string(./description)", ctxt); if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) { diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index 20a42bd572..2599c6b57f 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -101,6 +101,7 @@ typedef enum { VIR_DOMAIN_SNAPSHOT_PARSE_DISKS = 1 << 1, VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL = 1 << 2, VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE = 1 << 3, + VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE_NAME = 1 << 4, } virDomainSnapshotParseFlags; virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 37f10d286e..b29faffee1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15279,7 +15279,8 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, virDomainSnapshotDefPtr def = NULL; bool update_current = true; bool redefine = flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE; - unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS; + unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS | + VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE_NAME; virDomainSnapshotObjPtr other = NULL; int align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL; bool align_match = true; diff --git a/tests/domainsnapshotxml2xmlin/name_whitespace.xml b/tests/domainsnapshotxml2xmlin/name_whitespace.xml new file mode 100644 index 0000000000..901bcce62f --- /dev/null +++ b/tests/domainsnapshotxml2xmlin/name_whitespace.xml @@ -0,0 +1,3 @@ +<domainsnapshot> + <name> </name> +</domainsnapshot> diff --git a/tests/domainsnapshotxml2xmltest.c b/tests/domainsnapshotxml2xmltest.c index 5ea0f325de..046e8f7525 100644 --- a/tests/domainsnapshotxml2xmltest.c +++ b/tests/domainsnapshotxml2xmltest.c @@ -74,14 +74,16 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml, const char *uuid, bool internal, - bool redefine) + bool redefine, + bool expect_parse_fail) { char *inXmlData = NULL; char *outXmlData = NULL; char *actual = NULL; int ret = -1; virDomainSnapshotDefPtr def = NULL; - unsigned int flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS; + unsigned int flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS | + VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE_NAME; if (internal) flags |= VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL; @@ -97,8 +99,15 @@ testCompareXMLToXMLFiles(const char *inxml, if (!(def = virDomainSnapshotDefParseString(inXmlData, driver.caps, driver.xmlopt, - flags))) + flags))) { + if (expect_parse_fail) { + VIR_TEST_DEBUG("Got expected parse failure msg='%s'", + virGetLastErrorMessage()); + virResetLastError(); + ret = 0; + } goto cleanup; + } if (!(actual = virDomainSnapshotDefFormat(uuid, def, driver.caps, driver.xmlopt, @@ -135,6 +144,7 @@ struct testInfo { const char *uuid; bool internal; bool redefine; + bool expect_parse_fail; }; @@ -144,7 +154,8 @@ testCompareXMLToXMLHelper(const void *data) const struct testInfo *info = data; return testCompareXMLToXMLFiles(info->inxml, info->outxml, info->uuid, - info->internal, info->redefine); + info->internal, info->redefine, + info->expect_parse_fail); } @@ -168,11 +179,13 @@ mymain(void) } -# define DO_TEST(prefix, name, inpath, outpath, uuid, internal, redefine) \ +# define DO_TEST(prefix, name, inpath, outpath, uuid, internal, redefine, \ + expect_parse_fail) \ do { \ const struct testInfo info = {abs_srcdir "/" inpath "/" name ".xml", \ abs_srcdir "/" outpath "/" name ".xml", \ - uuid, internal, redefine}; \ + uuid, internal, redefine, \ + expect_parse_fail}; \ if (virTestRun("SNAPSHOT XML-2-XML " prefix " " name, \ testCompareXMLToXMLHelper, &info) < 0) \ ret = -1; \ @@ -181,18 +194,23 @@ mymain(void) # define DO_TEST_IN(name, uuid) DO_TEST("in->in", name,\ "domainsnapshotxml2xmlin",\ "domainsnapshotxml2xmlin",\ - uuid, false, false) + uuid, false, false, false) + +# define DO_TEST_IN_PARSE_FAIL(name, uuid) DO_TEST("in->in", name,\ + "domainsnapshotxml2xmlin",\ + "domainsnapshotxml2xmlin",\ + uuid, false, false, true) # define DO_TEST_OUT(name, uuid, internal) DO_TEST("out->out", name,\ "domainsnapshotxml2xmlout",\ "domainsnapshotxml2xmlout",\ - uuid, internal, true) + uuid, internal, true, false) # define DO_TEST_INOUT(name, uuid, internal, redefine) \ DO_TEST("in->out", name,\ "domainsnapshotxml2xmlin",\ "domainsnapshotxml2xmlout",\ - uuid, internal, redefine) + uuid, internal, redefine, false) /* Unset or set all envvars here that are copied in qemudBuildCommandLine * using ADD_ENV_COPY, otherwise these tests may fail due to unexpected @@ -219,6 +237,8 @@ mymain(void) DO_TEST_IN("description_only", NULL); DO_TEST_IN("name_only", NULL); + DO_TEST_IN_PARSE_FAIL("name_whitespace", NULL); + cleanup: if (testSnapshotXMLVariableLineRegex) regfree(testSnapshotXMLVariableLineRegex); -- 2.17.1 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list