Edgewall Software
Modify

Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#9898 closed defect (fixed)

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

Reported by: Michel Jouvin <jouvin@…> Owned by: Michel Jouvin <jouvin@…>
Priority: normal Milestone: 0.12.2
Component: notification Version: 0.12.1
Severity: normal Keywords: patch
Cc: Thijs Triemstra Branch:
Release Notes:
API Changes:
Internal Changes:

Description (last modified by Thijs Triemstra)

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

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

Attachments (0)

Change History (11)

comment:1 by Thijs Triemstra, 11 years ago

Cc: Thijs Triemstra added
Keywords: patch added
Summary: = considered a valid character in email address but causing problems with certificates[PATCH] = considered a valid character in email address but causing problems with certificates

in reply to:  description ; comment:2 by Remy Blank, 11 years ago

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]).

in reply to:  2 ; comment:3 by Michel Jouvin <jouvin@…>, 11 years ago

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.

in reply to:  3 ; comment:4 by Remy Blank, 11 years ago

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.

in reply to:  4 comment:5 by Michel Jouvin <jouvin@…>, 11 years ago

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.

comment:6 by Thijs Triemstra, 11 years ago

Description: modified (diff)

fix markup.

comment:7 by erne.castro@…, 11 years ago

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.

comment:8 by Christian Boos, 11 years ago

Owner: set to 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.

in reply to:  8 comment:9 by Remy Blank, 11 years ago

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.

comment:10 by Remy Blank, 11 years ago

Resolution: fixed
Status: newclosed

Patch applied in [10398].

comment:11 by Remy Blank, 11 years ago

Owner: changed from Christian Boos to Michel Jouvin <jouvin@…>

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Michel Jouvin <jouvin@…>.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Michel Jouvin <jouvin@…> to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.