[PATCH v4 wayland] client: Allow absolute paths in WAYLAND_DISPLAY
Pekka Paalanen
ppaalanen at gmail.com
Mon Dec 11 08:03:22 UTC 2017
On Mon, 27 Nov 2017 08:54:54 -0600
Matt Hoosier <matt.hoosier at gmail.com> wrote:
> From: Matt Hoosier <matt.hoosier at garmin.com>
>
> In order to support system compositor instances, it is necessary to
> allow clients' wl_display_connect() to find the compositor's listening
> socket somewhere outside of XDG_RUNTIME_DIR. For a full account, see
> the discussion beginning here:
>
> https://lists.freedesktop.org/archives/wayland-devel/2017-November/035664.html
>
> This change adjusts the client-side connection logic so that, if
> WAYLAND_DISPLAY is formatted as an absolute pathname, the socket
> connection attempt is made to just $WAYLAND_DISPLAY rather than
> usual user-private location $XDG_RUNTIME_DIR/$WAYLAND_DISPLAY.
>
> This change is based on Davide Bettio's submission of the same concept
> at:
>
> https://lists.freedesktop.org/archives/wayland-devel/2015-August/023838.html.
>
> v4 changes:
>
> * Improved internal comments and some boundary-condition
> error checks in test case.
> * Refer to compositor as "Wayland server" rather than "Wayland
> display" in wl_display_connect() doxygen comments.
> * Remove redundant descriptions of parameter-interpretation
> mechanics from wl_display_connect() manpage. Reworked things
> to make it clear that 'name' and $WAYLAND_DISLAY are each
> capable of encoding absolute server socket paths.
> * Remove callout to reference implementation behavior in protocol
> documented. In its place there is now a simple statement that
> implementations can optionally support absolute socket paths.
>
> v3 changes:
>
> * Added test case.
> * Clarified documentation to note that 'name' parameter to wl_display_connect()
> can also be an absolute path.
>
> v2 changes:
>
> * Added backward incompatibility note to wl_display_connect() manpage.
> * Rephased wl_display_connect() manpage changes to precisely match actual
> changed behavior.
> * Added mention of new absolute path behavior in wl_display_connect()
> doxygen comments.
> * Mentioned new absolute path interpretation of WAYLAND_DISPLAY in
> protocol documentation.
>
> Signed-off-by: Matt Hoosier <matt.hoosier at gmail.com>
> Acked-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> Acked-by: Jonas Ådahl <jadahl at gmail.com>
> ---
> doc/man/wl_display_connect.xml | 32 +++++++++--
> doc/publican/sources/Protocol.xml | 5 +-
> src/wayland-client.c | 47 ++++++++++++----
> tests/socket-test.c | 109 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 177 insertions(+), 16 deletions(-)
Hi Matt,
this patch is:
Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
The wording in the man page sounds little like WAYLAND_DISPLAY
accepting an absolute path is a side-effect rather than an intentional
feature, but it doesn't matter.
Everyone,
if there are no objections, I will push this patch on Wednesday, that
is in two days. If you want your R-b or Acks recorded that are not
already in the above, please send them explicitly.
Thanks,
pq
>
> diff --git a/doc/man/wl_display_connect.xml b/doc/man/wl_display_connect.xml
> index 7e6e05c..dab4ddb 100644
> --- a/doc/man/wl_display_connect.xml
> +++ b/doc/man/wl_display_connect.xml
> @@ -55,15 +55,39 @@
> <title>Description</title>
> <para><function>wl_display_connect</function> connects to a Wayland socket
> that was previously opened by a Wayland server. The server socket must
> - be placed in <envar>XDG_RUNTIME_DIR</envar> for this function to
> - find it. The <varname>name</varname> argument specifies the name of
> + be placed in <envar>XDG_RUNTIME_DIR</envar> when <envar>WAYLAND_DISPLAY</envar>
> + (or <varname>name</varname>, see below) is a simple name, for this
> + function to find it. The server socket is also allowed to exist at an
> + arbitrary path; usage details follow. See below for compatibility issue
> + details.</para>
> +
> + <para>The <varname>name</varname> argument specifies the name of
> the socket or <constant>NULL</constant> to use the default (which is
> <constant>"wayland-0"</constant>). The environment variable
> - <envar>WAYLAND_DISPLAY</envar> replaces the default value. If
> - <envar>WAYLAND_SOCKET</envar> is set, this function behaves like
> + <envar>WAYLAND_DISPLAY</envar> replaces the default value.
> +
> + If <varname>name</varname> is an absolute path, then that path is used
> + as the Wayland socket to which the connection is attempted. Note that
> + in combination with the default-value behavior described above, this
> + implies that setting <envar>WAYLAND_DISPLAY</envar> to an absolute
> + path will implicitly cause <varname>name</varname> to take on that
> + absolute path if <varname>name</varname> is <constant>NULL</constant>.
> +
> + If <envar>WAYLAND_SOCKET</envar> is set, this function behaves like
> <function>wl_display_connect_to_fd</function> with the file-descriptor
> number taken from the environment variable.</para>
>
> + <para>Support for interpreting <envar>WAYLAND_DISPLAY</envar> as an
> + absolute path is a change in behavior compared to
> + <function>wl_display_connect</function>'s behavior in versions
> + 1.14 and older of Wayland. It is no longer guaranteed in versions
> + 1.15 and higher that the Wayland socket chosen is equivalent to
> + manually constructing a socket pathname by concatenating
> + <envar>XDG_RUNTIME_DIR</envar> and <envar>WAYLAND_DISPLAY</envar>.
> + Manual construction of the socket path must account for the
> + possibility that <envar>WAYLAND_DISPLAY</envar> contains an absolute
> + path.</para>
> +
> <para><function>wl_display_connect_to_fd</function> connects to a Wayland
> socket with an explicit file-descriptor. The file-descriptor is passed
> as argument <varname>fd</varname>.</para>
> diff --git a/doc/publican/sources/Protocol.xml b/doc/publican/sources/Protocol.xml
> index ba6b5f1..9fdee9a 100644
> --- a/doc/publican/sources/Protocol.xml
> +++ b/doc/publican/sources/Protocol.xml
> @@ -94,7 +94,10 @@
> The protocol is sent over a UNIX domain stream socket, where the endpoint
> usually is named <systemitem class="service">wayland-0</systemitem>
> (although it can be changed via <emphasis>WAYLAND_DISPLAY</emphasis>
> - in the environment).
> + in the environment). Beginning in Wayland 1.15, implementations can
> + optionally support server socket endpoints located at arbitrary
> + locations in the filesystem by setting <emphasis>WAYLAND_DISPLAY</emphasis>
> + to the absolute path at which the server endpoint listens.
> </para>
> <para>
> Every message is structured as 32-bit words; values are represented in the
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index 3d7361e..ecb8840 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c
> @@ -857,9 +857,17 @@ connect_to_socket(const char *name)
> socklen_t size;
> const char *runtime_dir;
> int name_size, fd;
> + bool path_is_absolute;
> +
> + if (name == NULL)
> + name = getenv("WAYLAND_DISPLAY");
> + if (name == NULL)
> + name = "wayland-0";
> +
> + path_is_absolute = name[0] == '/';
>
> runtime_dir = getenv("XDG_RUNTIME_DIR");
> - if (!runtime_dir) {
> + if (!runtime_dir && !path_is_absolute) {
> wl_log("error: XDG_RUNTIME_DIR not set in the environment.\n");
> /* to prevent programs reporting
> * "failed to create display: Success" */
> @@ -867,25 +875,32 @@ connect_to_socket(const char *name)
> return -1;
> }
>
> - if (name == NULL)
> - name = getenv("WAYLAND_DISPLAY");
> - if (name == NULL)
> - name = "wayland-0";
> -
> fd = wl_os_socket_cloexec(PF_LOCAL, SOCK_STREAM, 0);
> if (fd < 0)
> return -1;
>
> memset(&addr, 0, sizeof addr);
> addr.sun_family = AF_LOCAL;
> - name_size =
> - snprintf(addr.sun_path, sizeof addr.sun_path,
> - "%s/%s", runtime_dir, name) + 1;
> + if (!path_is_absolute) {
> + name_size =
> + snprintf(addr.sun_path, sizeof addr.sun_path,
> + "%s/%s", runtime_dir, name) + 1;
> + } else {
> + /* absolute path */
> + name_size =
> + snprintf(addr.sun_path, sizeof addr.sun_path,
> + "%s", name) + 1;
> + }
>
> assert(name_size > 0);
> if (name_size > (int)sizeof addr.sun_path) {
> - wl_log("error: socket path \"%s/%s\" plus null terminator"
> - " exceeds 108 bytes\n", runtime_dir, name);
> + if (!path_is_absolute) {
> + wl_log("error: socket path \"%s/%s\" plus null terminator"
> + " exceeds %i bytes\n", runtime_dir, name, (int) sizeof(addr.sun_path));
> + } else {
> + wl_log("error: socket path \"%s\" plus null terminator"
> + " exceeds %i bytes\n", name, (int) sizeof(addr.sun_path));
> + }
> close(fd);
> /* to prevent programs reporting
> * "failed to add socket: Success" */
> @@ -994,6 +1009,16 @@ wl_display_connect_to_fd(int fd)
> * its value will be replaced with the WAYLAND_DISPLAY environment
> * variable if it is set, otherwise display "wayland-0" will be used.
> *
> + * If \c name is an absolute path, then that path is used as-is for
> + * the location of the socket at which the Wayland server is listening;
> + * no qualification inside XDG_RUNTIME_DIR is attempted.
> + *
> + * If \c name is \c NULL and the WAYLAND_DISPLAY environment variable
> + * is set to an absolute pathname, then that pathname is used as-is
> + * for the socket in the same manner as if \c name held an absolute
> + * path. Support for absolute paths in \c name and WAYLAND_DISPLAY
> + * is present since Wayland version 1.15.
> + *
> * \memberof wl_display
> */
> WL_EXPORT struct wl_display *
> diff --git a/tests/socket-test.c b/tests/socket-test.c
> index bb034f4..e970562 100644
> --- a/tests/socket-test.c
> +++ b/tests/socket-test.c
> @@ -26,10 +26,14 @@
> #include <errno.h>
> #include <string.h>
> #include <stdio.h>
> +#include <sys/socket.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> #include <sys/un.h>
> #include <unistd.h>
>
> #include "wayland-client.h"
> +#include "wayland-os.h"
> #include "wayland-server.h"
> #include "test-runner.h"
>
> @@ -173,3 +177,108 @@ TEST(add_socket_auto)
>
> wl_display_destroy(d);
> }
> +
> +struct client_create_listener {
> + struct wl_listener listener;
> + struct wl_display *display;
> +};
> +
> +struct client_destroy_listener {
> + struct wl_listener listener;
> + struct wl_display *display;
> +};
> +
> +static void
> +client_destroy_notify(struct wl_listener *l, void *data)
> +{
> + struct client_destroy_listener *listener =
> + wl_container_of(l, listener, listener);
> + wl_display_terminate(listener->display);
> + free(listener);
> +}
> +
> +static void
> +client_create_notify(struct wl_listener *l, void *data)
> +{
> + struct wl_client *client = data;
> + struct client_create_listener *listener =
> + wl_container_of(l, listener, listener);
> + struct client_destroy_listener *destroy_listener = (struct client_destroy_listener *)malloc(sizeof *destroy_listener);
> + assert(destroy_listener != NULL);
> + destroy_listener->display = listener->display;
> + destroy_listener->listener.notify = client_destroy_notify;
> + wl_client_add_destroy_listener(client, &destroy_listener->listener);
> +}
> +
> +TEST(absolute_socket_path)
> +{
> + struct wl_display *display;
> + struct client_create_listener client_create_listener;
> + struct sockaddr_un addr;
> + int fd;
> + socklen_t size;
> + const char *xdg_runtime_dir;
> + size_t len;
> + int ret;
> + pid_t pid;
> +
> + /* It's a little weird that this test about absolute socket paths
> + * uses XDG_RUNTIME_DIR, but that's the only location guaranteed
> + * by test-runner to be both writable and unique. This isn't
> + * really a problem; we'll just take care that the leaf-level
> + * filename used for the socket isn't anything that would
> + * accidentally be generated by a default usage of wl_display_connect(). */
> + xdg_runtime_dir = require_xdg_runtime_dir();
> + memset(&addr, 0, sizeof addr);
> + len = snprintf(addr.sun_path, sizeof addr.sun_path,
> + "%s/%s", xdg_runtime_dir, "wayland-absolute-0");
> + assert(len < sizeof addr.sun_path
> + && "Bug in test. Path too long");
> +
> + /* The path must not exist prior to binding. */
> + assert(access(addr.sun_path, F_OK) == -1);
> +
> + size = offsetof (struct sockaddr_un, sun_path) + strlen(addr.sun_path);
> + addr.sun_family = AF_LOCAL;
> + fd = wl_os_socket_cloexec(PF_LOCAL, SOCK_STREAM, 0);
> + assert(fd >= 0 );
> + ret = bind(fd, (struct sockaddr *) &addr, size);
> + assert(ret >= 0);
> + ret = listen(fd, 128);
> + assert(ret >= 0);
> +
> + /* Start server display. Be careful (by avoiding wl_display_add_socket_auto()
> + * to offer only the absolutely qualified socket made above. */
> + display = wl_display_create();
> + assert(display != NULL);
> + client_create_listener.listener.notify = client_create_notify;
> + client_create_listener.display = display;
> + wl_display_add_client_created_listener(display, &client_create_listener.listener);
> + ret = wl_display_add_socket_fd(display, fd);
> + assert(ret == 0);
> +
> + /* Execute client that connects to the absolutely qualified server socket path. */
> + pid = fork();
> + assert(pid != -1);
> +
> + if (pid == 0) {
> + ret = setenv("WAYLAND_DISPLAY", addr.sun_path, 1);
> + assert(ret == 0);
> + struct wl_display *client_display = wl_display_connect(NULL);
> + assert(client_display != NULL);
> + ret = wl_display_roundtrip(client_display);
> + assert(ret != -1);
> + wl_display_disconnect(client_display);
> + exit(0);
> + assert(false);
> + }
> +
> + wl_display_run(display);
> + ret = waitpid(pid, NULL, 0);
> + assert(ret == pid);
> +
> + wl_display_destroy(display);
> +
> + ret = unlink(addr.sun_path);
> + assert(ret == 0);
> +}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20171211/3d4a4b43/attachment.sig>
More information about the wayland-devel
mailing list