Edgewall Software

Opened 13 years ago

Last modified 13 years ago

#9898 closed defect

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

Reported by: Michel Jouvin <jouvin@…> Owned by:
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

Change History (6)

comment:1 by Thijs Triemstra, 13 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, 13 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@…>, 13 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, 13 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@…>, 13 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, 13 years ago

Description: modified (diff)

fix markup.

Note: See TracTickets for help on using tickets.