Skip to content

Commit

Permalink
acl: Fix return value of acl_attribute_get_acl
Browse files Browse the repository at this point in the history 
If matching acl entry is not found, it must return 0
and not 1 because it did not find anything.

Fixes dsync: Panic: file mailbox-attribute.c: line 362 (mailbox_attribute_get_stream): assertion failed: (value_r->value != NULL || value_r->value_stream != NULL)

Broken in 37c72fa

Found by @dl8bh
  • Loading branch information
@cmouse @sirainen
cmouse authored and sirainen committed Jun 29, 2018
1 parent bc280b1 commit 4ff4bd0
Showing 1 changed file with 8 additions and 2 deletions.
10 changes: 8 additions & 2 deletions src/plugins/acl/acl-attributes.c
View file
Expand Up @@ -60,7 +60,7 @@ static int acl_attribute_get_acl(struct mailbox *box, const char *key,
struct acl_object_list_iter *iter;
struct acl_rights rights, wanted_rights;
const char *id;
int ret;
int ret = 0;

i_zero(value_r);

Expand Down Expand Up @@ -88,11 +88,17 @@ static int acl_attribute_get_acl(struct mailbox *box, const char *key,
rights.id_type == wanted_rights.id_type &&
null_strcmp(rights.identifier, wanted_rights.identifier) == 0) {
value_r->value = acl_rights_export(&rights);
ret = 1;
break;
}
}
if ((ret = acl_object_list_deinit(&iter)) < 0)
/* the return value here cannot be used, because this function
needs to return whether it actually matched something
or not */
if (acl_object_list_deinit(&iter) < 0) {
mail_storage_set_internal_error(box->storage);
ret = -1;
}
return ret;
}

Expand Down

3 comments on commit 4ff4bd0

@dl8bh
Copy link

@dl8bh dl8bh commented on 4ff4bd0 Jul 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did not fix my problem, if this patch was meant to be included in 2.3.2 (iirc it turned out in our IRC debugging session, that acl containing "@" in usernames are not handled correctly):

root@mail-1:~ # dovecot --version
2.3.2 (582970113)

root@mail-1:~ # doveadm -Dv backup -P -n INBOX -u listen@example.com mdbox:/usr/local/mail-snapshots/example.com/listen/mdbox
[...]
dsync(listen@example.com): Debug: Namespace : Using permissions from /usr/local/mail-snapshots/example.com/listen/mdbox: mode=0700 gid=default
dsync(listen@example.com): Debug: Namespace : Using permissions from /usr/local/mail/example.com/listen/mdbox: mode=0700 gid=default
dsync(listen@example.com): Debug: acl vfile: reading file /usr/local/mail/example.com/listen/mdbox/mailboxes/INBOX/dbox-Mails/dovecot-acl
dsync(listen@example.com): Debug: acl vfile: reading file /usr/local/mail-snapshots/example.com/listen/mdbox/mailboxes/INBOX/dbox-Mails/dovecot-acl
dsync(listen@example.com): Debug: brain S: Import INBOX: Import attribute vendor/vendor.dovecot/pvt/acl/user=privat@example.com: Unchanged value
dsync(listen@example.com): Panic: file mailbox-attribute.c: line 360 (mailbox_attribute_get_stream): assertion failed: (value_r->value != NULL || value_r->value_stream != NULL)
Abort

root@mail-1:~ # cat /usr/local/mail/example.com/listen/mdbox/mailboxes/INBOX/dbox-Mails/dovecot-acl 
user=privat@example.com eilprwts

@sirainen
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It didn't make it into v2.3.2 QA, so it'll be in v2.3.3.

@dl8bh
Copy link

@dl8bh dl8bh commented on 4ff4bd0 Jul 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized this myself minutes after posting this, but thanks for answering anyways :-)

Please sign in to comment.