[gjs: 1/2] overrides: Allow DBus methods with file descriptor lists

[Philip Chimento <pchimento src gnome org>]

commit a13bb7a0a0ee0abed6b05eb6e913b42991dd616c
Author: Philip Chimento <philip endlessm com>
Date:   Thu Jan 10 17:59:51 2019 -0800

    overrides: Allow DBus methods with file descriptor lists
    
    For DBus proxies, we now allow calling methods with an extra
    Gio.UnixFDList argument. This does not break existing code because we
    already allowed a bunch of arguments in any order. Gio.DBusProxy.call()
    is the same under the hood as Gio.DBusProxy.call_with_unix_fd_list(),
    only with a null FD list parameter, so we can replace the former with
    the latter.
    
    Previously, synchronously called DBus proxy methods would return an
    unpacked GVariant. Now, if called with a Gio.UnixFDList they will return
    [unpacked GVariant, FDList]. This also does not break existing code
    because it was not possible to call a method with an FDList before, and
    the return value is unchanged if not calling with an FDList. This does
    mean, unfortunately, that if you have a method with an 'h' in its return
    signature but not in its argument signatures, you will have to call it
    with an empty FDList in order to receive an FDList with the return
    value, if calling synchronously.
    
    On the DBus service side, when receiving a method call, we now pass the
    FDList received from DBus to the called method. Previously, sync methods
    were passed the parameters, and async methods were passed the parameters
    plus the Gio.DBusInvocation object. Appending the Gio.UnixFDList to
    those parameters also should not break existing code, although it could
    if the method examines the number of arguments. (That is unlikely, since
    DBus doesn't have methods with variable arguments.)
    
    TODO: Check whether this works on non-Unix platforms.
    
    Closes #204.

 installed-tests/js/testGDBus.js | 114 ++++++++++++++++++++++++++++++++++++++--
 libgjs-private/gjs-util.c       |  79 ++++++++++++++++++++++++++++
 libgjs-private/gjs-util.h       |   3 ++
 modules/overrides/Gio.js        | 114 ++++++++++++++++++++++++++++++----------
 4 files changed, 279 insertions(+), 31 deletions(-)
---
diff --git a/installed-tests/js/testGDBus.js b/installed-tests/js/testGDBus.js
index 6ebb018b..95c9649f 100644
--- a/installed-tests/js/testGDBus.js
+++ b/installed-tests/js/testGDBus.js
@@ -1,9 +1,9 @@
-const Gio = imports.gi.Gio;
-const GLib = imports.gi.GLib;
+const ByteArray = imports.byteArray;
+const {Gio, GjsPrivate, GLib} = imports.gi;
 
 /* The methods list with their signatures.
  *
- * *** NOTE: If you add stuff here, you need to update testIntrospectReal
+ * *** NOTE: If you add stuff here, you need to update the Test class below.
  */
 var TestIface = '<node> \
 <interface name="org.gnome.gjs.Test"> \
@@ -72,6 +72,22 @@ var TestIface = '<node> \
 <method name="structArray"> \
     <arg type="a(ii)" direction="out"/> \
 </method> \
+<method name="fdIn"> \
+    <arg type="h" direction="in"/> \
+    <arg type="ay" direction="out"/> \
+</method> \
+<method name="fdIn2"> \
+    <arg type="h" direction="in"/> \
+    <arg type="ay" direction="out"/> \
+</method> \
+<method name="fdOut"> \
+    <arg type="ay" direction="in"/> \
+    <arg type="h" direction="out"/> \
+</method> \
+<method name="fdOut2"> \
+    <arg type="ay" direction="in"/> \
+    <arg type="h" direction="out"/> \
+</method> \
 <signal name="signalFoo"> \
     <arg type="s" direction="out"/> \
 </signal> \
@@ -201,6 +217,39 @@ class Test {
     structArray() {
         return [[128, 123456], [42, 654321]];
     }
+
+    fdIn(fdIndex, fdList) {
+        const fd = fdList.get(fdIndex);
+        const stream = new Gio.UnixInputStream({fd, closeFd: true});
+        const bytes = stream.read_bytes(4096, null);
+        return bytes;
+    }
+
+    // Same as fdIn(), but implemented asynchronously
+    fdIn2Async([fdIndex], invocation, fdList) {
+        const fd = fdList.get(fdIndex);
+        const stream = new Gio.UnixInputStream({fd, closeFd: true});
+        stream.read_bytes_async(4096, GLib.PRIORITY_DEFAULT, null, (obj, res) => {
+            const bytes = obj.read_bytes_finish(res);
+            invocation.return_value(new GLib.Variant('(ay)', [bytes]));
+        });
+    }
+
+    fdOut(bytes) {
+        const fd = GjsPrivate.open_bytes(bytes);
+        const fdList = Gio.UnixFDList.new_from_array([fd]);
+        return [0, fdList];
+    }
+
+    fdOut2Async([bytes], invocation) {
+        GLib.idle_add(GLib.PRIORITY_DEFAULT, function() {
+            const fd = GjsPrivate.open_bytes(bytes);
+            const fdList = Gio.UnixFDList.new_from_array([fd]);
+            invocation.return_value_with_unix_fd_list(new GLib.Variant('(h)', [0]),
+                fdList);
+            return GLib.SOURCE_REMOVE;
+        });
+    }
 }
 
 const ProxyClass = Gio.DBusProxy.makeProxyWrapper(TestIface);
@@ -484,4 +533,63 @@ describe('Exported DBus object', function () {
         });
         loop.run();
     });
