gdbus-codegen: Add call_flags and timeout_msec args

Currently the code generated by gdbus-codegen uses
G_DBUS_CALL_FLAGS_NONE in its D-Bus calls, which occur for each method
defined by the input XML, and for proxy_set_property functions. This
means that if the daemon which implements the methods checks for
G_DBUS_FLAGS_ALLOW_INTERACTIVE_AUTHORIZATION and only does interactive
authorization if that flag is present, users of the generated code have
no way to cause the daemon to use interactive authorization (e.g. polkit
dialogs).

If we simply changed the generated code to always use
G_DBUS_FLAGS_ALLOW_INTERACTIVE_AUTHORIZATION, its users would have no
way to disallow interactive authorization (except for manually calling
the D-Bus method themselves).

So instead, this commit adds a GDBusCallFlags argument to method call
functions. Since this is an API break which will require changes in
projects using gdbus-codegen code, the change is conditional on the
command line argument --glib-min-version having the value 2.64 or
higher.

The impetus for this change is that I'm changing accountsservice to
properly respect G_DBUS_FLAGS_ALLOW_INTERACTIVE_AUTHORIZATION, and
libaccountsservice uses generated code for D-Bus method calls. So
these changes will allow libaccountsservice to continue allowing
interactive authorization, and avoid breaking any users of it which
expect that. See
https://gitlab.freedesktop.org/accountsservice/accountsservice/merge_requests/46

It might make sense to also let GDBusCallFlags be specified for property
set operations, but that is not needed in the case of accountsservice,
and would require significant work and breaking API in multiple places.

Similarly, the generated code currently hard codes -1 as the timeout
value when calling g_dbus_proxy_call*(). Add a timeout_msec argument so
the user of the generated code can specify the timeout as well.

Also, test this new API. In gio/tests/codegen.py we test that the new
arguments are generated if and only of --glib-min-version is used with a
value greater than or equal to 2.64, and in gio/tests/meson.build we
test that the generated code with the new API can be linked against.

The test_unix_fd_list() test also needed modification to continue
working now that we're using gdbus-test-codegen.c with code generated
with --glib-min-version=2.64 in one test.

Finally, update the docs for gdbus-codegen to explain the effect of
using --glib-min-version 2.64, both from this commit and from
"gdbus-codegen: Emit GUnixFDLists if an arg has type `h` w/
min-version".
This commit is contained in:
Matthew Leeds 2019-11-25 11:51:13 -08:00
parent 6f34e84002
commit 2a605f6e15
5 changed files with 200 additions and 11 deletions

View File

@ -435,6 +435,12 @@ gdbus-codegen --c-namespace MyApp \
upwards, as that is when <command>gdbus-codegen</command> was first
released.
</para>
<para>
Note that some version parameters introduce incompatible changes: all callers
of the generated code might need to be updated, and if the generated code is part of
a library's API or ABI, then increasing the version parameter can result in an API
or ABI break.
</para>
<para>
The version number must be of the form
<literal><replaceable>MAJOR</replaceable>.<replaceable>MINOR</replaceable>.<replaceable>MICRO</replaceable></literal>,
@ -442,6 +448,15 @@ gdbus-codegen --c-namespace MyApp \
<replaceable>MICRO</replaceable> are optional. The version number may not be smaller
than <literal>2.30</literal>.
</para>
<para>
If the version number is <literal>2.64</literal> or greater, the generated code will
have the following features: (1) If a method has <literal>h</literal> (file
descriptor) parameter(s), a <type>GUnixFDList</type> parameter will exist in the
generated code for it (whereas previously the annotation
<literal>org.gtk.GDBus.C.UnixFD</literal> was required), and (2) Method call functions will
have two additional arguments to allow the user to specify <type>GDBusCallFlags</type>
and a timeout value, as is possible when using <function>g_dbus_proxy_call()</function>.
</para>
</listitem>
</varlistentry>

View File

