[Libguestfs] URI Handling Patch

[Richard W.M. Jones]

On Wed, Jul 01, 2015 at 09:58:05PM +0000, Gabriel Hartmann wrote:
> Hi All,
> 
> Here's the latest patch.  I think this should address the problem.  The
> query string is now only appended to the end of a URI in the HTTP and HTTPS
> cases.
> 
> The add-uri test now passes, and 'make check' still passes.

As a general note, it's easier for us to review patches if you send
them using 'git send-email'.

Also I think you should split up the commit into changes made to the
core library, and changes made to guestfish.

---

diff --git a/.gitignore b/.gitignore
index 6f14915..efa7179 100644
--- a/.gitignore
+++ b/.gitignore
@@ -27,6 +27,7 @@ cscope.out
 .libs
 Makefile
 Makefile.in
+tags

^ This should be in a separate commit.

[...] 
+  if ((STREQ (protocol, "http") || STREQ (protocol, "https")) &&
+      optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_QUERY_BITMASK ) {

^ Probably better to put parentheses around the & expression, although
  I don't believe it is strictly necessary.

diff --git a/src/launch-direct.c b/src/launch-direct.c
index ea67ec9..cb82f20 100644
--- a/src/launch-direct.c
+++ b/src/launch-direct.c
@@ -1224,7 +1224,7 @@ make_uri (guestfs_h *g, const char *scheme, const char *user,
     break;
   }
 
-  return (char *) xmlSaveUri (&uri);
+  return xmlURIUnescapeString((char *) xmlSaveUri (&uri), -1, NULL);

^ I suspect this is wrong.  What is it supposed to do?

---

The general patch looks OK, however it desperately needs unit tests.
There are two places where unit tests might be added:

 - tests/c-api/test-add-drive-opts.c (for the core library)

 - fish/test-add-uri.sh (for guestfish URI parsing)

The tests should be included in the corresponding patch, and you
should also run them to ensure they really work (I know this sounds
obvious, but many people don't do this).

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org