[Bug 27021] crash when calling CreateAccount with a malformed property

[bugzilla-daemon at freedesktop.org]

http://bugs.freedesktop.org/show_bug.cgi?id=27021





--- Comment #6 from Tomeu Vizoso <tomeu at sugarlabs.org>  2010-03-26 01:56:17 PST ---
(In reply to comment #4)
> Can you reproduce this crash in a regression test by attempting to set a
> nonexistent parameter name over D-Bus, for instance?

A non-existent property name makes it crash, yes:

diff --git a/test/twisted/account-manager/create-with-properties.py
b/test/twisted/account-manager/create-with-properties.py
index bfd5b2b..3849524 100644
--- a/test/twisted/account-manager/create-with-properties.py
+++ b/test/twisted/account-manager/create-with-properties.py
@@ -71,6 +71,7 @@ def test(q, bus, mc):

     creation_properties = dbus.Dictionary({
         cs.ACCOUNT + '.Enabled': True,
+        cs.ACCOUNT + '.NonExistent': 'foo',
         cs.ACCOUNT + '.AutomaticPresence': dbus.Struct((
             dbus.UInt32(cs.PRESENCE_TYPE_BUSY),
             'busy', 'Exploding'), signature='uss'),


What would be better, a new test in its own module, or one more test inside
account-manager/create-with-properties.py ?

> (To be honest, your stack trace looks as though there's been a missing NUL
> termination further down the stack, so you're fixing symptoms rather than
> causes. However, if the same symptom can be caused in a valid way, there's no
> harm in fixing *that* bug.)

I'm confused, AFAICS the crash is caused because when the parameters are valid
but the properties are invalid, the code incorrectly tries to propagate the
params error, which doesn't exist.

> This patch is rather less obvious than it could be... it might well be *valid*,
> but I think it'd be considerably clearer if expressed like this:
> 
> > refactor the
> > function so that cad->ok and cad->error are always set at the same time
> 
> Something like this:
> 
> cad->ok = TRUE;
> 
> if (set_error != NULL)
> {
>     cad->ok = FALSE;
>     /* moved from [A] */
>     g_set_error (&cad->error, blah blah blah, set_error->message);
> }
> 
> if (cad->ok && cad->properties != NULL)
> {
>     cad->ok = ... as before ... (&cad->error);
> }
> 
> if (cad->ok)
> {
>     ... as before ...
> }
> else
> {
>     /* [A] */
>     complete_account_creation_finish (account, TRUE, cad);
> }
> 
> (The comments mentioning [A] wouldn't be committed, obviously.)

Sounds good, thanks.


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.