@ -73,6 +73,10 @@ class HeaderCodeGenerator:
self.glib_min_version = glib_min_version
self.outfile = outfile
self.glib_min_version_is_2_64 = (glib_min_version[0] > 2 or
(glib_min_version[0] == 2 and
glib_min_version[1] >= 64))
# ----------------------------------------------------------------------------------------------------
def generate_header_preamble(self):
@ -222,6 +226,9 @@ class HeaderCodeGenerator:
' %s *proxy'%(i.name_lower, m.name_lower, i.camel_name))
for a in m.in_args:
self.outfile.write(',\n %sarg_%s'%(a.ctype_in, a.name))
if self.glib_min_version_is_2_64:
self.outfile.write(',\n GDBusCallFlags call_flags'
',\n gint timeout_msec')
if m.unix_fd:
self.outfile.write(',\n GUnixFDList *fd_list')
self.outfile.write(',\n'
@ -249,6 +256,9 @@ class HeaderCodeGenerator:
' %s *proxy'%(i.name_lower, m.name_lower, i.camel_name))
for a in m.in_args:
self.outfile.write(',\n %sarg_%s'%(a.ctype_in, a.name))
if self.glib_min_version_is_2_64:
self.outfile.write(',\n GDBusCallFlags call_flags'
',\n gint timeout_msec')
if m.unix_fd:
self.outfile.write(',\n GUnixFDList *fd_list')
for a in m.out_args:
@ -916,6 +926,10 @@ class CodeGenerator:
self.glib_min_version = glib_min_version
self.outfile = outfile
self.glib_min_version_is_2_64 = (glib_min_version[0] > 2 or
(glib_min_version[0] == 2 and
glib_min_version[1] >= 64))
# ----------------------------------------------------------------------------------------------------
def generate_body_preamble(self):
@ -1666,6 +1680,11 @@ class CodeGenerator:
%(i.name_lower, m.name_lower, i.camel_name))
for a in m.in_args:
self.outfile.write(' * @arg_%s: Argument to pass with the method invocation.\n'%(a.name))
if self.glib_min_version_is_2_64:
self.outfile.write(' * @call_flags: Flags from the #GDBusCallFlags enumeration. If you want to allow interactive\n'
' authorization be sure to set %G_DBUS_CALL_FLAGS_ALLOW_INTERACTIVE_AUTHORIZATION.\n'
' * @timeout_msec: The timeout in milliseconds (with %G_MAXINT meaning "infinite") or\n'
' -1 to use the proxy default timeout.\n')
if m.unix_fd:
self.outfile.write(' * @fd_list: (nullable): A #GUnixFDList or %NULL.\n')
self.outfile.write(self.docbook_gen.expand(
@ -1685,6 +1704,9 @@ class CodeGenerator:
' %s *proxy'%(i.name_lower, m.name_lower, i.camel_name))
for a in m.in_args:
self.outfile.write(',\n %sarg_%s'%(a.ctype_in, a.name))
if self.glib_min_version_is_2_64:
self.outfile.write(',\n GDBusCallFlags call_flags'
',\n gint timeout_msec')
if m.unix_fd:
self.outfile.write(',\n GUnixFDList *fd_list')
self.outfile.write(',\n'
@ -1703,9 +1725,13 @@ class CodeGenerator:
self.outfile.write(')"')
for a in m.in_args:
self.outfile.write(',\n arg_%s'%(a.name))
self.outfile.write('),\n'
' G_DBUS_CALL_FLAGS_NONE,\n'
' -1,\n')
self.outfile.write('),\n')
if self.glib_min_version_is_2_64:
self.outfile.write(' call_flags,\n'
' timeout_msec,\n')
else:
self.outfile.write(' G_DBUS_CALL_FLAGS_NONE,\n'
' -1,\n')
if m.unix_fd:
self.outfile.write(' fd_list,\n')
self.outfile.write(' cancellable,\n'
@ -1771,6 +1797,11 @@ class CodeGenerator:
%(i.name_lower, m.name_lower, i.camel_name))
for a in m.in_args:
self.outfile.write(' * @arg_%s: Argument to pass with the method invocation.\n'%(a.name))
if self.glib_min_version_is_2_64:
self.outfile.write(' * @call_flags: Flags from the #GDBusCallFlags enumeration. If you want to allow interactive\n'
' authorization be sure to set %G_DBUS_CALL_FLAGS_ALLOW_INTERACTIVE_AUTHORIZATION.\n'
' * @timeout_msec: The timeout in milliseconds (with %G_MAXINT meaning "infinite") or\n'
' -1 to use the proxy default timeout.\n')
if m.unix_fd:
self.outfile.write(' * @fd_list: (nullable): A #GUnixFDList or %NULL.\n')
for a in m.out_args:
@ -1793,6 +1824,9 @@ class CodeGenerator:
' %s *proxy'%(i.name_lower, m.name_lower, i.camel_name))
for a in m.in_args:
self.outfile.write(',\n %sarg_%s'%(a.ctype_in, a.name))
if self.glib_min_version_is_2_64:
self.outfile.write(',\n GDBusCallFlags call_flags'
',\n gint timeout_msec')
if m.unix_fd:
self.outfile.write(',\n GUnixFDList *fd_list')
for a in m.out_args:
@ -1815,9 +1849,13 @@ class CodeGenerator:
self.outfile.write(')"')
for a in m.in_args:
self.outfile.write(',\n arg_%s'%(a.name))
self.outfile.write('),\n'
' G_DBUS_CALL_FLAGS_NONE,\n'
' -1,\n')
self.outfile.write('),\n')
if self.glib_min_version_is_2_64:
self.outfile.write(' call_flags,\n'
' timeout_msec,\n')
else:
self.outfile.write(' G_DBUS_CALL_FLAGS_NONE,\n'
' -1,\n')
if m.unix_fd:
self.outfile.write(' fd_list,\n'
' out_fd_list,\n')

View File

@ -447,6 +447,44 @@ G_END_DECLS
self.assertEqual('', result.err)
self.assertEqual(result.out.strip().count('GUnixFDList'), 18)
def test_call_flags_and_timeout_method_args(self):
"""Test that generated method call functions have @call_flags and
@timeout_msec args if and only if GLib >= 2.64.
"""
interface_xml = '''
<node>
<interface name="org.project.UsefulInterface">
<method name="UsefulMethod"/>
</interface>
</node>'''
# Try without specifying --glib-min-version.
result = self.runCodegenWithInterface(interface_xml,
'--output', '/dev/stdout',
'--header')
self.assertEqual('', result.err)
self.assertEqual(result.out.strip().count('GDBusCallFlags call_flags,'), 0)
self.assertEqual(result.out.strip().count('gint timeout_msec,'), 0)
# Specify an old --glib-min-version.
result = self.runCodegenWithInterface(interface_xml,
'--output', '/dev/stdout',
'--header',
'--glib-min-version', '2.32')
self.assertEqual('', result.err)
self.assertEqual(result.out.strip().count('GDBusCallFlags call_flags,'), 0)
self.assertEqual(result.out.strip().count('gint timeout_msec,'), 0)
# Specify a --glib-min-version ≥ 2.64. The two arguments should be
# present for both the async and sync method call functions.
result = self.runCodegenWithInterface(interface_xml,
'--output', '/dev/stdout',
'--header',
'--glib-min-version', '2.64')
self.assertEqual('', result.err)
self.assertEqual(result.out.strip().count('GDBusCallFlags call_flags,'), 2)
self.assertEqual(result.out.strip().count('gint timeout_msec,'), 2)
if __name__ == '__main__':
unittest.main(testRunner=taptestrunner.TAPTestRunner())

View File

@ -25,7 +25,12 @@
#include "gdbus-tests.h"
#if GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_64
#include "gdbus-test-codegen-generated-min-version-2-64.h"
#else
#include "gdbus-test-codegen-generated.h"
#endif
#include "gdbus-test-codegen-generated-interface-info.h"
/* ---------------------------------------------------------------------------------------------------- */
@ -878,6 +883,10 @@ check_bar_proxy (FooiGenBar *proxy,
"/a/path",
"asig",
"bytestring\xff",
#if GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_64
G_DBUS_CALL_FLAGS_NONE,
-1,
#endif
&ret_val_byte,
&ret_val_boolean,
&ret_val_int16,
@ -913,6 +922,10 @@ check_bar_proxy (FooiGenBar *proxy,
array_of_objpaths,
array_of_signatures,
array_of_bytestrings,
#if GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_64
G_DBUS_CALL_FLAGS_NONE,
-1,
#endif
&ret_array_of_strings,
&ret_array_of_objpaths,
&ret_array_of_signatures,
@ -946,7 +959,11 @@ check_bar_proxy (FooiGenBar *proxy,
* unimplemented methods.
*/
error = NULL;
#if GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_64
ret = foo_igen_bar_call_unimplemented_method_sync (proxy, G_DBUS_CALL_FLAGS_NONE, -1, NULL /* GCancellable */, &error);
#else
ret = foo_igen_bar_call_unimplemented_method_sync (proxy, NULL /* GCancellable */, &error);
#endif
g_assert_error (error, G_DBUS_ERROR, G_DBUS_ERROR_UNKNOWN_METHOD);
g_error_free (error);
error = NULL;
@ -957,7 +974,11 @@ check_bar_proxy (FooiGenBar *proxy,
G_CALLBACK (on_test_signal),
data);
error = NULL;
#if GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_64
ret = foo_igen_bar_call_request_signal_emission_sync (proxy, 0, G_DBUS_CALL_FLAGS_NONE, -1, NULL, &error);
#else
ret = foo_igen_bar_call_request_signal_emission_sync (proxy, 0, NULL, &error);
#endif
g_assert_no_error (error);
g_assert (ret);
@ -1011,7 +1032,11 @@ check_bar_proxy (FooiGenBar *proxy,
G_CALLBACK (on_g_properties_changed),
data);
error = NULL;
#if GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_64
ret = foo_igen_bar_call_request_multi_property_mods_sync (proxy, G_DBUS_CALL_FLAGS_NONE, -1, NULL, &error);
#else
ret = foo_igen_bar_call_request_multi_property_mods_sync (proxy, NULL, &error);
#endif
g_assert_no_error (error);
g_assert (ret);
g_main_loop_run (thread_loop);
@ -1077,6 +1102,10 @@ check_bar_proxy (FooiGenBar *proxy,
*/
error = NULL;
foo_igen_bar_call_property_cancellation (proxy,
#if GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_64
G_DBUS_CALL_FLAGS_NONE,
-1,
#endif
NULL, /* GCancellable */
(GAsyncReadyCallback) on_property_cancellation_cb,
data);
@ -1159,6 +1188,10 @@ check_bat_proxy (FooiGenBat *proxy,
g_variant_new_string ("a string"),
g_variant_new_bytestring ("a bytestring\xff"),
g_variant_new ("(i)", 4200),
#if GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_64
G_DBUS_CALL_FLAGS_NONE,
-1,
#endif
&ret_i,
&ret_s,
&ret_ay,
@ -1191,18 +1224,30 @@ check_authorize_proxy (FooiGenAuthorize *proxy,
/* Check that g-authorize-method works as intended */
error = NULL;
#if GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_64
ret = foo_igen_authorize_call_check_not_authorized_sync (proxy, G_DBUS_CALL_FLAGS_NONE, -1, NULL, &error);
#else
ret = foo_igen_authorize_call_check_not_authorized_sync (proxy, NULL, &error);
#endif
g_assert_error (error, G_IO_ERROR, G_IO_ERROR_PERMISSION_DENIED);
g_error_free (error);
g_assert (!ret);
error = NULL;
#if GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_64
ret = foo_igen_authorize_call_check_authorized_sync (proxy, G_DBUS_CALL_FLAGS_NONE, -1, NULL, &error);
#else
ret = foo_igen_authorize_call_check_authorized_sync (proxy, NULL, &error);
#endif
g_assert_no_error (error);
g_assert (ret);
error = NULL;
#if GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_64
ret = foo_igen_authorize_call_check_not_authorized_from_object_sync (proxy, G_DBUS_CALL_FLAGS_NONE, -1, NULL, &error);
#else
ret = foo_igen_authorize_call_check_not_authorized_from_object_sync (proxy, NULL, &error);
#endif
g_assert_error (error, G_IO_ERROR, G_IO_ERROR_PENDING);
g_error_free (error);
g_assert (!ret);
@ -1219,7 +1264,11 @@ get_self_via_proxy (FooiGenMethodThreads *proxy_1)
gpointer self;
error = NULL;
#if GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_64
ret = foo_igen_method_threads_call_get_self_sync (proxy_1, G_DBUS_CALL_FLAGS_NONE, -1, &self_str, NULL, &error);
#else
ret = foo_igen_method_threads_call_get_self_sync (proxy_1, &self_str, NULL, &error);
#endif
g_assert_no_error (error);
g_assert (ret);
@ -2599,6 +2648,28 @@ handle_hello_fd (FooiGenFDPassing *object,
return TRUE;
}
#if GLIB_VERSION_MIN_REQUIRED >= GLIB_VERSION_2_64
static gboolean
handle_no_annotation (FooiGenFDPassing *object,
GDBusMethodInvocation *invocation,
GUnixFDList *fd_list,
GVariant *arg_greeting,
const gchar *arg_greeting_locale)
{
foo_igen_fdpassing_complete_no_annotation (object, invocation, fd_list, arg_greeting, arg_greeting_locale);
return TRUE;
}
static gboolean
handle_no_annotation_nested (FooiGenFDPassing *object,
GDBusMethodInvocation *invocation,
GUnixFDList *fd_list,
GVariant *arg_files)
{
foo_igen_fdpassing_complete_no_annotation_nested (object, invocation, fd_list);
return TRUE;
}
#else
static gboolean
handle_no_annotation (FooiGenFDPassing *object,
GDBusMethodInvocation *invocation,
@ -2617,9 +2688,12 @@ handle_no_annotation_nested (FooiGenFDPassing *object,
foo_igen_fdpassing_complete_no_annotation_nested (object, invocation);
return TRUE;
}
#endif
/* Test that generated code for methods includes GUnixFDList arguments if and
* only if the method is explicitly annotated as C.UnixFD.
/* Test that generated code for methods includes GUnixFDList arguments
* unconditionally if the method is explicitly annotated as C.UnixFD, and only
* emits GUnixFDList arguments when there's merely an 'h' parameter if
* --glib-min-version=2.64 or greater.
*/
static void
test_unix_fd_list (void)
@ -2631,10 +2705,12 @@ test_unix_fd_list (void)
/* This method is explicitly annotated. */
iface.handle_hello_fd = handle_hello_fd;
/* This one is not annotated; even though it's got an in and out 'h' parameter, for
* backwards compatibility we cannot emit GUnixFDList arguments.
/* This one is not annotated; even though it's got an in and out 'h'
* parameter, for backwards compatibility we cannot emit GUnixFDList
* arguments unless --glib-min-version >= 2.64 was used.
*/
iface.handle_no_annotation = handle_no_annotation;
/* This method has an 'h' inside a complex type. */
iface.handle_no_annotation_nested = handle_no_annotation_nested;

View File

@ -230,7 +230,23 @@ if host_machine.system() != 'windows'
'--generate-docbook', 'gdbus-test-codegen-generated-doc',
annotate_args,
'@INPUT@'])
# Generate gdbus-test-codegen-generated-min-version-2-64.{c,h}
gdbus_test_codegen_generated_min_version_2_64 = custom_target('gdbus-test-codegen-generated-min-version-2-64',
input : ['test-codegen.xml'],
output : ['gdbus-test-codegen-generated-min-version-2-64.h',
'gdbus-test-codegen-generated-min-version-2-64.c'],
depend_files : gdbus_codegen_built_files,
command : [python, gdbus_codegen,
'--glib-min-version', '2.64',
'--interface-prefix', 'org.project.',
'--output-directory', '@OUTDIR@',
'--generate-c-code', 'gdbus-test-codegen-generated-min-version-2-64',
'--c-generate-object-manager',
'--c-generate-autocleanup', 'all',
'--c-namespace', 'Foo_iGen',
'--generate-docbook', 'gdbus-test-codegen-generated-doc',
annotate_args,
'@INPUT@'])
gdbus_test_codegen_generated_interface_info = [
custom_target('gdbus-test-codegen-generated-interface-info-h',
input : ['test-codegen.xml'],
@ -282,6 +298,7 @@ if host_machine.system() != 'windows'
'gdbus-proxy-well-known-name' : {'extra_sources' : extra_sources},
'gdbus-test-codegen' : {
'extra_sources' : [extra_sources, gdbus_test_codegen_generated, gdbus_test_codegen_generated_interface_info],
'c_args' : ['-DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_32'],
},
'gdbus-threading' : {
'extra_sources' : extra_sources,
@ -300,6 +317,11 @@ if host_machine.system() != 'windows'
'c_args' : ['-DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_36',
'-DGLIB_VERSION_MAX_ALLOWED=GLIB_VERSION_2_36'],
},
'gdbus-test-codegen-min-version-2-64' : {
'source' : 'gdbus-test-codegen.c',
'extra_sources' : [extra_sources, gdbus_test_codegen_generated_min_version_2_64, gdbus_test_codegen_generated_interface_info],
'c_args' : ['-DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_64'],
},
'gapplication' : {'extra_sources' : extra_sources},
}