| Message ID | 20240412132415.282354-13-thuth@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Drop old distros, bump glib and switch to glib URI parsing code | expand |
On Fri, Apr 12, 2024 at 03:24:14PM +0200, Thomas Huth wrote: > Since version 2.66, glib has useful URI parsing functions, too. > Use those instead of the QEMU-internal ones to be finally able > to get rid of the latter. > > Reviewed-by: Richard W.M. Jones <rjones@redhat.com> > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > block/ssh.c | 75 ++++++++++++++++++++++++++++------------------------- > 1 file changed, 40 insertions(+), 35 deletions(-) > > > - if (g_strcmp0(uri->scheme, "ssh") != 0) { > + if (g_strcmp0(g_uri_get_scheme(uri), "ssh") != 0) { Yet another case-sensitive spot to consider. > > - qdict_put_str(options, "path", uri->path); > - > - /* Pick out any query parameters that we understand, and ignore > - * the rest. > - */ > - for (i = 0; i < qp->n; ++i) { > - if (strcmp(qp->p[i].name, "host_key_check") == 0) { > - qdict_put_str(options, "host_key_check", qp->p[i].value); > + qdict_put_str(options, "path", uri_path); > + > + uri_query = g_uri_get_query(uri); > + if (uri_query) { > + g_uri_params_iter_init(&qp, uri_query, -1, "&", G_URI_PARAMS_NONE); > + while (g_uri_params_iter_next(&qp, &qp_name, &qp_value, &gerror)) { > + if (!qp_name || !qp_value || gerror) { > + warn_report("Failed to parse SSH URI parameters '%s'.", > + uri_query); > + break; > + } > + /* > + * Pick out the query parameters that we understand, and ignore > + * (or rather warn about) the rest. > + */ > + if (g_str_equal(qp_name, "host_key_check")) { > + qdict_put_str(options, "host_key_check", qp_value); > + } else { > + warn_report("Unsupported parameter '%s' in URI.", qp_name); Do we want the trailing '.' in warn_report? The warning is new; it was not in the old code, nor mentioned in the commit message. It seems like a good idea, but we should be more intentional if we intend to make that change.
Eric Blake <eblake@redhat.com> writes: > On Fri, Apr 12, 2024 at 03:24:14PM +0200, Thomas Huth wrote: >> Since version 2.66, glib has useful URI parsing functions, too. >> Use those instead of the QEMU-internal ones to be finally able >> to get rid of the latter. >> >> Reviewed-by: Richard W.M. Jones <rjones@redhat.com> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> block/ssh.c | 75 ++++++++++++++++++++++++++++------------------------- >> 1 file changed, 40 insertions(+), 35 deletions(-) >> > >> >> - if (g_strcmp0(uri->scheme, "ssh") != 0) { >> + if (g_strcmp0(g_uri_get_scheme(uri), "ssh") != 0) { > > Yet another case-sensitive spot to consider. > >> >> - qdict_put_str(options, "path", uri->path); >> - >> - /* Pick out any query parameters that we understand, and ignore >> - * the rest. >> - */ >> - for (i = 0; i < qp->n; ++i) { >> - if (strcmp(qp->p[i].name, "host_key_check") == 0) { >> - qdict_put_str(options, "host_key_check", qp->p[i].value); >> + qdict_put_str(options, "path", uri_path); >> + >> + uri_query = g_uri_get_query(uri); >> + if (uri_query) { >> + g_uri_params_iter_init(&qp, uri_query, -1, "&", G_URI_PARAMS_NONE); >> + while (g_uri_params_iter_next(&qp, &qp_name, &qp_value, &gerror)) { >> + if (!qp_name || !qp_value || gerror) { >> + warn_report("Failed to parse SSH URI parameters '%s'.", >> + uri_query); >> + break; >> + } >> + /* >> + * Pick out the query parameters that we understand, and ignore >> + * (or rather warn about) the rest. >> + */ >> + if (g_str_equal(qp_name, "host_key_check")) { >> + qdict_put_str(options, "host_key_check", qp_value); >> + } else { >> + warn_report("Unsupported parameter '%s' in URI.", qp_name); > > Do we want the trailing '.' in warn_report? We do not. > The warning is new; it was not in the old code, nor mentioned in the > commit message. It seems like a good idea, but we should be more > intentional if we intend to make that change.
diff --git a/block/ssh.c b/block/ssh.c index 2748253d4a..18ae565e55 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -37,7 +37,6 @@ #include "qemu/ctype.h" #include "qemu/cutils.h" #include "qemu/sockets.h" -#include "qemu/uri.h" #include "qapi/qapi-visit-sockets.h" #include "qapi/qapi-visit-block-core.h" #include "qapi/qmp/qdict.h" @@ -181,65 +180,71 @@ static void sftp_error_trace(BDRVSSHState *s, const char *op) static int parse_uri(const char *filename, QDict *options, Error **errp) { - URI *uri = NULL; - QueryParams *qp; + g_autoptr(GUri) uri = g_uri_parse(filename, G_URI_FLAGS_NONE, NULL); + const char *uri_host, *uri_path, *uri_user, *uri_query; char *port_str; - int i; + int port; + g_autoptr(GError) gerror = NULL; + char *qp_name, *qp_value; + GUriParamsIter qp; - uri = uri_parse(filename); if (!uri) { return -EINVAL; } - if (g_strcmp0(uri->scheme, "ssh") != 0) { + if (g_strcmp0(g_uri_get_scheme(uri), "ssh") != 0) { error_setg(errp, "URI scheme must be 'ssh'"); - goto err; + return -EINVAL; } - if (!uri->server || strcmp(uri->server, "") == 0) { + uri_host = g_uri_get_host(uri); + if (!uri_host || g_str_equal(uri_host, "")) { error_setg(errp, "missing hostname in URI"); - goto err; + return -EINVAL; } - if (!uri->path || strcmp(uri->path, "") == 0) { + uri_path = g_uri_get_path(uri); + if (!uri_path || g_str_equal(uri_path, "")) { error_setg(errp, "missing remote path in URI"); - goto err; - } - - qp = query_params_parse(uri->query); - if (!qp) { - error_setg(errp, "could not parse query parameters"); - goto err; + return -EINVAL; } - if(uri->user && strcmp(uri->user, "") != 0) { - qdict_put_str(options, "user", uri->user); + uri_user = g_uri_get_user(uri); + if (uri_user && !g_str_equal(uri_user, "")) { + qdict_put_str(options, "user", uri_user); } - qdict_put_str(options, "server.host", uri->server); + qdict_put_str(options, "server.host", uri_host); - port_str = g_strdup_printf("%d", uri->port ?: 22); + port = g_uri_get_port(uri); + port_str = g_strdup_printf("%d", port > 0 ? port : 22); qdict_put_str(options, "server.port", port_str); g_free(port_str); - qdict_put_str(options, "path", uri->path); - - /* Pick out any query parameters that we understand, and ignore - * the rest. - */ - for (i = 0; i < qp->n; ++i) { - if (strcmp(qp->p[i].name, "host_key_check") == 0) { - qdict_put_str(options, "host_key_check", qp->p[i].value); + qdict_put_str(options, "path", uri_path); + + uri_query = g_uri_get_query(uri); + if (uri_query) { + g_uri_params_iter_init(&qp, uri_query, -1, "&", G_URI_PARAMS_NONE); + while (g_uri_params_iter_next(&qp, &qp_name, &qp_value, &gerror)) { + if (!qp_name || !qp_value || gerror) { + warn_report("Failed to parse SSH URI parameters '%s'.", + uri_query); + break; + } + /* + * Pick out the query parameters that we understand, and ignore + * (or rather warn about) the rest. + */ + if (g_str_equal(qp_name, "host_key_check")) { + qdict_put_str(options, "host_key_check", qp_value); + } else { + warn_report("Unsupported parameter '%s' in URI.", qp_name); + } } } - query_params_free(qp); - uri_free(uri); return 0; - - err: - uri_free(uri); - return -EINVAL; } static bool ssh_has_filename_options_conflict(QDict *options, Error **errp)