[Libguestfs] RFC: Handle query strings for http and https (RHBZ#1092583)
Richard W.M. Jones
rjones at redhat.com
Mon Feb 2 12:56:20 UTC 2015
On Mon, Feb 02, 2015 at 01:11:07PM +0100, Pino Toscano wrote:
> Parse the query string from URLs, and pass it as new add_drive optarg;
> use it when building the file= URL for qemu.
>
> Accept query string only for http and https protocols, for now.
> ---
> Possibly it looks like an ad-hoc solution for http(s), although I'm not
> sure how it could possibly be generalized somehow (maybe "extra params"
> which would be the query string for http(s)?).
A few comments:
It needs test case(s) in fish/test-add-uri.sh.
I think in the API and code we should either consistently call
it 'query' or 'querystring' (whichever, but consistently).
What's quoting/escaping does the querystring require? Probably the
caller should not have to quote it, but then there are two problems:
(a) does the quoting need to be removed when we parse the guestfish
URL and (b) does quoting need to be added when we call curl at the API
level?
Rich.
>
> customize/customize_main.ml | 4 ++--
> fish/options.c | 6 ++++++
> fish/options.h | 1 +
> fish/uri.c | 27 ++++++++++++++++++++++++---
> fish/uri.h | 1 +
> generator/actions.ml | 11 ++++++++++-
> mllib/uRI.ml | 1 +
> mllib/uRI.mli | 1 +
> mllib/uri-c.c | 13 ++++++++++++-
> resize/resize.ml | 4 ++--
> src/drives.c | 41 +++++++++++++++++++++++++++++++++++++++++
> src/guestfs-internal.h | 2 ++
> src/launch-direct.c | 24 +++++++++++++-----------
> sysprep/main.ml | 4 ++--
> 14 files changed, 118 insertions(+), 22 deletions(-)
>
> diff --git a/customize/customize_main.ml b/customize/customize_main.ml
> index 5bba71a..9b9613f 100644
> --- a/customize/customize_main.ml
> +++ b/customize/customize_main.ml
> @@ -167,12 +167,12 @@ read the man page virt-customize(1).
> fun (uri, format) ->
> let { URI.path = path; protocol = protocol;
> server = server; username = username;
> - password = password } = uri in
> + password = password; query = query } = uri in
> let discard = if readonly then None else Some "besteffort" in
> g#add_drive
> ~readonly ?discard
> ?format ~protocol ?server ?username ?secret:password
> - path
> + ?querystring:query path
> ) files
> in
>
> diff --git a/fish/options.c b/fish/options.c
> index 9ffcc6b..0d5ec2e 100644
> --- a/fish/options.c
> +++ b/fish/options.c
> @@ -68,6 +68,7 @@ option_a (const char *arg, const char *format, struct drv **drvsp)
> drv->uri.server = uri.server;
> drv->uri.username = uri.username;
> drv->uri.password = uri.password;
> + drv->uri.query = uri.query;
> drv->uri.format = format;
> drv->uri.orig_uri = arg;
> }
> @@ -172,6 +173,10 @@ add_drives_handle (guestfs_h *g, struct drv *drv, char next_drive)
> ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_SECRET_BITMASK;
> ad_optargs.secret = drv->uri.password;
> }
> + if (drv->uri.query) {
> + ad_optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_QUERYSTRING_BITMASK;
> + ad_optargs.querystring = drv->uri.query;
> + }
>
> r = guestfs_add_drive_opts_argv (g, drv->uri.path, &ad_optargs);
> if (r == -1)
> @@ -307,6 +312,7 @@ free_drives (struct drv *drv)
> guestfs___free_string_list (drv->uri.server);
> free (drv->uri.username);
> free (drv->uri.password);
> + free (drv->uri.query);
> break;
> case drv_d:
> /* d.filename is optarg, don't free it */
> diff --git a/fish/options.h b/fish/options.h
> index cf68122..d4b0fa5 100644
> --- a/fish/options.h
> +++ b/fish/options.h
> @@ -73,6 +73,7 @@ struct drv {
> char **server; /* server(s) - can be NULL */
> char *username; /* username - can be NULL */
> char *password; /* password - can be NULL */
> + char *query; /* query - can be NULL */
> const char *format; /* format (NULL == autodetect) */
> const char *orig_uri; /* original URI (for error messages etc.) */
> } uri;
> diff --git a/fish/uri.c b/fish/uri.c
> index f45c907..8459a7c 100644
> --- a/fish/uri.c
> +++ b/fish/uri.c
> @@ -34,7 +34,7 @@
> #include "uri.h"
>
> static int is_uri (const char *arg);
> -static int parse (const char *arg, char **path_ret, char **protocol_ret, char ***server_ret, char **username_ret, char **password_ret);
> +static int parse (const char *arg, char **path_ret, char **protocol_ret, char ***server_ret, char **username_ret, char **password_ret, char **query_ret);
> static char *query_get (xmlURIPtr uri, const char *search_name);
> static int make_server (xmlURIPtr uri, const char *socket, char ***ret);
>
> @@ -46,10 +46,11 @@ parse_uri (const char *arg, struct uri *uri_ret)
> char **server = NULL;
> char *username = NULL;
> char *password = NULL;
> + char *query = NULL;
>
> /* Does it look like a URI? */
> if (is_uri (arg)) {
> - if (parse (arg, &path, &protocol, &server, &username, &password) == -1)
> + if (parse (arg, &path, &protocol, &server, &username, &password, &query) == -1)
> return -1;
> }
> else {
> @@ -72,6 +73,7 @@ parse_uri (const char *arg, struct uri *uri_ret)
> uri_ret->server = server;
> uri_ret->username = username;
> uri_ret->password = password;
> + uri_ret->query = query;
> return 0;
> }
>
> @@ -101,7 +103,8 @@ is_uri (const char *arg)
>
> static int
> parse (const char *arg, char **path_ret, char **protocol_ret,
> - char ***server_ret, char **username_ret, char **password_ret)
> + char ***server_ret, char **username_ret, char **password_ret,
> + char **query_ret)
> {
> CLEANUP_XMLFREEURI xmlURIPtr uri = NULL;
> CLEANUP_FREE char *socket = NULL;
> @@ -201,6 +204,24 @@ parse (const char *arg, char **path_ret, char **protocol_ret,
> return -1;
> }
>
> + *query_ret = NULL;
> + /* Copy the query string part only when not building a unix: URI
> + * (e.g. for nbd). See logic done in make_server.
> + */
> + if (uri->query_raw && STRNEQ (uri->query_raw, "") &&
> + !(socket && (uri->server == NULL || STREQ (uri->server, "")))) {
> + *query_ret = strdup (uri->query_raw);
> + if (*query_ret == NULL) {
> + perror ("strdup: query");
> + free (*protocol_ret);
> + guestfs___free_string_list (*server_ret);
> + free (*username_ret);
> + free (*password_ret);
> + free (*path_ret);
> + return -1;
> + }
> + }
> +
> return 0;
> }
>
> diff --git a/fish/uri.h b/fish/uri.h
> index 9202a70..d9b100a 100644
> --- a/fish/uri.h
> +++ b/fish/uri.h
> @@ -27,6 +27,7 @@ struct uri {
> char **server; /* server(s) - can be NULL */
> char *username; /* username - can be NULL */
> char *password; /* password - can be NULL */
> + char *query; /* query string - can be NULL */
> };
>
> /* Parse the '-a' option parameter 'arg', and place the result in
> diff --git a/generator/actions.ml b/generator/actions.ml
> index c0beaae..18cbfc0 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -1318,7 +1318,7 @@ not all belong to a single logical operating system
>
> { defaults with
> name = "add_drive";
> - style = RErr, [String "filename"], [OBool "readonly"; OString "format"; OString "iface"; OString "name"; OString "label"; OString "protocol"; OStringList "server"; OString "username"; OString "secret"; OString "cachemode"; OString "discard"; OBool "copyonread"];
> + style = RErr, [String "filename"], [OBool "readonly"; OString "format"; OString "iface"; OString "name"; OString "label"; OString "protocol"; OStringList "server"; OString "username"; OString "secret"; OString "cachemode"; OString "discard"; OBool "copyonread"; OString "querystring"];
> once_had_no_optargs = true;
> blocking = false;
> fish_alias = ["add"];
> @@ -1575,6 +1575,15 @@ of the same area of disk.
>
> The default is false.
>
> +=item C<querystring>
> +
> +The string parameter C<querystring> specifies the optional query string
> +to be used when accessing some kind of resources, for example:
> +
> + https://example.org/images/?image=test1
> +
> +in such cases, C<image=test1> needs to passed as C<querystring>.
> +
> =back" };
>
> { defaults with
> diff --git a/mllib/uRI.ml b/mllib/uRI.ml
> index d4f7522..9ee53ee 100644
> --- a/mllib/uRI.ml
> +++ b/mllib/uRI.ml
> @@ -22,6 +22,7 @@ type uri = {
> server : string array option;
> username : string option;
> password : string option;
> + query : string option;
> }
>
> external parse_uri : string -> uri = "virt_resize_parse_uri"
> diff --git a/mllib/uRI.mli b/mllib/uRI.mli
> index 0692f95..7fb9acd 100644
> --- a/mllib/uRI.mli
> +++ b/mllib/uRI.mli
> @@ -24,6 +24,7 @@ type uri = {
> server : string array option; (** list of servers *)
> username : string option; (** username *)
> password : string option; (** password *)
> + query : string option; (** query string *)
> }
>
> val parse_uri : string -> uri
> diff --git a/mllib/uri-c.c b/mllib/uri-c.c
> index aa63c48..e649cdb 100644
> --- a/mllib/uri-c.c
> +++ b/mllib/uri-c.c
> @@ -49,7 +49,7 @@ virt_resize_parse_uri (value argv /* arg value, not an array! */)
> caml_invalid_argument ("URI.parse_uri");
>
> /* Convert the struct into an OCaml tuple. */
> - rv = caml_alloc_tuple (5);
> + rv = caml_alloc_tuple (6);
>
> /* path : string */
> sv = caml_copy_string (uri.path);
> @@ -94,5 +94,16 @@ virt_resize_parse_uri (value argv /* arg value, not an array! */)
> ov = Val_int (0);
> Store_field (rv, 4, ov);
>
> + /* query : string option */
> + if (uri.query) {
> + sv = caml_copy_string (uri.query);
> + free (uri.query);
> + ov = caml_alloc (1, 0);
> + Store_field (ov, 0, sv);
> + }
> + else
> + ov = Val_int (0);
> + Store_field (rv, 5, ov);
> +
> CAMLreturn (rv);
> }
> diff --git a/resize/resize.ml b/resize/resize.ml
> index 871c6a4..2a2765c 100644
> --- a/resize/resize.ml
> +++ b/resize/resize.ml
> @@ -338,8 +338,8 @@ read the man page virt-resize(1).
> if verbose then g#set_verbose true;
> let _, { URI.path = path; protocol = protocol;
> server = server; username = username;
> - password = password } = infile in
> - g#add_drive ?format ~readonly:true ~protocol ?server ?username ?secret:password path;
> + password = password; query = query } = infile in
> + g#add_drive ?format ~readonly:true ~protocol ?server ?username ?secret:password ?querystring:query path;
> (* The output disk is being created, so use cache=unsafe here. *)
> g#add_drive ?format:output_format ~readonly:false ~cachemode:"unsafe"
> outfile;
> diff --git a/src/drives.c b/src/drives.c
> index 34bf63d..3668e29 100644
> --- a/src/drives.c
> +++ b/src/drives.c
> @@ -63,6 +63,7 @@ struct drive_create_data {
> const char *cachemode;
> enum discard discard;
> bool copyonread;
> + const char *query;
> };
>
> COMPILE_REGEXP (re_hostname_port, "(.*):(\\d+)$", 0)
> @@ -144,6 +145,7 @@ create_drive_non_file (guestfs_h *g,
> drv->src.username = data->username ? safe_strdup (g, data->username) : NULL;
> drv->src.secret = data->secret ? safe_strdup (g, data->secret) : NULL;
> drv->src.format = data->format ? safe_strdup (g, data->format) : NULL;
> + drv->src.query = data->query ? safe_strdup (g, data->query) : NULL;
>
> drv->readonly = data->readonly;
> drv->iface = data->iface ? safe_strdup (g, data->iface) : NULL;
> @@ -171,6 +173,13 @@ static struct drive *
> create_drive_curl (guestfs_h *g,
> const struct drive_create_data *data)
> {
> + if (data->query != NULL &&
> + data->protocol != drive_protocol_http &&
> + data->protocol != drive_protocol_https) {
> + error (g, _("curl: you cannot specify a query string with this protocol"));
> + return NULL;
> + }
> +
> if (data->nr_servers != 1) {
> error (g, _("curl: you must specify exactly one server"));
> return NULL;
> @@ -207,6 +216,10 @@ create_drive_gluster (guestfs_h *g,
> error (g, _("gluster: you cannot specify a secret with this protocol"));
> return NULL;
> }
> + if (data->query != NULL) {
> + error (g, _("gluster: you cannot specify a query string with this protocol"));
> + return NULL;
> + }
>
> if (data->nr_servers != 1) {
> error (g, _("gluster: you must specify exactly one server"));
> @@ -250,6 +263,10 @@ create_drive_nbd (guestfs_h *g,
> error (g, _("nbd: you cannot specify a secret with this protocol"));
> return NULL;
> }
> + if (data->query != NULL) {
> + error (g, _("nbd: you cannot specify a query string with this protocol"));
> + return NULL;
> + }
>
> if (data->nr_servers != 1) {
> error (g, _("nbd: you must specify exactly one server"));
> @@ -268,6 +285,11 @@ create_drive_rbd (guestfs_h *g,
> {
> size_t i;
>
> + if (data->query != NULL) {
> + error (g, _("rbd: you cannot specify a query string with this protocol"));
> + return NULL;
> + }
> +
> for (i = 0; i < data->nr_servers; ++i) {
> if (data->servers[i].transport != drive_transport_none &&
> data->servers[i].transport != drive_transport_tcp) {
> @@ -307,6 +329,10 @@ create_drive_sheepdog (guestfs_h *g,
> error (g, _("sheepdog: you cannot specify a secret with this protocol"));
> return NULL;
> }
> + if (data->query != NULL) {
> + error (g, _("sheepdog: you cannot specify a query string with this protocol"));
> + return NULL;
> + }
>
> for (i = 0; i < data->nr_servers; ++i) {
> if (data->servers[i].transport != drive_transport_none &&
> @@ -337,6 +363,11 @@ static struct drive *
> create_drive_ssh (guestfs_h *g,
> const struct drive_create_data *data)
> {
> + if (data->query != NULL) {
> + error (g, _("ssh: you cannot specify a query string with this protocol"));
> + return NULL;
> + }
> +
> if (data->nr_servers != 1) {
> error (g, _("ssh: you must specify exactly one server"));
> return NULL;
> @@ -379,6 +410,10 @@ create_drive_iscsi (guestfs_h *g,
> error (g, _("iscsi: you cannot specify a secret with this protocol"));
> return NULL;
> }
> + if (data->query != NULL) {
> + error (g, _("iscsi: you cannot specify a query string with this protocol"));
> + return NULL;
> + }
>
> if (data->nr_servers != 1) {
> error (g, _("iscsi: you must specify exactly one server"));
> @@ -770,6 +805,8 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
> ? optargs->secret : NULL;
> data.cachemode = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_CACHEMODE_BITMASK
> ? optargs->cachemode : NULL;
> + data.query = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_QUERYSTRING_BITMASK
> + ? optargs->querystring : NULL;
>
> if (optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_DISCARD_BITMASK) {
> if (STREQ (optargs->discard, "disable"))
> @@ -835,6 +872,10 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
> error (g, _("you cannot specify a secret with file-backed disks"));
> return -1;
> }
> + if (data.query != NULL) {
> + error (g, _("you cannot specify a query string with file-backed disks"));
> + return -1;
> + }
>
> if (STREQ (filename, "/dev/null"))
> drv = create_drive_dev_null (g, &data);
> diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
> index 573c3da..439a80b 100644
> --- a/src/guestfs-internal.h
> +++ b/src/guestfs-internal.h
> @@ -242,6 +242,8 @@ struct drive_source {
> char *username;
> /* Optional secret (may be NULL if not specified). */
> char *secret;
> + /* Optional query string (may be NULL if not specified). */
> + char *query;
> };
>
> enum discard {
> diff --git a/src/launch-direct.c b/src/launch-direct.c
> index e6ed54a..00f3190 100644
> --- a/src/launch-direct.c
> +++ b/src/launch-direct.c
> @@ -1189,7 +1189,8 @@ qemu_escape_param (guestfs_h *g, const char *param)
> static char *
> make_uri (guestfs_h *g, const char *scheme, const char *user,
> const char *password,
> - struct drive_server *server, const char *path)
> + struct drive_server *server, const char *path,
> + const char *querystring)
> {
> xmlURI uri = { .scheme = (char *) scheme,
> .user = (char *) user };
> @@ -1217,6 +1218,7 @@ make_uri (guestfs_h *g, const char *scheme, const char *user,
> case drive_transport_tcp:
> uri.server = server->u.hostname;
> uri.port = server->port;
> + uri.query_raw = (char *) querystring;
> break;
> case drive_transport_unix:
> query = safe_asprintf (g, "socket=%s", server->u.socket);
> @@ -1258,36 +1260,36 @@ guestfs___drive_source_qemu_param (guestfs_h *g, const struct drive_source *src)
>
> case drive_protocol_ftp:
> return make_uri (g, "ftp", src->username, src->secret,
> - &src->servers[0], src->u.exportname);
> + &src->servers[0], src->u.exportname, NULL);
>
> case drive_protocol_ftps:
> return make_uri (g, "ftps", src->username, src->secret,
> - &src->servers[0], src->u.exportname);
> + &src->servers[0], src->u.exportname, NULL);
>
> case drive_protocol_gluster:
> switch (src->servers[0].transport) {
> case drive_transport_none:
> return make_uri (g, "gluster", NULL, NULL,
> - &src->servers[0], src->u.exportname);
> + &src->servers[0], src->u.exportname, NULL);
> case drive_transport_tcp:
> return make_uri (g, "gluster+tcp", NULL, NULL,
> - &src->servers[0], src->u.exportname);
> + &src->servers[0], src->u.exportname, NULL);
> case drive_transport_unix:
> return make_uri (g, "gluster+unix", NULL, NULL,
> - &src->servers[0], NULL);
> + &src->servers[0], NULL, NULL);
> }
>
> case drive_protocol_http:
> return make_uri (g, "http", src->username, src->secret,
> - &src->servers[0], src->u.exportname);
> + &src->servers[0], src->u.exportname, src->query);
>
> case drive_protocol_https:
> return make_uri (g, "https", src->username, src->secret,
> - &src->servers[0], src->u.exportname);
> + &src->servers[0], src->u.exportname, src->query);
>
> case drive_protocol_iscsi:
> return make_uri (g, "iscsi", NULL, NULL,
> - &src->servers[0], src->u.exportname);
> + &src->servers[0], src->u.exportname, NULL);
>
> case drive_protocol_nbd: {
> CLEANUP_FREE char *p = NULL;
> @@ -1374,11 +1376,11 @@ guestfs___drive_source_qemu_param (guestfs_h *g, const struct drive_source *src)
>
> case drive_protocol_ssh:
> return make_uri (g, "ssh", src->username, src->secret,
> - &src->servers[0], src->u.exportname);
> + &src->servers[0], src->u.exportname, NULL);
>
> case drive_protocol_tftp:
> return make_uri (g, "tftp", src->username, src->secret,
> - &src->servers[0], src->u.exportname);
> + &src->servers[0], src->u.exportname, NULL);
> }
>
> abort ();
> diff --git a/sysprep/main.ml b/sysprep/main.ml
> index 249800f..ff9852c 100644
> --- a/sysprep/main.ml
> +++ b/sysprep/main.ml
> @@ -201,12 +201,12 @@ read the man page virt-sysprep(1).
> fun (uri, format) ->
> let { URI.path = path; protocol = protocol;
> server = server; username = username;
> - password = password } = uri in
> + password = password; query = query } = uri in
> let discard = if readonly then None else Some "besteffort" in
> g#add_drive
> ~readonly ?discard
> ?format ~protocol ?server ?username ?secret:password
> - path
> + ?querystring:query path
> ) files
> in
>
> --
> 1.9.3
>
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
More information about the Libguestfs
mailing list