+
+    it('can call a remote method with a Unix FD', function (done) {
+        const expectedBytes = ByteArray.fromString('some bytes');
+        const fd = GjsPrivate.open_bytes(expectedBytes);
+        const fdList = Gio.UnixFDList.new_from_array([fd]);
+        proxy.fdInRemote(0, fdList, ([bytes], exc, outFdList) => {
+            expect(exc).toBeNull();
+            expect(outFdList).toBeNull();
+            expect(bytes).toEqual(expectedBytes);
+            done();
+        });
+    });
+
+    it('can call an asynchronously implemented remote method with a Unix FD', function (done) {
+        const expectedBytes = ByteArray.fromString('some bytes');
+        const fd = GjsPrivate.open_bytes(expectedBytes);
+        const fdList = Gio.UnixFDList.new_from_array([fd]);
+        proxy.fdIn2Remote(0, fdList, ([bytes], exc, outFdList) => {
+            expect(exc).toBeNull();
+            expect(outFdList).toBeNull();
+            expect(bytes).toEqual(expectedBytes);
+            done();
+        });
+    });
+
+    function readBytesFromFdSync(fd) {
+        const stream = new Gio.UnixInputStream({fd, closeFd: true});
+        const bytes = stream.read_bytes(4096, null);
+        return ByteArray.fromGBytes(bytes);
+    }
+
+    it('can call a remote method that returns a Unix FD', function (done) {
+        const expectedBytes = ByteArray.fromString('some bytes');
+        proxy.fdOutRemote(expectedBytes, ([fdIndex], exc, outFdList) => {
+            expect(exc).toBeNull();
+            const bytes = readBytesFromFdSync(outFdList.get(fdIndex));
+            expect(bytes).toEqual(expectedBytes);
+            done();
+        });
+    });
+
+    it('can call an asynchronously implemented remote method that returns a Unix FD', function (done) {
+        const expectedBytes = ByteArray.fromString('some bytes');
+        proxy.fdOut2Remote(expectedBytes, ([fdIndex], exc, outFdList) => {
+            expect(exc).toBeNull();
+            const bytes = readBytesFromFdSync(outFdList.get(fdIndex));
+            expect(bytes).toEqual(expectedBytes);
+            done();
+        });
+    });
+
+    it('throws an exception when not passing a Gio.UnixFDList to a method that requires one', function () {
+        expect(() => proxy.fdInRemote(0, () => {})).toThrow();
+    });
+
+    it('throws an exception when passing a handle out of range of a Gio.UnixFDList', function () {
+        const fdList = new Gio.UnixFDList();
+        expect(() => proxy.fdInRemote(0, fdList, () => {})).toThrow();
+    });
 });
