631206 - Improve checks for invalid email addresses when filling "Email" field of address book contact [validation, refuse, do not accept ()[]\;:,<> before @, multiple @, trailing, leading spaces/whitespace unless quoted]

Status: NEW

(Thunderbird :: Address Book, enhancement)

Description

[Natalia]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US) AppleWebKit/534.10 (KHTML, like Gecko) Chrome/8.0.552.237 Safari/534.10
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; ru; rv:1.9.2.13) Gecko/20101207 Thunderbird/3.1.7

When creating a new contact in Address Book "Email" field checks the presence of character "@" only. Meanwhile it should check all type of invalid email addresses: email which consists of two or more "@" and email which has characters ()[]\;:,<> before the @.

Reproducible: Always

Steps to Reproduce:
1. Hit "Address Book" in the Tool Bar and choose New Contact.
2. Type 111@222@>3<@mail.ru in "Email" field of "Edit Contact" tab and hit OK. 

Actual Results:  
The data accepted without warning about incorrect email address format.

Expected Results:  
The warning message should be displayed.

Comment 1

[Thomas D. (:thomas8)]

:thun,mail :addr su:invalid,wrong,bad,mal su:address,email

https://bugzilla.mozilla.org/buglist.cgi?quicksearch=%3Athun%2Cmail%20%3Aaddr%20su%3Ainvalid%2Cwrong%2Cbad%2Cmal%20su%3Aaddress%2Cemail&list_id=7006924

no dupes found -> confirming

A much more frequent case that we should also prevent by email field pre-validation is trailing or leading whitespace, see related attempts to catch that after-the-fact in bug 286760 and 853437.

Comment 2

[Phil Frost]

I'd caution that "111@222@>3<"@mail.ru is a perfectly valid email address. In fact, there's basically nothing that can't be in the local part as long as it's quoted. See RFC 822, section 6.1. Thus, the proposed action of "do not accept ()[]\;:,<> before @, multiple @, trailing, leading spaces/whitespace", if implemented naively, is incorrect.

Comment 3

[Thomas D. (:thomas8)]

(In reply to Phil Frost from comment #2)
> I'd caution that "111@222@>3<"@mail.ru is a perfectly valid email address.
> In fact, there's basically nothing that can't be in the local part as long
> as it's quoted. See RFC 822, section 6.1. Thus, the proposed action of "do
> not accept ()[]\;:,<> before @, multiple @, trailing, leading
> spaces/whitespace", if implemented naively, is incorrect.

Yes Phil, you are right. Thanks for that caveat.

Comment 4

[Thomas D. (:thomas8)]

Phil, could you elaborate how you'd expect the UI of TB to handle certain inputs, ideally with examples?

- Should we auto-quote invalid inputs (I suppose not, because it's much more likely that user made a mistake compared to entering a weird email address without quotes)
- or should we only accept weird inputs when they are quoted by user? (I think that's more reasonable)
- or what else?

Comment 5

[Phil Frost]

I would expect the UI to accept email addresses formatted exactly as RFC 822 (and friends) specify. The RFCs are the only specifications of email address grammar that everyone can agree on. I should be able to paste any valid To: header in the corresponding field of the UI and have it accepted and parsed according to the RFC.

This alone would resolve some of this and related bugs -- some of them result from TB erronously "correcting" an email address that was already valid. This is DWIM at its worst.

When user input isn't valid, I'd stick to these principles:

  - input is validated when it is first entered, be it in the address book or compose window
  - if input has passed validation previously, it won't get munged or rejected later
  - if TB wants to "correct" an input, it does so only with user confirmation

Perhaps if an invalid address is entered, the UI could respond with an error including suggestions. For example, if I enter

  111@222@>3<@mail.ru

The UI might respond:

  Did you mean:
  "111@222@>3<"@mail.ru
  [pick one] or [go back and edit manually]

This would clearly communicate to the user that there's a problem, but also provide a way forward, yet while never doing anything the user did not specifically request.

Comment 6

[Chuck Aurora]

Phil is correct, per RFC 5322 (which I believe is the latest successor to 822.)  I notice that Thunderbird applies arbitrary limits to the characters it considers "valid" in an email address localpart.  In fact ANY printable character is valid.  Quoting is necessary for " ", "<", ">" and "@", but basically optional for any and all other 7-bit ASCII characters greater than 0x31.  (0x32 being a space.)

My company uses an address which contains "^", like "test^@example.org", which of course is perfectly valid (as is, without quoting.)  But Thunderbird does not recognize it as an email address (that is, I can't click it and get a New Message composer window.

Why didn't you guys read these standards before imposing these arbitrary limits?  Please remove them.  Thanks.

Comment 7

[u]

I noticed today that TB's address book keeps accepting email addresses which are not valid according to RC 5322 (https://www.ietf.org/rfc/rfc5322.txt). I mistyped an address so that it spelled something along the lines of sara@@example.org - this is not a valid email address. I believe that sanitization of user input is basic programming security.

Comment 8

[u]

(In reply to u from comment #7)

I noticed today that TB's address book keeps accepting email addresses which are not valid according to RC 5322 (https://www.ietf.org/rfc/rfc5322.txt). I mistyped an address so that it spelled something along the lines of sara@@example.org - this is not a valid email address. I believe that sanitization of user input is basic programming security.

I'm using TB 78.12.0 (64-bit) on Debian.