1:50 p.m.
> 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.
I'm not sure that's the actual issue here, but I'm somehow included to
think this is another consequence of the lack of query string handling
for http/https URIs.
In any case, do you have a simple reproducer for this escaping handling
for qemu?
I don't have a simple repro for qemu, but this is pretty close. In
guestfish do this:
add
"/vhds/osdiskforconsul0-osdisk.vhd?se=2016-01-01T00%3A00%3A00Z&sp=r&sv=2014-02-14&sr=b&sig=LOlrHXrQeaqlSEP51hRi7E5KDa9lnkqSvLTaZBmTkrQ%3D"
readonly:true protocol:https server:
gabhartswarmstorage.blob.core.windows.net
After you execute 'run', you should see qemu complaining about something
like a 404 Error and the file not existing. After the unescape change the
above will start working.
> 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 don't think that appending the query string to the path is a good idea.
For example, we do a minimum of parsing of the URI, and for some
protocols (like the "We may have to adjust the path ..." comment says)
we adjust path according to the elements in the query string.
Are you talking about this comment?
/* We may have to adjust the path depending on the protocol. For
* example ceph/rbd URIs look like rbd:///pool/disk, but the
* exportname expected will be "pool/disk". Here, uri->path will be
* "/pool/disk" so we have to knock off the leading '/' character.
*/
This is talking about removing leading the '/' in the path sometimes
depending on protocol. Nothing to do with the query string.
> + if (asprintf(&path, "%s?%s", uri->path,
uri->query_raw) == -1) {
> + perror ("asprintf: path + query_raw");
> + free (*protocol_ret);
> + guestfs_int_free_string_list (*server_ret);
> + free (*username_ret);
> + free (*password_ret);
> + return -1;
> + }
'path' created here is leaked. Also, this needs to take into account
that either uri->path or uri->query_raw may be null.
Good point. Fixed. See attached patch.
> *path_ret = strdup (path ? path : "");
> - if (*path_ret == NULL) {
> - perror ("strdup: path");
> - free (*protocol_ret);
> - guestfs_int_free_string_list (*server_ret);
> - free (*username_ret);
> - free (*password_ret);
> - return -1;
> - }
Why did you remove the error checking?