[PATCH] = considered a valid character in email address but causing problems with certificates

[Michel Jouvin <jouvin@…>]

Notification module (trac/notification.py) uses a regular expression EMAIL_LOOKALIKE_PATTERN to extract email from connection name, if any is defined. For the email local part, the valid characters include in particular =. This is may be valid from a RFC point of view but I have never seen it used in an email address local part and this is causing problems with certificates whose DN contains the attribute /emailAddress=.... In this case, an email address is found in the connection name (which in principle is fine) but the email address extracted is emailAddress=... instead of the email itself.

For example the DN of my certificate is:

/C=FR/O=CNRS/OU=UMR8607/CN=Michel Jouvin/emailAddress=jouvin@lal.in2p3.fr

and results in notifications to be sent to emailAddress=jouvin@lal.in2p3.fr instead of jouvin@lal.in2p3.fr which obviously results in a failure.

This is a major problem if a user with such a connection name tries to define its email address in the preferences. In this case, he will not be able to connect to the project until the session attributes are removed from the database.

If removal of = from valid characters is considered acceptable an easy fix is:

notification.py

@@ -29 +29 @@
 MAXHEADERLEN = 76
 EMAIL_LOOKALIKE_PATTERN = (
         # the local part
-        r"[a-zA-Z0-9.'=+_-]+" '@'
+        r"[a-zA-Z0-9.'+_-]+" '@'
         # the domain name part (RFC:1035)
         '(?:[a-zA-Z0-9_-]+\.)+' # labels (but also allow '_')
         '[a-zA-Z](?:[-a-zA-Z\d]*[a-zA-Z\d])?' # TLD

[Remy Blank]

Replying to Michel Jouvin <jouvin@…>:

… and this is causing problems with certificates whose DN contains the attribute /emailAddress=.... In this case, an email address is found in the connection name (which in principle is fine) but the email address extracted is emailAddress=... instead of the email itself.

Can't you set the e-mail address explicitly in the user preferences? Or is this about the convenience of not having to repeat the e-mail address there?

Note that until lately, an e-mail address found in the user's name field was preferred to the e-mail address field. You may want to try again with a recent trunk (see [10347]).

[Michel Jouvin <jouvin@…>]

Replying to rblank:

Can't you set the e-mail address explicitly in the user preferences? Or is this about the convenience of not having to repeat the e-mail address there?

No it doesn't help. In fact this is worst! Until you define your email address in your user prefs, notification is just broken but you can use Trac. Once you start to define it, it finds your defined email address is different from the one in your user name and try to send you a confirmation request using the wrongly parsed address. The Trac project becomes unusable : for every request you get an internal error with the Python traceback.

Note that until lately, an e-mail address found in the user's name field was preferred to the e-mail address field. You may want to try again with a recent trunk (see [10347]).

I am using 0.12-stable r10351. I cannot use the trunk anymore as it requires Python ≥ 2.5.

I really insist that I never saw an = in an email address and that I cannot imagine that this will break anything… where it really solves this issue and allows to take advantage of the /emailAddress in a certificate which by design must be valid. In the allowed characters, there is also the single quote which is a bit suspect IMO… but at least I have not seen it causing a problem so far.

[Remy Blank]

Replying to Michel Jouvin <jouvin@…>:

Once you start to define it, it finds your defined email address is different from the one in your user name and try to send you a confirmation request using the wrongly parsed address.

This is probably an issue with ​th:AccountManagerPlugin. Trac core never sends confirmation e-mails. You may want to file a ticket on trac-hacks.org about that.

I am using 0.12-stable r10351. I cannot use the trunk anymore as it requires Python ≥ 2.5.

In that case, setting the e-mail address won't help anyway.

I really insist that I never saw an = in an email address and that I cannot imagine that this will break anything…

I understand that. At the same time, it is a hack, and we try to avoid hacks if there's a correct way to do something. But maybe here it's the right solution.

[Michel Jouvin <jouvin@…>]

Replying to rblank:

Replying to Michel Jouvin <jouvin@…>:

I really insist that I never saw an = in an email address and that I cannot imagine that this will break anything…

I understand that. At the same time, it is a hack, and we try to avoid hacks if there's a correct way to do something. But maybe here it's the right solution.

I fully support avoiding hacks in any case. But here this is not a hack, I disagree : you apply a pattern matching rule to the username to see if it contains an email and the rule is buggy (as I hardly believe a = present in the local part of an email address will work anyway with most of the mailers). Fixing the rule just allows to handle another case where the email is part of the username.

[Thijs Triemstra]

fix markup.

[erne.castro@…]

I'm no one, but I fully support rblank on this.

Email addresses format is defined in ​rfc5322 and ​rfc5321 and they do explicitly allowed '=' in the local part.

I'm not an email guru, just read the ​wikipedia.

I would close this ticket as wontfix.

[Christian Boos]

We can't close this as wontfix or worksforme if this triggers an internal error as mentioned in comment:3. Michel, any chance to get more detail about this?

As for ​rfc:5322, our regexp is already a very rough approximation of it. As noted elsewhere, getting a "correct" regexp for this is asking for trouble (e.g. ​Mail::RFC822::Address).

So I personally have nothing against bending our current regexp a little more if this can avoid some problems without introducing new ones, as this seems to be the case here.

[Remy Blank]

Replying to cboos:

We can't close this as wontfix or worksforme if this triggers an internal error as mentioned in comment:3. Michel, any chance to get more detail about this?

The internal error is due to a bug in ​th:AccountManagerPlugin AFAICT. As a workaround, Michel could disable the acct_mgr.web_ui.EmailVerificationModule component (as he doesn't seem to be using e-mail verification).

As for removing the = from the regexp, I'm neutral. I have to admit that I haven't ever seen an e-mail address containing that character. So let's remove it, and we can still add it back if anyone complains.

[Remy Blank]

Patch applied in [10398].