From 70c07e01dee24df0a1591d65799b66a8e89a3bd6 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Mon, 26 Mar 2012 11:23:45 +0100 Subject: [PATCH] Fix and test round-trip of query parameters 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. This round trip was broken, due to use of the old 'query' field of xmlUriPtr, instead of the new 'query_raw' Also, we forgot to report an OOM error. * 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. Use query_raw, instead of query for xmlUriPtr object. --- src/util/viruri.c | 8 ++++- tests/viruritest.c | 78 +++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 72 insertions(+), 14 deletions(-) diff --git a/src/util/viruri.c b/src/util/viruri.c index 7cca977..2c6de51 100644 --- a/src/util/viruri.c +++ b/src/util/viruri.c @@ -243,15 +243,21 @@ virURIFormat(virURIPtr uri) xmluri.server = uri->server; xmluri.port = uri->port; xmluri.path = uri->path; +#ifdef HAVE_XMLURI_QUERY_RAW + xmluri.query_raw = uri->query; +#else xmluri.query = uri->query; +#endif xmluri.fragment = uri->fragment; /* First check: does it make sense to do anything */ 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.1