[libvirt] [RFC: PATCH 2/2] viruri: add more tests

[Eric Blake]

When qparams support was dropped in commit bc1ff160, we forgot
to add tests to ensure that viruri can do the same round trip
handling of a URI.

Also, we forgot to report an OOM error.

NOTE: THIS TEST CURRENTLY FAILS ON ANY QUERY WITH % ESCAPING.

* tests/viruritest.c (mymain): Add tests based on just-deleted
qparamtest.
(testURIParse): Allow difference in input and expected output.
* src/util/viruri.c (virURIFormat): Add missing error.
---

Right now, if uri->query contains escaping, such as one%20two,
virURIFormat mistakenly turns it into one%2520two.  But I'm not
sure how best to fix our code to allow proper round trips.

Therefore, I'm only posting this for review, but we have to fix
the bug before this can go into the tree.

 src/util/viruri.c  |    4 ++-
 tests/viruritest.c |   78 +++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 68 insertions(+), 14 deletions(-)

diff --git a/src/util/viruri.c b/src/util/viruri.c
index 7cca977..8968165 100644
--- a/src/util/viruri.c
+++ b/src/util/viruri.c
@@ -250,8 +250,10 @@ virURIFormat(virURIPtr uri)
     if (xmluri.server != NULL &&
         strchr(xmluri.server, ':') != NULL) {

-        if (virAsprintf(&tmpserver, "[%s]", xmluri.server) < 0)
+        if (virAsprintf(&tmpserver, "[%s]", xmluri.server) < 0) {
+            virReportOOMError();
             return NULL;
+        }

         xmluri.server = tmpserver;
     }
diff --git a/tests/viruritest.c b/tests/viruritest.c
index 9504a3b..d97e9c7 100644
--- a/tests/viruritest.c
+++ b/tests/viruritest.c
@@ -35,6 +35,7 @@

 struct URIParseData {
     const char *uri;
+    const char *uri_out;
     const char *scheme;
     const char *server;
     int port;
@@ -49,21 +50,12 @@ static int testURIParse(const void *args)
     int ret = -1;
     virURIPtr uri = NULL;
     const struct URIParseData *data = args;
-    char *uristr;
+    char *uristr = NULL;
     size_t i;

     if (!(uri = virURIParse(data->uri)))
         goto cleanup;

-    if (!(uristr = virURIFormat(uri)))
-        goto cleanup;
-
-    if (!STREQ(uristr, data->uri)) {
-        VIR_DEBUG("URI did not roundtrip, expect '%s', actual '%s'",
-                  data->uri, uristr);
-        goto cleanup;
-    }
-
     if (!STREQ(uri->scheme, data->scheme)) {
         VIR_DEBUG("Expected scheme '%s', actual '%s'",
                   data->scheme, uri->scheme);
@@ -123,6 +115,18 @@ static int testURIParse(const void *args)
         goto cleanup;
     }

+    VIR_FREE(uri->query);
+    uri->query = virURIFormatParams(uri);
+
+    if (!(uristr = virURIFormat(uri)))
+        goto cleanup;
+
+    if (!STREQ(uristr, data->uri_out)) {
+        VIR_DEBUG("URI did not roundtrip, expect '%s', actual '%s'",
+                  data->uri_out, uristr);
+        goto cleanup;
+    }
+
     ret = 0;
 cleanup:
     VIR_FREE(uristr);
@@ -138,14 +142,22 @@ mymain(void)

     signal(SIGPIPE, SIG_IGN);

-#define TEST_PARSE(uri, scheme, server, port, path, query, fragment, params) \
+#define TEST_FULL(uri, uri_out, scheme, server, port, path, query,      \
+                  fragment, params)                                     \
     do  {                                                               \
         const struct URIParseData data = {                              \
-            uri, scheme, server, port, path, query, fragment, params    \
+            uri, (uri_out) ? (uri_out) : (uri), scheme, server, port,   \
+            path, query, fragment, params                               \
         };                                                              \
-        if (virtTestRun("Test IPv6 " # uri,  1, testURIParse, &data) < 0) \
+        if (virtTestRun("Test URI " # uri,  1, testURIParse, &data) < 0) \
             ret = -1;                                                   \
     } while (0)
+#define TEST_PARSE(uri, scheme, server, port, path, query, fragment, params) \
+    TEST_FULL(uri, NULL, scheme, server, port, path, query, fragment, params)
+#define TEST_PARAMS(query_in, query_out, params)                        \
+    TEST_FULL("test://example.com/?" query_in,                          \
+              *query_out ? "test://example.com/?" query_out : NULL,     \
+              "test", "example.com", 0, "/", query_in, NULL, params)

     virURIParam params[] = {
         { (char*)"name", (char*)"value" },
@@ -159,6 +171,46 @@ mymain(void)
     TEST_PARSE("test://[::1]:123/system", "test", "::1", 123, "/system", NULL, NULL, NULL);
     TEST_PARSE("test://[2001:41c8:1:4fd4::2]:123/system", "test", "2001:41c8:1:4fd4::2", 123, "/system", NULL, NULL, NULL);

+    virURIParam params1[] = {
+        { (char*)"foo", (char*)"one" },
+        { (char*)"bar", (char*)"two" },
+        { NULL, NULL },
+    };
+    virURIParam params2[] = {
+        { (char*)"foo", (char*)"one" },
+        { (char*)"foo", (char*)"two" },
+        { NULL, NULL },
+    };
+    virURIParam params3[] = {
+        { (char*)"foo", (char*)"&one" },
+        { (char*)"bar", (char*)"&two" },
+        { NULL, NULL },
+    };
+    virURIParam params4[] = {
+        { (char*)"foo", (char*)"" },
+        { NULL, NULL },
+    };
+    virURIParam params5[] = {
+        { (char*)"foo", (char*)"one two" },
+        { NULL, NULL },
+    };
+    virURIParam params6[] = {
+        { (char*)"foo", (char*)"one" },
+        { NULL, NULL },
+    };
+
+    TEST_PARAMS("foo=one&bar=two", "", params1);
+    TEST_PARAMS("foo=one&foo=two", "", params2);
+    TEST_PARAMS("foo=one&&foo=two", "foo=one&foo=two", params2);
+    TEST_PARAMS("foo=one;foo=two", "foo=one&foo=two", params2);
+    TEST_PARAMS("foo=%26one&bar=%26two", "", params3);
+    TEST_PARAMS("foo", "foo=", params4);
+    TEST_PARAMS("foo=", "", params4);
+    TEST_PARAMS("foo=&", "foo=", params4);
+    TEST_PARAMS("foo=&&", "foo=", params4);
+    TEST_PARAMS("foo=one%20two", "", params5);
+    TEST_PARAMS("=bogus&foo=one", "foo=one", params6);
+
     return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
 }

-- 
1.7.7.6