GNOME Bugzilla – Bug 656443
Make GTlsInteraction ask_password cancellable
Last modified: 2011-08-26 05:47:09 UTC
After discussion in bug #656343, it's likely that the various TLS interactions will need to be cancellable. This means changing the virtual ask_password* methods in GTlsInteraction. Since these just got merged, and haven't been in a stable release yet, we should be able to change them.
Created attachment 193758 [details] [review] Make GTlsInteraction virtual methods cancellable * Add cancellable argument to g_tls_interaction_ask_password and g_tls_interaction_ask_password_async. * This is API breakage, but this API has not yet been released in a stable release (and very unlikely used yet). * Since we're breaking unreleased API, expand amount of padding on GTlsInteractionClass because we're going to need it.
Branch here: http://cgit.collabora.com/git/user/stefw/glib.git/log/?h=cancellable-interaction Tested with gtls-frob tool, and tls-pkcs11 glib-networking branch. Found here: http://cgit.collabora.com/git/user/stefw/gtls-frob.git/log/ http://cgit.collabora.com/git/user/stefw/glib-networking.git/log?h=tls-pkcs11
Dan or David, could one of you review this, so it can get in before the next release? I believe this is necessary to support proper cancellation on GTlsConnection handshakes.
Looks good to me. One more thought, shouldn't g_tls_interaction_ask_password() or g_tls_interaction_ask_password_finish() take a GError? It seems reasonable to be able to return an error if there is e.g. no way to ask the user for the password (not in a desktop session or no controlling tty) ... or to convey that the user canceled the auth dialog.
I guess so. I was thinking it would return NULL if the user cancelled, and I imagined that errors would just act like a user cancelling. But I don't mind making it so that implementations can return more nuanced errors.
(In reply to comment #5) > I guess so. I was thinking it would return NULL if the user cancelled, and I > imagined that errors would just act like a user cancelling. But I don't mind > making it so that implementations can return more nuanced errors. I found that I needed that in PolicyKit-using applications - for example, udisks2 has three errors for this, see the _NOT_AUTHORIZED errors http://people.freedesktop.org/~david/udisks2-20110719/udisks2-UDisksError.html#UDisksError
Created attachment 194598 [details] [review] David, thanks for the review. Added error arguments to the ask_password method. Is this ready to merge now? Make GTlsInteraction virtual methods cancellable * Add cancellable argument to g_tls_interaction_ask_password and g_tls_interaction_ask_password_async. * This is API breakage, but this API has not yet been released in a stable release (and very unlikely used yet). * Since we're breaking unreleased API, expand amount of padding on GTlsInteractionClass because we're going to need it.
Comment on attachment 194598 [details] [review] David, thanks for the review. Added error arguments to the ask_password looks good, except: >+ * @cancellable: (allow-none): a #GCancellable cancellation object >+ * @error: (allow-none): an optional location to place an error on failure you don't need to specify (allow-none) for either of those
Made that change and merged. Thanks for looking it over.