Attachment 96034 Details for Bug 75006 – Multiple cookie files fix 2/2

[patch] Multiple cookie files fix 2/2

0003-client-conf-Don-t-create-multiple-cookie-files.patch (text/plain), 12.85 KB, created by Tanu Kaskinen on 2014-03-19 10:38:26 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tanu Kaskinen

Created: 2014-03-19 10:38:26 UTC

Size: 12.85 KB

patch

obsolete

>From 87e6dcac72e410225f3f66a77bc2885e2b0ceae9 Mon Sep 17 00:00:00 2001
>From: Tanu Kaskinen <tanuk@iki.fi>
>Date: Wed, 19 Mar 2014 11:49:58 +0200
>Subject: [PATCH 3/3] client-conf: Don't create multiple cookie files
>
>The old code loaded cookies at the time of loading the client
>configuration, which could lead to creation of multiple cookie files.
>For example, when pa_client_conf_load() was called, the default cookie
>file was created, and then if PULSE_COOKIE was set,
>pa_client_conf_env() would create another cookie file.
>
>This patch moves the loading of the cookie to a separate function,
>which pa_context calls just before needing the cookie, so the cookie
>won't be loaded from the default file if PULSE_COOKIE is set. This
>patch also splits the single cookie and cookie_file fields in
>pa_client_conf into multiple fields, one for each possible cookie
>source. That change allows falling back to another cookie source if
>the primary source doesn't work.
>
>BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=75006
>---
> src/pulse/client-conf-x11.c |   4 +-
> src/pulse/client-conf.c     | 148 +++++++++++++++++++++++---------------------
> src/pulse/client-conf.h     |  29 +++++----
> src/pulse/context.c         |  11 ++--
> 4 files changed, 104 insertions(+), 88 deletions(-)
>
>diff --git a/src/pulse/client-conf-x11.c b/src/pulse/client-conf-x11.c
>index b37f837..0036e4a 100644
>--- a/src/pulse/client-conf-x11.c
>+++ b/src/pulse/client-conf-x11.c
>@@ -92,10 +92,12 @@ int pa_client_conf_from_x11(pa_client_conf *c) {
>     }
> 
>     if (pa_x11_get_prop(xcb, screen, "PULSE_COOKIE", t, sizeof(t))) {
>-        if (pa_client_conf_load_cookie_from_hex(c, t) < 0) {
>+        if (pa_parsehex(t, c->cookie_from_x11, sizeof(c->cookie_from_x11)) != sizeof(c->cookie_from_x11)) {
>             pa_log(_("Failed to parse cookie data"));
>             goto finish;
>         }
>+
>+        c->cookie_from_x11_valid = true;
>     }
> 
>     ret = 0;
>diff --git a/src/pulse/client-conf.c b/src/pulse/client-conf.c
>index ea583a3..1c24212 100644
>--- a/src/pulse/client-conf.c
>+++ b/src/pulse/client-conf.c
>@@ -57,23 +57,22 @@ static const pa_client_conf default_conf = {
>     .default_source = NULL,
>     .default_server = NULL,
>     .default_dbus_server = NULL,
>+    .cookie_file_from_env = NULL,
>+    .cookie_from_x11_valid = false,
>+    .cookie_file_from_application = NULL,
>+    .cookie_file_from_client_conf = NULL,
>     .autospawn = true,
>     .disable_shm = false,
>-    .cookie_file = NULL,
>-    .cookie_valid = false,
>     .shm_size = 0,
>     .auto_connect_localhost = false,
>     .auto_connect_display = false
> };
> 
>-static int parse_cookie_file(pa_client_conf* c);
>-
> pa_client_conf *pa_client_conf_new(void) {
>     pa_client_conf *c = pa_xmemdup(&default_conf, sizeof(default_conf));
> 
>     c->daemon_binary = pa_xstrdup(PA_BINARY);
>     c->extra_arguments = pa_xstrdup("--log-target=syslog");
>-    c->cookie_file = NULL;
> 
>     return c;
> }
>@@ -86,7 +85,9 @@ void pa_client_conf_free(pa_client_conf *c) {
>     pa_xfree(c->default_source);
>     pa_xfree(c->default_server);
>     pa_xfree(c->default_dbus_server);
>-    pa_xfree(c->cookie_file);
>+    pa_xfree(c->cookie_file_from_env);
>+    pa_xfree(c->cookie_file_from_application);
>+    pa_xfree(c->cookie_file_from_client_conf);
>     pa_xfree(c);
> }
> 
>@@ -104,7 +105,7 @@ int pa_client_conf_load(pa_client_conf *c) {
>         { "default-server",         pa_config_parse_string,   &c->default_server, NULL },
>         { "default-dbus-server",    pa_config_parse_string,   &c->default_dbus_server, NULL },
>         { "autospawn",              pa_config_parse_bool,     &c->autospawn, NULL },
>-        { "cookie-file",            pa_config_parse_string,   &c->cookie_file, NULL },
>+        { "cookie-file",            pa_config_parse_string,   &c->cookie_file_from_client_conf, NULL },
>         { "disable-shm",            pa_config_parse_bool,     &c->disable_shm, NULL },
>         { "enable-shm",             pa_config_parse_not_bool, &c->disable_shm, NULL },
>         { "shm-size-bytes",         pa_config_parse_size,     &c->shm_size, NULL },
>@@ -119,9 +120,6 @@ int pa_client_conf_load(pa_client_conf *c) {
> 
>     r = f ? pa_config_parse(fn, f, table, NULL, NULL) : 0;
> 
>-    if (!r)
>-        r = parse_cookie_file(c);
>-
> finish:
>     pa_xfree(fn);
> 
>@@ -131,6 +129,67 @@ finish:
>     return r;
> }
> 
>+int pa_client_conf_load_cookie(pa_client_conf *c, uint8_t *cookie, size_t cookie_length) {
>+    int r;
>+
>+    pa_assert(c);
>+    pa_assert(cookie);
>+    pa_assert(cookie_length > 0);
>+
>+    if (c->cookie_file_from_env) {
>+        r = pa_authkey_load_auto(c->cookie_file_from_env, true, cookie, cookie_length);
>+        if (r >= 0)
>+            return 0;
>+
>+        pa_log_warn("Failed to load cookie from %s (configured with environment variable PULSE_COOKIE): %s",
>+                    c->cookie_file_from_env, pa_cstrerror(errno));
>+    }
>+
>+    if (c->cookie_from_x11_valid) {
>+        if (cookie_length == sizeof(c->cookie_from_x11)) {
>+            memcpy(cookie, c->cookie_from_x11, cookie_length);
>+            return 0;
>+        }
>+
>+        pa_log_warn("Failed to load cookie from X11 root window property PULSE_COOKIE: size mismatch.");
>+    }
>+
>+    if (c->cookie_file_from_application) {
>+        r = pa_authkey_load_auto(c->cookie_file_from_application, true, cookie, cookie_length);
>+        if (r >= 0)
>+            return 0;
>+
>+        pa_log_warn("Failed to load cookie from %s (configured by the application): %s", c->cookie_file_from_application,
>+                    pa_cstrerror(errno));
>+    }
>+
>+    if (c->cookie_file_from_client_conf) {
>+        r = pa_authkey_load_auto(c->cookie_file_from_client_conf, true, cookie, cookie_length);
>+        if (r >= 0)
>+            return 0;
>+
>+        pa_log_warn("Failed to load cookie from %s (configured in client.conf): %s", c->cookie_file_from_client_conf,
>+                    pa_cstrerror(errno));
>+    }
>+
>+    r = pa_authkey_load_auto(PA_NATIVE_COOKIE_FILE, false, cookie, cookie_length);
>+    if (r >= 0)
>+        return 0;
>+
>+    r = pa_authkey_load_auto(PA_NATIVE_COOKIE_FILE_FALLBACK, false, cookie, cookie_length);
>+    if (r >= 0)
>+        return 0;
>+
>+    r = pa_authkey_load_auto(PA_NATIVE_COOKIE_FILE, true, cookie, cookie_length);
>+    if (r >= 0)
>+        return 0;
>+
>+    pa_log_warn("Failed to load cookie file from %s: %s", PA_NATIVE_COOKIE_FILE, pa_cstrerror(errno));
>+    memset(cookie, 0, cookie_length);
>+
>+    return -1;
>+}
>+
> int pa_client_conf_env(pa_client_conf *c) {
>     char *e;
> 
>@@ -157,73 +216,18 @@ int pa_client_conf_env(pa_client_conf *c) {
>         c->daemon_binary = pa_xstrdup(e);
>     }
> 
>-    if ((e = getenv(ENV_COOKIE_FILE))) {
>-        return pa_client_conf_load_cookie_from_file(c, e);
>-    }
>-
>-    return 0;
>-}
>-
>-static int parse_cookie_file(pa_client_conf* c) {
>-    int k;
>-
>-    pa_assert(c);
>-
>-    c->cookie_valid = false;
>-
>-    if (c->cookie_file)
>-        k = pa_authkey_load_auto(c->cookie_file, true, c->cookie, sizeof(c->cookie));
>-    else {
>-        k = pa_authkey_load_auto(PA_NATIVE_COOKIE_FILE, false, c->cookie, sizeof(c->cookie));
>-
>-        if (k < 0) {
>-            k = pa_authkey_load_auto(PA_NATIVE_COOKIE_FILE_FALLBACK, false, c->cookie, sizeof(c->cookie));
>-
>-            if (k < 0)
>-                k = pa_authkey_load_auto(PA_NATIVE_COOKIE_FILE, true, c->cookie, sizeof(c->cookie));
>-        }
>+    if ((e = getenv(ENV_COOKIE_FILE)) && *e) {
>+        pa_xfree(c->cookie_file_from_env);
>+        c->cookie_file_from_env = pa_xstrdup(e);
>     }
> 
>-    if (k < 0)
>-        return k;
>-
>-    c->cookie_valid = true;
>     return 0;
> }
> 
>-int pa_client_conf_load_cookie_from_hex(pa_client_conf* c, const char *cookie_in_hex) {
>-    uint8_t cookie[PA_NATIVE_COOKIE_LENGTH];
>-
>-    pa_assert(c);
>-    pa_assert(cookie_in_hex);
>-
>-    if (pa_parsehex(cookie_in_hex, cookie, sizeof(cookie)) != sizeof(cookie)) {
>-        pa_log(_("Failed to parse cookie data"));
>-        return -PA_ERR_INVALID;
>-    }
>-
>-    pa_xfree(c->cookie_file);
>-    c->cookie_file = NULL;
>-
>-    return pa_client_conf_set_cookie(c, cookie, PA_NATIVE_COOKIE_LENGTH);
>-}
>-
>-int pa_client_conf_load_cookie_from_file(pa_client_conf *c, const char *cookie_file_path) {
>+void pa_client_conf_set_cookie_file_from_application(pa_client_conf *c, const char *cookie_file) {
>     pa_assert(c);
>-    pa_assert(cookie_file_path);
>+    pa_assert(!cookie_file || *cookie_file);
> 
>-    pa_xfree(c->cookie_file);
>-    c->cookie_file = pa_xstrdup(cookie_file_path);
>-    return parse_cookie_file(c);
>-}
>-
>-int pa_client_conf_set_cookie(pa_client_conf *c, uint8_t *cookie, size_t cookie_size) {
>-    pa_assert(c);
>-    pa_assert(cookie);
>-
>-    if (cookie_size != PA_NATIVE_COOKIE_LENGTH)
>-        return -PA_ERR_INVALID;
>-    memcpy(c->cookie, cookie, cookie_size);
>-    c->cookie_valid = true;
>-    return 0;
>+    pa_xfree(c->cookie_file_from_application);
>+    c->cookie_file_from_application = pa_xstrdup(cookie_file);
> }
>diff --git a/src/pulse/client-conf.h b/src/pulse/client-conf.h
>index c6c74ca..cf424f5 100644
>--- a/src/pulse/client-conf.h
>+++ b/src/pulse/client-conf.h
>@@ -28,10 +28,18 @@
> /* A structure containing configuration data for PulseAudio clients. */
> 
> typedef struct pa_client_conf {
>-    char *daemon_binary, *extra_arguments, *default_sink, *default_source, *default_server, *default_dbus_server, *cookie_file;
>+    char *daemon_binary;
>+    char *extra_arguments;
>+    char *default_sink;
>+    char *default_source;
>+    char *default_server;
>+    char *default_dbus_server;
>+    char *cookie_file_from_env;
>+    uint8_t cookie_from_x11[PA_NATIVE_COOKIE_LENGTH];
>+    bool cookie_from_x11_valid;
>+    char *cookie_file_from_application;
>+    char *cookie_file_from_client_conf;
>     bool autospawn, disable_shm, auto_connect_localhost, auto_connect_display;
>-    uint8_t cookie[PA_NATIVE_COOKIE_LENGTH];
>-    bool cookie_valid; /* non-zero, when cookie is valid */
>     size_t shm_size;
> } pa_client_conf;
> 
>@@ -43,17 +51,16 @@ void pa_client_conf_free(pa_client_conf *c);
>  * the current settings in *c. */
> int pa_client_conf_load(pa_client_conf *c);
> 
>+/* Load the cookie from the cookie sources specified in the configuration, or
>+ * if nothing is specified or none of the sources work, load the cookie from
>+ * the default source. If the default source doesn't work either, this function
>+ * returns a negative value and initializes the cookie to all-zeroes. */
>+int pa_client_conf_load_cookie(pa_client_conf *c, uint8_t *cookie, size_t cookie_length);
>+
> /* Load the configuration data from the environment of the current
>    process, overwriting the current settings in *c. */
> int pa_client_conf_env(pa_client_conf *c);
> 
>-/* Load cookie data from cookie_file_path into c->cookie */
>-int pa_client_conf_load_cookie_from_file(pa_client_conf *c, const char *cookie_file_path);
>-
>-/* Load cookie data from hexdecimal string into c->cookie */
>-int pa_client_conf_load_cookie_from_hex(pa_client_conf *c, const char *cookie_in_hex);
>-
>-/* Set cookie direct from memory */
>-int pa_client_conf_set_cookie(pa_client_conf *c, uint8_t *cookie, size_t cookie_size);
>+void pa_client_conf_set_cookie_file_from_application(pa_client_conf *c, const char *cookie_file);
> 
> #endif
>diff --git a/src/pulse/context.c b/src/pulse/context.c
>index 761d13c..ce19d91 100644
>--- a/src/pulse/context.c
>+++ b/src/pulse/context.c
>@@ -506,6 +506,7 @@ finish:
> }
> 
> static void setup_context(pa_context *c, pa_iochannel *io) {
>+    uint8_t cookie[PA_NATIVE_COOKIE_LENGTH];
>     pa_tagstruct *t;
>     uint32_t tag;
> 
>@@ -524,7 +525,7 @@ static void setup_context(pa_context *c, pa_iochannel *io) {
>     pa_assert(!c->pdispatch);
>     c->pdispatch = pa_pdispatch_new(c->mainloop, c->use_rtclock, command_table, PA_COMMAND_MAX);
> 
>-    if (!c->conf->cookie_valid)
>+    if (pa_client_conf_load_cookie(c->conf, cookie, sizeof(cookie)) < 0)
>         pa_log_info(_("No cookie loaded. Attempting to connect without."));
> 
>     t = pa_tagstruct_command(c, PA_COMMAND_AUTH, &tag);
>@@ -538,7 +539,7 @@ static void setup_context(pa_context *c, pa_iochannel *io) {
>     /* Starting with protocol version 13 we use the MSB of the version
>      * tag for informing the other side if we could do SHM or not */
>     pa_tagstruct_putu32(t, PA_PROTOCOL_VERSION | (c->do_shm ? 0x80000000U : 0));
>-    pa_tagstruct_put_arbitrary(t, c->conf->cookie, sizeof(c->conf->cookie));
>+    pa_tagstruct_put_arbitrary(t, cookie, sizeof(cookie));
> 
> #ifdef HAVE_CREDS
> {
>@@ -1451,11 +1452,13 @@ size_t pa_context_get_tile_size(pa_context *c, const pa_sample_spec *ss) {
> 
> int pa_context_load_cookie_from_file(pa_context *c, const char *cookie_file_path) {
>     pa_assert(c);
>-    pa_assert(cookie_file_path);
>     pa_assert(PA_REFCNT_VALUE(c) >= 1);
> 
>     PA_CHECK_VALIDITY(c, !pa_detect_fork(), PA_ERR_FORKED);
>     PA_CHECK_VALIDITY(c, c->state == PA_CONTEXT_UNCONNECTED, PA_ERR_BADSTATE);
>+    PA_CHECK_VALIDITY(c, !cookie_file_path || *cookie_file_path, PA_ERR_INVALID);
> 
>-    return pa_client_conf_load_cookie_from_file(c->conf, cookie_file_path);
>+    pa_client_conf_set_cookie_file_from_application(c->conf, cookie_file_path);
>+
>+    return 0;
> }
>-- 
>1.9.0
>

Actions: View | Splinter Review

Attachments on bug 75006: 96033 | 96034

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.