> From: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
>
> spice:// has a weird scheme encoding, where it can accept both plain
> and tls ports with URI query parameters. However, it's not very
> convenient nor very common to use (who really want to mix plain & tls
> channels?).
>
> Instead, let's introduce the more readable form spice+tls://host:port
> This form will not accept 'port' or 'tls-port' query string parameter.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> ---
> man/spice-client.pod | 29 +++++++++++++++++------------
> src/spice-session.c | 23 +++++++++++++++++++----
> 2 files changed, 36 insertions(+), 16 deletions(-)
>
> diff --git a/man/spice-client.pod b/man/spice-client.pod
> index 7288b84..459e5f1 100644
> --- a/man/spice-client.pod
> +++ b/man/spice-client.pod
> @@ -12,23 +12,24 @@ can be used to tweak some SPICE-specific option.
>
> =head1 URI
>
> -The most basic SPICE URI which can be used is in the form
> +To initiate a plain SPICE connection (the connection will be
> +unencrypted) to hostname.example.com and port 5900, use the following
> +URI:
> +
> spice://hostname.example.com:5900
>
> -This will try to initiate a SPICE connection to hostname.example.com
> -to port 5900. This connection will be unencrypted. This URI is
> -equivalent to
> - spice://hostname.example.com?port=5900
> +In order to start a TLS connection, one would use:
>
> -In order to start a TLS connection, one would use
> - spice://hostname.example.com?tls-port=5900
> + spice+tls://hostname.example.com:5900
>
> -Other valid URI parameters are 'username' and 'password'. Be careful that
> -passing a password through a SPICE URI might cause the password to be
> -visible by any local user through 'ps'.
> +Note: 'spice+tls' is available since v0.35, you have to use the
> +spice:// query string with the 'tls-port' parameter before that.
> +
> +=head1 URI query string
> +
> +spice URI accepts query string. Several parameters can be specified at
> +once if they are separated by & or ;
>
> -Several parameters can be specified at once if they are separated
> -by & or ;
> spice://hostname.example.com?port=5900;tls-port=5901
>
> When using 'tls-port', it's recommended to not specify any non-TLS port.
> @@ -39,6 +40,10 @@ then try to use the TLS port. This means a
> man-in-the-middle could force
> the whole SPICE session to go in clear text regardless of the TLS settings
> of the SPICE server.
>
> +Other valid URI parameters are 'username' and 'password'. Be careful that
> +passing a password through a SPICE URI might cause the password to be
> +visible by any local user through 'ps'.
> +
> =head1 OPTIONS
>
> The following options are accepted when running a SPICE client which
> diff --git a/src/spice-session.c b/src/spice-session.c
> index e6db424..d2aa5e7 100644
> --- a/src/spice-session.c
> +++ b/src/spice-session.c
> @@ -389,6 +389,7 @@ spice_session_finalize(GObject *gobject)
>
> #define URI_SCHEME_SPICE "spice://"
> #define URI_SCHEME_SPICE_UNIX "spice+unix://"
> +#define URI_SCHEME_SPICE_TLS "spice+tls://"
> #define URI_QUERY_START ";?"
> #define URI_QUERY_SEP ";&"
>
> @@ -425,6 +426,7 @@ static int spice_parse_uri(SpiceSession *session, const
> char *original_uri)
> gchar *authority = NULL;
> gchar *query = NULL;
> gchar *tmp = NULL;
> + bool tls_scheme = false;
>
bool/gboolean?
Fine for me, I like bool is C style and uses less bits.
> g_return_val_if_fail(original_uri != NULL, -1);
>
> @@ -438,12 +440,16 @@ static int spice_parse_uri(SpiceSession *session, const
> char *original_uri)
> /* Break up the URI into its various parts, scheme, authority,
> * path (ignored) and query
> */
> - if (!g_str_has_prefix(uri, URI_SCHEME_SPICE)) {
> + if (g_str_has_prefix(uri, URI_SCHEME_SPICE)) {
> + authority = uri + strlen(URI_SCHEME_SPICE);
> + } else if (g_str_has_prefix(uri, URI_SCHEME_SPICE_TLS)) {
> + authority = uri + strlen(URI_SCHEME_SPICE_TLS);
> + tls_scheme = true;
> + } else {
> g_warning("Expected a URI scheme of '%s' in URI '%s'",
> URI_SCHEME_SPICE, uri);
maybe this can be confusing here?
Not a big deal I think.
> goto fail;
> }
> - authority = uri + strlen(URI_SCHEME_SPICE);
>
> tmp = strchr(authority, '@');
> if (tmp) {
> @@ -531,6 +537,11 @@ static int spice_parse_uri(SpiceSession *session, const
> char *original_uri)
> if (*query)
> query++;
>
> + if (tls_scheme && (g_str_equal(key, "port") || g_str_equal(key,
> "tls-port"))) {
> + g_warning(URI_SCHEME_SPICE_TLS " scheme doesn't accept '%s'",
> key);
> + continue;
> + }
> +
> target_key = NULL;
> if (g_str_equal(key, "port")) {
> target_key = &port;
> @@ -568,8 +579,12 @@ end:
> s->unix_path = g_strdup(path);
> g_free(uri);
> s->host = host;
> - s->port = port;
> - s->tls_port = tls_port;
> + if (tls_scheme) {
> + s->tls_port = port;
> + } else {
> + s->port = port;
> + s->tls_port = tls_port;
> + }
> s->username = username;
> s->password = password;
> return 0;
Otherwise,
Acked-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
OT: there are a lot of mix between of "if (ptr)" and "if (ptr != NULL)" in
these patches and the current code too, no much preference.
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel