Opened 7 years ago

Closed 7 years ago

#17 closed bug (fixed)

Non-hosted domains in virtual domains with the Files backend

Reported by: sbocahu Owned by: vcaron
Priority: major Component: Bearmail
Keywords: Cc:

Description

There is an annoying bug in the Files backend of Bearmail: bearmail/lib/BearMail/Backend/Files.pm

Items such as:

postmaster@ourdomain.com::postmaster@wedonthostthatdomain.com

result in having both ourdomain.com and wedonthostthatdomain.com as postfix virtual domains (postfix configuration file written by bearmail-update)

This is caused by the calls to $records{$_}->{password} in the _sort_mailmap() method (lines 305-309): if the item of "$_" key does not exists, it creates one with no value... And this is not logical to me.

(if you don't understand this very well, let's look at this:

#!/usr/bin/env perl

use strict;
use warnings;
use Data::Dumper;

my %hash;

print "undef - not printed" if defined($hash{'test'}); 
print "undef - not printed" if defined($hash{'test'}->{'test'}); 
print "empty but defined, now" if defined($hash{'test'}); 

So maybe the solution is to test if the item is defined, then, and only then, test for its subhash values...

Change History (6)

comment:1 Changed 7 years ago by zecrazytux

(In [498]) Fixed the non-hosted-domains-in-postfix-conf bug :), see #17

comment:2 Changed 7 years ago by sbocahu

Altough I still don't consider perl's defined() effect on the hash very logical... The bug is now fixed.

_But_, it does higlight a problem I've never considered until now:

the authentication mechanism we have imagined is based on posmasters emails && password (same if they are aliases: we look for aliases emails && password).

How do we manage the fact that some postmasters may host their accounts _elsewhere_ ?

comment:3 Changed 7 years ago by vcaron

I'm looking at quickly releasing the [498] bugfix ASAP, it's hitting us on Bearstech production servers.

So maybe the solution is to test if the item is defined, then, and only then, test for its subhash values...

The beauty and nastiness of dynamic typing. Accessing $hash{'test'}->{'test'} implicitly tells Perl that $hash{'test']} is a hashref. It might have barked that you were looking up an inexistent hash but "DWIM" does not mean "consistent" :). BUT the coding bug is that you should not even try to lookup a hashref if it's not defined in the first place; said otherwise, you have to test your data structure from the outter to inner element, layer by layer (like... an onion!)

How do we manage the fact that some postmasters may host their accounts _elsewhere_ ?

It doesn't matter, they'll still need to use some postmaster@… identifier.

But we need to allow "alias+password" entries in mailmap, something which was initially rejected because it had no use case. Now it does :).

comment:4 Changed 7 years ago by vcaron

OK, I'm releasing 0.3.3 as of trunk without the [500] which has maybe some issues (see #10). That is 0.3.3 is frozen at r499.

comment:5 Changed 7 years ago by zerodeux

(In [505]) Releasing bearmail 0.3.3, mainly fixes non-hosted-domain-loophole bug, see #17

comment:6 Changed 7 years ago by vcaron

  • Resolution set to fixed
  • Status changed from new to closed

0.3.3 deb released + 0.3.3 released blog entry added

Also deployed on our own servers, works better now.

Note: See TracTickets for help on using tickets.