+Snesha Foss <sneshaf@microsoft.com> who is taking over this work. It definitely looks like adding the query string to every path is just wrong. I'm not sure I understand why we'd want to parse, deconstruct key value pairs and then reconstruct the query string from these values and append them to the path selectively. This seems like added complexity for a benefit I don't understand. I think in all http and https cases appending the query string is required. It is part of the file path. I think possibly having a flag for turning on and off query string appending would be a good idea for protocols where this is not the case. The question I asked in the previous e-mail, which still needs to be answered no matter what scheme is ultimately chosen, is this: Under what circumstances should the query string be appended to the path? -- Gabriel On Mon, Jun 29, 2015 at 4:03 AM Richard W.M. Jones <rjones@redhat.com> wrote:> On Thu, Jun 25, 2015 at 06:44:50PM +0000, Gabriel Hartmann wrote: > > I have written a patch (please see attached) which fixes both of these > bugs: > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1092583 > > https://bugzilla.redhat.com/show_bug.cgi?id=1232477 > > > > By default, when saving a URI using xmlSaveUri it escapes everything in > the > > URI. QEMU doesn't want anything escaped, so now I unescape everything > > after the URI is generated. Unfortunately there's no flag to change the > > default behavior. > > > > The other problem was that only the "path" portion of the URI struct was > > being used to indicate the path. That's natural enough, but that > practice > > is what was dropping the query string. The query string is kept > separately > > from the path. I now concat the query string back onto the URI with a > > separating '?'. > > > > I've successfully mounted remote vhds in Azure with this new code, and > the > > basic set of tests pass. If there's anything else I can do by way of > > verification, please let me know. > > As Pino says, we can't just append the path and the query string. > > I think the way to go is to add an extra parameter to the > guestfs_add_drive_opts API which would contain an OStringList of (key, > value) pairs (unescaped). > > See generator/actions.ml add_drive > > Rich. > > -- > Richard Jones, Virtualization Group, Red Hat > http://people.redhat.com/~rjones > Read my programming and virtualization blog: http://rwmj.wordpress.com > virt-p2v converts physical machines to virtual machines. Boot with a > live CD or over the network (PXE) and turn machines into KVM guests. > http://libguestfs.org/virt-v2v >
On Mon, Jun 29, 2015 at 05:57:38PM +0000, Gabriel Hartmann wrote:> +Snesha Foss <sneshaf@microsoft.com> who is taking over this work. > > It definitely looks like adding the query string to every path is just > wrong. > > I'm not sure I understand why we'd want to parse, deconstruct key value > pairs and then reconstruct the query string from these values and append > them to the path selectively. This seems like added complexity for a > benefit I don't understand.Yes, you're probably right that we shouldn't do that. However an extra query string parameter (as an OString) would still be needed to the add_drive call.> I think in all http and https cases appending the query string is > required. It is part of the file path. I think possibly having a flag for > turning on and off query string appending would be a good idea for > protocols where this is not the case.There's what happens at the C API level (which is what I'm talking about here), and what guestfish does when it separately parses up URIs. I think there is value in keeping the path and query separate because they require different escaping, or perhaps more accurate to say, currently we escape '?' in the path part, and changing that would break the API for existing users. Guestfish complicates everything by parsing the query for one parameter (?socket=...) although that parameter is not used by the http protocol backend.> The question I asked in the previous e-mail, which still needs to be > answered no matter what scheme is ultimately chosen, is this: > > Under what circumstances should the query string be appended to the path?If there are two separate parameters (path, optional query string) to guestfs_add_drive then I hope it's clear when - you'd append the query string when the query string is passed by the caller. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
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. -- Gabriel
Maybe Matching Threads
- RFC: Handle query strings for http and https (RHBZ#1092583)
- [PATCH] make-fs: respect libguestfs' temporary dir
- [PATCH v2 0/2] lib: qemu: Memoize qemu feature detection.
- [PATCH 2/2] Fix handling of passwords in URLs
- [PATCH 0/4] lib: qemu: Memoize qemu feature detection.