diff --git a/libgjs-private/gjs-util.c b/libgjs-private/gjs-util.c
index f0e941f8..ea691f56 100644
--- a/libgjs-private/gjs-util.c
+++ b/libgjs-private/gjs-util.c
@@ -21,8 +21,13 @@
  */
 
 #include <config.h>
+
+#include <errno.h>
+#include <fcntl.h>
 #include <string.h>
 
+#include <gio/gio.h>
+#include <glib-unix.h>
 #include <glib.h>
 #include <glib/gi18n.h>
 
@@ -108,3 +113,77 @@ gjs_param_spec_get_owner_type(GParamSpec *pspec)
 {
     return pspec->owner_type;
 }
+
+// Adapted from glnx_throw_errno_prefix()
+G_GNUC_PRINTF(2, 3)
+static gboolean throw_errno_prefix(GError** error, const char* fmt, ...) {
+    int errsv = errno;
+    char* old_msg;
+    GString* buf;
+
+    va_list args;
+
+    if (!error)
+        return FALSE;
+
+    va_start(args, fmt);
+
+    g_set_error_literal(error, G_IO_ERROR, g_io_error_from_errno(errsv),
+                        g_strerror(errsv));
+
+    old_msg = g_steal_pointer(&(*error)->message);
+    buf = g_string_new("");
+    g_string_append_vprintf(buf, fmt, args);
+    g_string_append(buf, ": ");
+    g_string_append(buf, old_msg);
+    g_free(old_msg);
+    (*error)->message = g_string_free(g_steal_pointer(&buf), FALSE);
+
+    va_end(args);
+
+    errno = errsv;
+    return FALSE;
+}
+
+/**
+ * gjs_open_bytes:
+ * @bytes: bytes to send to the pipe
+ * @error: Return location for a #GError, or %NULL
+ *
+ * Creates a pipe and sends @bytes to it, such that it is suitable for passing
+ * to g_subprocess_launcher_take_fd().
+ *
+ * Returns: file descriptor, or -1 on error
+ */
+int gjs_open_bytes(GBytes* bytes, GError** error) {
+    int pipefd[2], result;
+    size_t count;
+    const void* buf;
+    ssize_t bytes_written;
+
+    g_return_val_if_fail(bytes, -1);
+    g_return_val_if_fail(error == NULL || *error == NULL, -1);
+
+    if (!g_unix_open_pipe(pipefd, FD_CLOEXEC, error))
+        return -1;
+
+    buf = g_bytes_get_data(bytes, &count);
+
+    bytes_written = write(pipefd[1], buf, count);
+    if (bytes_written < 0) {
+        throw_errno_prefix(error, "write");
+        return -1;
+    }
+
+    if ((size_t)bytes_written != count)
+        g_warning("%s: %zd bytes sent, only %zu bytes written", __func__, count,
+                  bytes_written);
+
+    result = close(pipefd[1]);
+    if (result == -1) {
+        throw_errno_prefix(error, "close");
+        return -1;
+    }
+
+    return pipefd[0];
+}
diff --git a/libgjs-private/gjs-util.h b/libgjs-private/gjs-util.h
index ef9ed5e8..0a337987 100644
--- a/libgjs-private/gjs-util.h
+++ b/libgjs-private/gjs-util.h
@@ -59,6 +59,9 @@ GParamFlags gjs_param_spec_get_flags (GParamSpec *pspec);
 GType       gjs_param_spec_get_value_type (GParamSpec *pspec);
 GType       gjs_param_spec_get_owner_type (GParamSpec *pspec);
 
