[libvirt] [PATCH 1/2] build: drop obsolete qparams test
Otherwise, 'make check' breaks since commit bc1ff160 deleted qparams.h. A later patch will ensure that viruri takes over what qparams used to do. * tests/qparamtest.c (mymain): Delete, now that we have viruri. * tests/Makefile.am (check_PROGRAMS, TESTS, qparamtest_SOURCES): Delete old test. * .gitignore: Add recent test additions. --- Pushing this under the build-breaker rule. .gitignore | 3 +++ tests/Makefile.am | 7 +------ 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index b35e863..2f27ff4 100644 --- a/.gitignore +++ b/.gitignore @@ -139,11 +139,14 @@ /tests/ssh /tests/statstest /tests/utiltest +/tests/virauthconfigtest /tests/virbuftest /tests/virhashtest +/tests/virkeyfiletest /tests/virnet*test /tests/virshtest /tests/virtimetest +/tests/viruritest /tests/vmx2xmltest /tests/xencapstest /tests/xmconfigtest diff --git a/tests/Makefile.am b/tests/Makefile.am index 0e5ca39..4755a3e 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -92,7 +92,7 @@ EXTRA_DIST = \ .valgrind.supp check_PROGRAMS = virshtest conftest sockettest \ - nodeinfotest qparamtest virbuftest \ + nodeinfotest virbuftest \ commandtest commandhelper seclabeltest \ virhashtest virnetmessagetest virnetsockettest ssh \ utiltest virnettlscontexttest shunloadtest \ @@ -210,7 +210,6 @@ EXTRA_DIST += $(test_scripts) TESTS = virshtest \ nodeinfotest \ - qparamtest \ virbuftest \ sockettest \ commandtest \ @@ -529,10 +528,6 @@ seclabeltest_SOURCES = \ seclabeltest.c seclabeltest_LDADD = ../src/libvirt_driver_security.la $(LDADDS) -qparamtest_SOURCES = \ - qparamtest.c testutils.h testutils.c -qparamtest_LDADD = $(LDADDS) - virbuftest_SOURCES = \ virbuftest.c testutils.h testutils.c virbuftest_LDADD = $(LDADDS) -- 1.7.7.6
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
On Fri, Mar 23, 2012 at 04:09:59PM -0600, Eric Blake wrote:
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.
This is due to mistaken use of the old 'query' field instead of 'query_raw'. The former has broken escape handliing. I pushed your patch with the following added @@ -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 Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange -
Eric Blake