Email address added twice in ADD CCs due to case-insensitive query

RESOLVED FIXED in Bugzilla 2.16

Status

()

defect
P1
normal
RESOLVED FIXED
18 years ago
7 years ago

People

(Reporter: gilbert.fang, Assigned: thomas+mozilla)

Tracking

2.15
Bugzilla 2.16
Bug Flags:
approval2.16 +
blocking2.16.6 +

Details

Attachments

(1 attachment)

Reporter

Description

18 years ago
My bugzilla account is my email address with initial-capitalized.
when I input my email address in the Add CC field , it is all lower-case.
The result is the two addresses (actully one ) are all added in the CCs.
And  I get the notifications twice with the two address seperatly.
Priority: -- → P2
Target Milestone: --- → Bugzilla 2.16
Well, this definitely shouldn't happen:

Email sent to: matty@chariot.net.au, Gilbert.Fang@sun.com, bzbot@bugzilla.org,
dkl@redhat.com, gerv@MOZILLA.org
Excluding: jake@acutex.net, mmelgazzi@rsasecurity.com, gerv@mozilla.org 

I think we need to be lower-casing email addresses somewhere in processmail, but
I can't immediately see where would be best.

bbaetz seems to be the local expert here...

Gerv
We should probably just invert the DBNameToIdAndCheck before adding to the hash
in process_bug, at at least the area round line 667. Or maybe we should do this
in processmail.

/me shrugs. I'll think about it

Eventually processmail really needs to become a sub which can be passed all this
in directly.
Why doesn't the CCs table have the relevant uniqueness constraint on IDs?  Do
these two different accounts with different case actually exist?
It does, which is why gerv is here only once in the cc list. Its probably passed
to -forcecc, or read from the activity log, or something
A quick hack is just to lowercase @emaillist in ProcessOneBug (and the
forcelist, too). Or this could be done in the alreadyseen bit, I guess, but then
you'd have to change multiple places (I'm not going to ask why we have multiple
places doing this). Comments?
Sorry to push this back, but in the grand scheme of things this is kind of a
minor issue.  If we get a patch in the next couple days we can put it back on 2.16
OS: Windows 2000 → All
Priority: P2 → P1
Hardware: PC → All
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18
Blocks: 103404
Changing default owner of Email Notifications component to JayPee, a.k.a.
J. Paul Reed (preed@sigkill.com).  Jake will be offline for a few months.
Assignee: jake → preed
*** Bug 103404 has been marked as a duplicate of this bug. ***
There are some hairy problems with case-sensitivity in email addresses.

domain names (the part after the @) are by definition case-insensitive.

user names (the part before the @) are by definition case-SENSITIVE by the RFC,
but in practice some hosts treat it case-insensitively.

Technically, usernames in two different cases on the same host could be two
separate users.  Then again it might not be.  Some hosts (AOL is a good example)
let you change the case of your email address on the fly whenever you feel like
it, because they don't treat it case-sensitively.  On other hosts (pretty much
anything running a variant of UNIX) the addresses are very much case-sensitive.

So we have a minor security hole here if we don't treat the username portion as
case-sensitive.  On the other hand, how likely is it that you'd have two users
with the same username, but using different cases on the same host?

In either case we should send the email address to processmail in the same case
it was entered when the user signed up, whatever that case is.  We should only
do case-insensitive matches when verifying entered data (CC list,
authenticating, etc), but use it as originally entered any time we display/use it.
Summary: Email address added twice in ADD CCs → Email address added twice in ADD CCs due to case-insensitive query
Assignee

Comment 10

17 years ago
This bug bit me today when upgrading our internal  deployment where all of the
2000 imported e-mail accounts have mixed case.

And as far as UNIX goes for e-mail reception, everything I know follows the RFC
(Postfix & Sendmail was just tested.. not to mention Exchange), in being case
insensitive for e-mails.

While it may not be the most elegant, I nominate that we if nothing else just
lowercase the e-mails as they get sent out. If anyone out there has an
RFC-non-complaint mailserver, let them have less e-mail because of it. Do you
have an example of a mailserver which breaks RFC in this manner?

I'll post an ugly hack patch later today to solve this, since this is an ugly
thing for us in production right now. 



Assignee

Comment 11

17 years ago
This patch changes processmail so that it checks case sensitivity in the
following manners:

ProcessOneBug(): When checking to see if it's already seen that user.
filterExcludeList(): When checking to see that it's already seen that user.
filterExcludeList(): When checking to see if the excludes are correct. This
fixes another issue where if a user has a mixed case login to bugzilla, but
logs in with a lowercase equivalent for that session - he will get e-mailed any
bug he enters.
*** Bug 156446 has been marked as a duplicate of this bug. ***

Updated

17 years ago
Keywords: patch, review

Comment 13

17 years ago
Comment on attachment 81878 [details] [diff] [review]
processmail patch to case-insensitive check who you've mailed. 

>+	# don't modify the original so it sends out with the right case
>+	# based on who came first.
>+	$checkperson = lc($person);

r=jouni, but convert those tabs into appropriate sequences of spaces.
Attachment #81878 - Flags: review+

Comment 14

17 years ago
Comment on attachment 81878 [details] [diff] [review]
processmail patch to case-insensitive check who you've mailed. 

why not use =1 instead of ++? we really do mean =1..
>+        if ( !($AlreadySeen{$checkperson}) ) {
>             push(@allEmail,$person);
>-            $AlreadySeen{$person}++;
>+            $AlreadySeen{$checkperson}++;
>         }

>+	$checkperson = lc($person);
>+        if ( !($alreadySeen{$checkperson}) ) {
>             push(@uniqueResult,$person);
>-            $alreadySeen{$person}++;
>+            $alreadySeen{$checkperson}++;
>         }
Attachment #81878 - Flags: review+

Comment 15

17 years ago
-> thomas+mozilla@stromberg.org, the patch author

Justdave: you had some doubts in comment 9. Is this patch an acceptable fix for
you? If so, I'll check this into the trunk.
Assignee: preed → thomas+mozilla
Comment on attachment 81878 [details] [diff] [review]
processmail patch to case-insensitive check who you've mailed. 

a=justdave

looks good to me.

Comment 17

17 years ago
Thanks for the patch, Thomas!

Checking in processmail;
/cvsroot/mozilla/webtools/bugzilla/processmail,v  <--  processmail
new revision: 1.84; previous revision: 1.83
done
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
*** Bug 165126 has been marked as a duplicate of this bug. ***
*** Bug 199061 has been marked as a duplicate of this bug. ***
*** Bug 243023 has been marked as a duplicate of this bug. ***
r= justdave for 2.16 branch, low-risk, high-value patch, applies unmodified, and
still works.

Fixed on 2.16 branch: (for 2.16.6)
Checking in processmail;
/cvsroot/mozilla/webtools/bugzilla/Attic/processmail,v  <--  processmail
new revision: 1.81.2.2; previous revision: 1.81.2.1
done
Flags: blocking2.16.6+
Flags: approval2.16+
Target Milestone: Bugzilla 2.18 → Bugzilla 2.16
Bug #271666 filed about sanity check checking this.
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.