+/* For tests */
+int gjs_open_bytes(GBytes* bytes, GError** error);
+
 G_END_DECLS
 
 #endif
diff --git a/modules/overrides/Gio.js b/modules/overrides/Gio.js
index c20ea475..4a0ff00d 100644
--- a/modules/overrides/Gio.js
+++ b/modules/overrides/Gio.js
@@ -24,10 +24,59 @@ var Lang = imports.lang;
 var Signals = imports.signals;
 var Gio;
 
+// Ensures that a Gio.UnixFDList being passed into or out of a DBus method with
+// a parameter type that includes 'h' somewhere, actually has entries in it for
+// each of the indices being passed as an 'h' parameter.
+function _validateFDVariant(variant, fdList) {
+    switch (String.fromCharCode(variant.classify())) {
+    case 'b':
+    case 'y':
+    case 'n':
+    case 'q':
+    case 'i':
+    case 'u':
+    case 'x':
+    case 't':
+    case 'd':
+    case 'o':
+    case 'g':
+    case 's':
+        return;
+    case 'h': {
+        const val = variant.get_handle();
+        const numFds = fdList.get_length();
+        if (val >= numFds)
+            throw new Error(`handle ${val} is out of range of Gio.UnixFDList ` +
+                `containing ${numFds} FDs`);
+        return;
+    }
+    case 'v':
+        _validateFDVariant(variant.get_variant(), fdList);
+        return;
+    case 'm': {
+        let val = variant.get_maybe();
+        if (val)
+            _validateFDVariant(val, fdList);
+        return;
+    }
+    case 'a':
+    case '(':
+    case '{': {
+        let nElements = variant.n_children();
+        for (let ix = 0; ix < nElements; ix++)
+            _validateFDVariant(variant.get_child_value(ix), fdList);
+        return;
+    }
+    }
+
+    throw new Error('Assertion failure: this code should not be reached');
+}
+
 function _proxyInvoker(methodName, sync, inSignature, arg_array) {
     var replyFunc;
     var flags = 0;
     var cancellable = null;
+    let fdList = null;
 
     /* Convert arg_array to a *real* array */
     arg_array = Array.prototype.slice.call(arg_array);
@@ -37,7 +86,7 @@ function _proxyInvoker(methodName, sync, inSignature, arg_array) {
 
     var signatureLength = inSignature.length;
     var minNumberArgs = signatureLength;
-    var maxNumberArgs = signatureLength + 3;
+    var maxNumberArgs = signatureLength + 4;
 
     if (arg_array.length < minNumberArgs) {
         throw new Error("Not enough arguments passed for method: " + methodName +
@@ -45,7 +94,7 @@ function _proxyInvoker(methodName, sync, inSignature, arg_array) {
     } else if (arg_array.length > maxNumberArgs) {
         throw new Error(`Too many arguments passed for method ${methodName}. ` +
             `Maximum is ${maxNumberArgs} including one callback, ` +
-            'cancellable, and/or flags');
+            'Gio.Cancellable, Gio.UnixFDList, and/or flags');
     }
 
     while (arg_array.length > signatureLength) {
@@ -57,41 +106,44 @@ function _proxyInvoker(methodName, sync, inSignature, arg_array) {
             flags = arg;
         } else if (arg instanceof Gio.Cancellable) {
             cancellable = arg;
+        } else if (arg instanceof Gio.UnixFDList) {
+            fdList = arg;
         } else {
-            throw new Error("Argument " + argNum + " of method " + methodName +
-                            " is " + typeof(arg) + ". It should be a callback, flags or a Gio.Cancellable");
+            throw new Error(`Argument ${argNum} of method ${methodName} is ` +
+                `${typeof arg}. It should be a callback, flags, ` +
+                'Gio.UnixFDList, or a Gio.Cancellable');
         }
     }
 
-    var inVariant = new GLib.Variant('(' + inSignature.join('') + ')', arg_array);
+    const inTypeString = `(${inSignature.join('')})`;
+    const inVariant = new GLib.Variant(inTypeString, arg_array);
+    if (inTypeString.includes('h')) {
+        if (!fdList)
+            throw new Error(`Method ${methodName} with input type containing ` +
+                '"h" must have a Gio.UnixFDList as an argument');
+        _validateFDVariant(inVariant, fdList);
+    }
 
     var asyncCallback = function (proxy, result) {
-        var outVariant = null, succeeded = false;
         try {
-            outVariant = proxy.call_finish(result);
-            succeeded = true;
+            const [outVariant, outFdList] =
+                proxy.call_with_unix_fd_list_finish(result);
+            replyFunc(outVariant.deep_unpack(), null, outFdList);
         } catch (e) {
-            replyFunc([], e);
+            replyFunc([], e, null);
         }
-
-        if (succeeded)
-            replyFunc(outVariant.deep_unpack(), null);
     };
 
     if (sync) {
-        return this.call_sync(methodName,
-                              inVariant,
-                              flags,
-                              -1,
-                              cancellable).deep_unpack();
-    } else {
-        return this.call(methodName,
-                         inVariant,
-                         flags,
-                         -1,
-                         cancellable,
-                         asyncCallback);
+        const [outVariant, outFdList] = this.call_with_unix_fd_list_sync(
+            methodName, inVariant, flags, -1, fdList, cancellable);
+        if (fdList)
+            return [outVariant.deep_unpack(), outFdList];
+        return outVariant.deep_unpack();
     }
+
+    return this.call_with_unix_fd_list(methodName, inVariant, flags, -1, fdList,
+        cancellable, asyncCallback);
 }
 
 function _logReply(result, exc) {
@@ -253,7 +305,8 @@ function _handleMethodCall(info, impl, method_name, parameters, invocation) {
     if (this[method_name]) {
         let retval;
         try {
-            retval = this[method_name].apply(this, parameters.deep_unpack());
+            const fdList = invocation.get_message().get_unix_fd_list();
+            retval = this[method_name](...parameters.deep_unpack(), fdList);
         } catch (e) {
             if (e instanceof GLib.Error) {
                 invocation.return_gerror(e);
@@ -273,26 +326,31 @@ function _handleMethodCall(info, impl, method_name, parameters, invocation) {
             retval = new GLib.Variant('()', []);
         }
         try {
+            let outFdList = null;
             if (!(retval instanceof GLib.Variant)) {
                 // attempt packing according to out signature
                 let methodInfo = info.lookup_method(method_name);
                 let outArgs = methodInfo.out_args;
                 let outSignature = _makeOutSignature(outArgs);
-                if (outArgs.length == 1) {
+                if (outSignature.includes('h') &&
+                    retval[retval.length - 1] instanceof Gio.UnixFDList) {
+                    outFdList = retval.pop();
+                } else if (outArgs.length == 1) {
                     // if one arg, we don't require the handler wrapping it
                     // into an Array
                     retval = [retval];
                 }
                 retval = new GLib.Variant(outSignature, retval);
             }
-            invocation.return_value(retval);
+            invocation.return_value_with_unix_fd_list(retval, outFdList);
         } catch(e) {
             // if we don't do this, the other side will never see a reply
             invocation.return_dbus_error('org.gnome.gjs.JSError.ValueError',
                                          "Service implementation returned an incorrect value type");
         }
     } else if (this[method_name + 'Async']) {
-        this[method_name + 'Async'](parameters.deep_unpack(), invocation);
+        const fdList = invocation.get_message().get_unix_fd_list();
+        this[`${method_name}Async`](parameters.deep_unpack(), invocation, fdList);
     } else {
         log('Missing handler for DBus method ' + method_name);
         invocation.return_gerror(new Gio.DBusError({ code: Gio.DBusError.UNKNOWN_METHOD,