1:46 p.m.
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(a)redhat.com>
---
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