util: storage: Properly parse URIs with missing trailing slash

The URI parser used by libvirt does not populate uri->path if the
trailing slash is missing. The code virStorageSourceParseBackingURI
would then not populate src->path.

As only NBD network disks are allowed to have the 'name' field in the
XML defining the disk source omitted we'd generate an invalid XML which
we'd not parse again.

Fix it by populating src->path with an empty string if the uri is
lacking slash.

As pointed out above NBD is special in this case since we actually allow
it being NULL. The URI path is used as export name. Since an empty
export does not make sense the new approach clears the src->path if the
trailing slash is present but nothing else.

Add test cases now to cover all the various cases for NBD and non-NBD
uris as there was to time only 1 test abusing the quirk witout slash for
NBD and all other URIs contained the slash or in case of NBD also the
export name.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index eba82918c153a08a0acd739f3fc7b3bbffd48992..94c32d818eaef4f3d39d714319522e86e4f9914c 100644 (file)
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2579,6 +2579,7 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src,
                                 const char *uristr)
 {
     virURIPtr uri = NULL;
+    const char *path = NULL;
     char **scheme = NULL;
     int ret = -1;
 
@@ -2621,10 +2622,23 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src,
 
     /* XXX We currently don't support auth, so don't bother parsing it */
 
-    /* possibly skip the leading slash */
-    if (uri->path &&
-        VIR_STRDUP(src->path,
-                   *uri->path == '/' ? uri->path + 1 : uri->path) < 0)
+    /* uri->path is NULL if the URI does not contain slash after host:
+     * transport://host:port */
+    if (uri->path)
+        path = uri->path;
+    else
+        path = "";
+
+    /* possibly skip the leading slash  */
+    if (path[0] == '/')
+        path++;
+
+    /* NBD allows empty export name (path) */
+    if (src->protocol == VIR_STORAGE_NET_PROTOCOL_NBD &&
+        path[0] == '\0')
+        path = NULL;
+
+    if (VIR_STRDUP(src->path, path) < 0)
         goto cleanup;
 
     if (src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER) {

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 6d85718d9296ebec8959526a707d0a4229692e5e..5848f6b5b5b677d50e76ce59adb9200663e37a16 100644 (file)
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -373,6 +373,9 @@ mymain(void)
     /* type VIR_STORAGE_TYPE_BLOCK is not tested since it parses back to 'file' */
     /* type VIR_STORAGE_TYPE_DIR it is a 'format' driver in qemu */
 
+    TEST_JSON_FORMAT_NET("<source protocol='http' name=''>\n"
+                         "  <host name='example.com' port='80'/>\n"
+                         "</source>\n");
     TEST_JSON_FORMAT_NET("<source protocol='http' name='file'>\n"
                          "  <host name='example.com' port='80'/>\n"
                          "</source>\n");

diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 83680a63a06ee7de19dec0baff04c218d8fa1986..be4f558035f7cffbd34db6df84aaaac02adbb5b9 100644 (file)
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -1303,6 +1303,14 @@ mymain(void)
 
     TEST_BACKING_PARSE("path", "<source file='path'/>\n");
     TEST_BACKING_PARSE("://", NULL);
+    TEST_BACKING_PARSE("http://example.com",
+                       "<source protocol='http' name=''>\n"
+                       "  <host name='example.com' port='80'/>\n"
+                       "</source>\n");
+    TEST_BACKING_PARSE("http://example.com/",
+                       "<source protocol='http' name=''>\n"
+                       "  <host name='example.com' port='80'/>\n"
+                       "</source>\n");
     TEST_BACKING_PARSE("http://example.com/file",
                        "<source protocol='http' name='file'>\n"
                        "  <host name='example.com' port='80'/>\n"
@@ -1315,6 +1323,18 @@ mymain(void)
                        "<source protocol='nbd' name='blah'>\n"
                        "  <host name='example.org' port='6000'/>\n"
                        "</source>\n");
+    TEST_BACKING_PARSE("nbd://example.org:1234",
+                       "<source protocol='nbd'>\n"
+                       "  <host name='example.org' port='1234'/>\n"
+                       "</source>\n");
+    TEST_BACKING_PARSE("nbd://example.org:1234/",
+                       "<source protocol='nbd'>\n"
+                       "  <host name='example.org' port='1234'/>\n"
+                       "</source>\n");
+    TEST_BACKING_PARSE("nbd://example.org:1234/exportname",
+                       "<source protocol='nbd' name='exportname'>\n"
+                       "  <host name='example.org' port='1234'/>\n"
+                       "</source>\n");
 
 #ifdef WITH_YAJL
     TEST_BACKING_PARSE("json:", NULL);