[libvirt] [PATCH 02/11] conf: Disallow new storage pools to use all white space as name

John Ferlan jferlan at redhat.com
Mon Jul 30 18:46:39 UTC 2018


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 at 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




More information about the libvir-list mailing list