Closed
Bug 65313
Opened 24 years ago
Closed 20 years ago
Failure to detect some invalid email addresses
Categories
(Bugzilla :: User Accounts, defect, P3)
Bugzilla
User Accounts
Tracking
()
RESOLVED
FIXED
Bugzilla 2.18
People
(Reporter: baulig, Assigned: goobix)
Details
Attachments
(1 file, 1 obsolete file)
1.26 KB,
patch
|
gerv
:
review+
|
Details | Diff | Splinter Review |
When you create a new Bugzilla account, it normally checks whether the email address you entered is valid. This check fails to detect some invalid email addresses. The problem is the default regexp in defparams.pl is q:^[^@, ]*@[^@, ]*\\.[^@, ]*$: This regexp doesn't catch embedded newlines, tab characters, backslashes and other weird stuff. It'd be better to explicitly list all characters which are allowed in an email address there: q:^[\\w\\d_\\.\\+\\-]+@[\\w\\d_\\.\\+\\-]+\\.[\\w\\d_\\.\\+\\-]+$: or something like this. Note that * after a character class allows zero characters, so that it won't catch stuff like "@.", this should better be a + to require at least one character for the set. Martin
Comment 1•24 years ago
|
||
We think someone created an account on an installation with an embedded newline and this caused an error in sanitycheck.cgi, however we haven't found a browser we can reliably reproduce this in. If this causes an error in sanitycheck.cgi we should check that regardless of emailregexp (or is the check just confirming all the addresses conform to emailregexp?). Should be nice and easy to change the default for new installations. For old installations, we might want to consider asking if they want to change it in checksetup.pl or whatever.
Updated•23 years ago
|
Target Milestone: --- → Bugzilla 2.16
Updated•23 years ago
|
Priority: -- → P3
Comment 2•23 years ago
|
||
-> Bugzilla product, trying User Accounts component, reassigning.
Assignee: tara → myk
Component: Bugzilla → User Accounts
Product: Webtools → Bugzilla
Version: other → unspecified
Comment 3•23 years ago
|
||
q:^[\\w\\d_\\.\\+\\-]+@[\\w\\d_\\.\\+\\-]+\\.[\\w\\d_\\.\\+\\-]+$: My regexps might not be up to much, but surely: q:^[\w\.\+\-]+@[\w\.\-]+\.[\w\.\-]+$: _ and \d are included in \w, and we don't need to escape the backslashes. + is not allowed in domain names. Ccing others for regexp fu. Gerv
Comment 4•23 years ago
|
||
q:^[\w\.\+\-]+@[\w\.\-]+\.[\w\-]+$: No need for the . in the last set as with a domain name such as "chariot.net.au", "chariot.net" will be mached in the first charset after the @ and "au" will bet matched in the last one. Also, w/the extra dot, it will match "chariot.net." just fine, even though that's far from valid.
Comment 5•23 years ago
|
||
We are currently trying to wrap up Bugzilla 2.16. We are now close enough to release time that anything that wasn't already ranked at P1 isn't going to make the cut. Thus this is being retargetted at 2.18. If you strongly disagree with this retargetting, please comment, however, be aware that we only have about 2 weeks left to review and test anything at this point, and we intend to devote this time to the remaining bugs that were designated as release blockers.
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18
Comment 6•22 years ago
|
||
The current default is: ^[^@]+@[^@]+\.[^@]+$ We should be using: ^[\w\.\+\-=]+@[\w\.\-]+\.[\w\-]+$ (as in previous comment, but with "=" sign in username.) Gerv
Assignee | ||
Comment 7•20 years ago
|
||
Tested. Appears like expected on the editparams.cgi page. Succesfully created accounts with it.
Assignee: myk → vlad
Status: NEW → ASSIGNED
Assignee | ||
Updated•20 years ago
|
Attachment #142183 -
Flags: review?(gerv)
Assignee | ||
Updated•20 years ago
|
Hardware: PC → All
Comment 8•20 years ago
|
||
Comment on attachment 142183 [details] [diff] [review] Version 1 (Gerv's proposed fix) The param has associated text, and is documented somewhere, I think. r=gerv if you check for, and possibly update, any associated docs. Gerv
Attachment #142183 -
Flags: review?(gerv) → review+
Assignee | ||
Comment 9•20 years ago
|
||
Thanks. There is first the text that appears in editparams.cgi. It talks about the regexp as trying to match any qualified email address. So we don't need to change that one. I found some references to it in the docs (by searching for "email" and "regexp"), but nothing specific. None of them seemed in need of updates.
Flags: approval?
Updated•20 years ago
|
Flags: approval? → approval+
Assignee | ||
Comment 10•20 years ago
|
||
It seems I forgot to modify the value in checksetup.pl; we could end up with it been validated by checksetup.pl but rejected in the web interface, so it's best to have the same regexp in both places.
Attachment #142183 -
Attachment is obsolete: true
Assignee | ||
Comment 11•20 years ago
|
||
Comment on attachment 142211 [details] [diff] [review] Version 2 You could rubber-stamp this one too for checkin. :)
Attachment #142211 -
Flags: review?(gerv)
Comment 12•20 years ago
|
||
Comment on attachment 142211 [details] [diff] [review] Version 2 r=gerv. No docs, then? :-) Gerv
Attachment #142211 -
Flags: review?(gerv) → review+
Assignee | ||
Comment 13•20 years ago
|
||
Nope. Forgot to CC you when I posted comment #9. The relevant stuff in regard to the docs is: > I found some references to it in the docs (by searching for "email" > and "regexp"), but nothing specific. None of them seemed in need of updates. Thanks! :-) Checking in checksetup.pl; /cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v <-- checksetup.pl new revision: 1.264; previous revision: 1.263 done Checking in defparams.pl; /cvsroot/mozilla/webtools/bugzilla/defparams.pl,v <-- defparams.pl new revision: 1.125; previous revision: 1.124 done
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Summary: Fails to detect some invalid email address when creating accounts → Failure to detect some invalid email addresses
Updated•11 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•