smtp_default_domain resurrects addresses with ignored domains

[Stephen Compall <stephen.compall@…>]

The notification options ignore_domains and smtp_default_domain interact to put in addresses like ignored-domain@default-domain when addresses with ignored domains are encountered.

The problem is illustrated with an extension of test_default_domain and fixed in trac.notification in the attached patch against 0.11rc1.

[Stephen Compall <stephen.compall@…>]

test and fix

[alon.barlev@…]

Just fixed this with a different patch... Don't know what is better.

[alon.barlev@…]

Before creating smtp mail address, strip domain from first component

[Stephen Compall <stephen.compall@…>]

Replying to alon.barlev@gmail.com:

Just fixed this with a different patch... Don't know what is better.

I am fairly sure this will fail the test I posted, for including otherjoe@default_domain. However, I did not try it.

[alon.barlev@…]

Replying to Stephen Compall <stephen.compall@desktopdoctorsinc.com>:

Replying to alon.barlev@gmail.com:

Just fixed this with a different patch... Don't know what is better.

I am fairly sure this will fail the test I posted, for including otherjoe@default_domain. However, I did not try it.

I don't understand how your patch solves the issue...

I have kerberos user:

alon(at)DOMAIN.ORG

I have ignore domain DOMAIN.ORG and default domain mail.com, the result mail should be:

alon(at)mail.com

is_email() returns false for alon(at)DOMAIN.ORG, this is correct, but then the current result address is: alon(at)DOMAIN.ORG(at)mail.com, which is incorrect.

So my fixup just remove the "@.*" from address before constructing email addres, thus result in correct alon(at)mail.com.

While I think that your patch don't construct address with default domain, it just ignores it.

BTW: How do I write (at) in this wiki?

[alon.barlev@…]

New patch, always strip domain before creating address.

[alon.barlev@…]

OK. New patch always strip the domain so that also use_short_addr will work. It should work:

if not email
   can still have @ignored domain
   but this is the user name so we cannot remove this right now.
   
   check email record for this user,
   if exists use this one skip the rest

   strip ignore domain if exists

   next is as-is while there is no domain name in name anymore.

BTW: I am not much of python coder...

[alon.barlev@…]

Why is this fix was not applied or assigned to any valid milestone?

It is simple issue to fix.

Please make it fixed.

[cboos]

manu, can you have a look at the patch?

[eblot]

Replying to cboos:

manu, can you have a look at the patch?

I'll look at it tonight.

[cboos]

Ok, I'm not familiar with the domain ignore logic, but note that there could some interference with the recent change I did to the domain part of the e-mail regexp in r7400.

[anonymous]

Replying to cboos:

Ok, I'm not familiar with the domain ignore logic, but note that there could some interference with the recent change I did to the domain part of the e-mail regexp in r7400.

Most of the complexity (and bugs) come from the original design choice to have a single input field that can be either used to specify a name or an email address. It's quite difficult to guess whether the field contains an email address or a user login - read: impossible to achieve a 100% recognition rate. Moreover, some authentication backends use the "@" character in the login name, which leads to even more issues.

The question is: should we keep using this unique field and add always more SMTP options in the trac.ini files - which has its own drawbacks, one of them is a unique configuration for all users - and keep tweaking regexps, or would it be better to drop this unique but handy field?

Dropping it would require major UI changes though...

[anonymous]

Replying to cboos:

Ok, I'm not familiar with the domain ignore logic, but note that there could some interference with the recent change I did to the domain part of the e-mail regexp in r7400.

This pattern is incorrect, you cannot expect domain names to be at least two parts. Many installations I got within internal organization have single domain. Please revert.

Replying to anonymous:

Most of the complexity (and bugs) come from the original design choice...

I fully agree, why guess?

Maybe add a simple hook to gain email address, which can take username and return email address (by LDAP for example).

[eblot]

Replying to anonymous:

This pattern is incorrect, you cannot expect domain names to be at least two parts. Many installations I got within internal organization have single domain. Please revert.

Before doing any more changes we need to define what we want to achieve, and what can be actually achieved, from a technical point of view. There have already been too many workarounds with email processing.

I fully agree, why guess? Maybe add a simple hook to gain email address, which can take username and return email address (by LDAP for example).

Define "simple hook" ;-)

• Some installations work with local installation only, with a bare SMTP server
• Some installations work with LDAP directories or other local email databases
• Some installations work with the Internet, where it is pretty impossible to validate an email address
• Some installations use a combination of the above typical configurations
• etc.

SMTP servers are very complex pieces of software. Trying to address these very different environments at once is not as easy as a "simple hook".

[alon.barlev@…]

Replying to eblot:

Before doing any more changes we need to define what we want to achieve, and what can be actually achieved, from a technical point of view. There have already been too many workarounds with email processing.

Current code try to guess the email address out of the user name.

It does so even if the user had configured email address. This is the first mistake.

The existence of @ in the username does not mean that it is email address, for example kerberos environment. But for this you added ignore domain configuration option.

But ignore domain option does not remove this domain so default domain option does not work.

The patch solves this issue, but better to first change the email field of user record first.

The revert of the new template has nothing to do with any future changes, as it is clearly wrong.

I believe the algorithm should be as follows:

if has explicit email in user record
    use it
else
    remove ignore domains from username (should be a list)
    if username looks like email address or use_short_addr
        use it
    else
        if smtp_default_domain
            use username@smtp_default_domain
        else
            fail

Define "simple hook" ;-)

Simple hook is a python code that you eval into the module. Because this is so complex, whoever wish to guess email should have an option to add external logic.