88.92 KB, image/jpeg
4.52 KB, patch
|Details | Diff | Splinter Review|
4.17 KB, patch
|Details | Diff | Splinter Review|
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040206 Firefox/0.8 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6) Gecko/20040206 Firefox/0.8 One line summary: You should allow semi-colons as a valid address separator. Using Thunderbird 0.6, when attempting to use an Exchange 2000 Server as my SMTP server, if I type multiple addresses separated by semi-colons, I get an error that says: "An error occured while sending mail. The mail server responded: Relaying not allowed. Please verify that your e-mail address is correct in your Mail preferences." This would seem to be an Exchange configuration issue, but relaying IS allowed from any IP on this network. I dig further: if I remove the semi-colons and replace them with commas, and it works fine. Reproducible: Always Steps to Reproduce: 1. Configure Thunderbird to use Exchange 2000 as an outgoing SMTP server. 2. Click "Write" 3. Type multiple addresses separated by semi-colons 4. Click send Actual Results: mail will not send Expected Results: realized the semi-colons were separators. if not, warn me that semi-colons are not allowed or could cause problems. I have only tested this with one Exchange server, as its all I have access to.
(In reply to comment #0) > One line summary: You should allow semi-colons as a valid address separator. Semi-colons in fact are not legal separators. The semicolon has a specific use, defined in RFC2822, as a group terminator. This looks like; groupname: email@example.com, firstname.lastname@example.org ; <- semicolon ends group You'll see this often in the form: Undisclosed-Recipients:; Mozilla does not support group addr's well, if at all -- bug 83521, bug 110605. See also bug 94676. > Using Thunderbird 0.6, when attempting to use an Exchange 2000 Server as my > SMTP server, if I type multiple addresses separated by semi-colons, I get an > error that says: > > "An error occured while sending mail. The mail server responded: Relaying not > allowed. Please verify that your e-mail address is correct in your Mail > preferences." See bug 254000 for alternate errors reported from other servers. Also see what happens if you simply Send Later, then look at how the message is addressed in the Unsent Messages folder. > Expected Results: > warn me that semi-colons are not allowed or could cause problems. This is a very reasonable expectation. Mozilla should already know the addresses are a problem before the SMTP server is contacted, and the user should be informed. Repurposing the bug for this request. See also bug 258653, bug 210521.
Here are results of the various ways using of semi-colons as separators. In each case, I enter a string in the address field, hit Send Later, then copy the To: header from the message source in Unsent Messages. Same results in TB 1.0+0222 and Moz 1.8b-0120 (Win2K) To: email@example.com; firstname.lastname@example.org is parsed into To: " <- yes, just a sole double-quote displays as "" in the envelope To: email@example.com; B <firstname.lastname@example.org> is parsed into To: "email@example.com; B" <firstname.lastname@example.org> To: A <email@example.com>; firstname.lastname@example.org is parsed into To: "A ; email@example.com" <firstname.lastname@example.org> To: A <email@example.com>; B <firstname.lastname@example.org> is parsed into To: "A <email@example.com>; B" <firstname.lastname@example.org> The first case is the one that will result in actual errors when sent to the SMTP server; the other parse results are legal if unexpected. Unfortunately, the first case is also the most likely to occur, e.g. in a bogus URL.
*** Bug 270615 has been marked as a duplicate of this bug. ***
(In reply to comment #2) > Semi-colons in fact are not legal separators. > If semi-colons are not "legal" separators, then there should be an option to override the default. Numerous programs that generate e-mail lists use semi-colons to separate them--this is a constant frustration for me. Interestingly, Outlook uses semi-colons as the default separator, and has an option to permit commas as well. I think Thunderbird/Mozilla should do the opposite: use commas as the default (the legal spec?) and give an option to also permit semi-colons (this option should default to "on"). This makes Thunderbird more interoperable with many resources that go by the Outlook default, whether we like it or not.
12 years ago
I've come across the semicolon vs. comma issue in the mailto: link while implementing our product. I tried to find a format that would be parsed successfully by widespread mail clients - Outlook, Outlook Express, Thunderbird, etc. As far as I am aware the Outlook does not (at least by default) support the comma separator for multiple recipients and thus does not follow the RFC. But when I replace the commas with semicolons it is Thunderbird that fails (and probably other Mozilla based mail clients).
*** Bug 254000 has been marked as a duplicate of this bug. ***
Happened to me too. The worst thing about this bug is that there is no feedback (while sending) that anything went wrong! I write a long mail, copy a long prepared list of e-mail addresses from the other file, click "Send", message is sent without problems, I go happily to sleep. Only three days later people start complaining to me that they received no e-mail. When I look into "Sent items", I see that the list of recipients is "". Why did I use ";" as a mail address separator? Well, on my computer this is a list separator as specified in regional settings. Many other programs (like Excel) require me to use it; so I used it without thinking in Thunderbird too. And the program behaved exactly as if I provided a valid list, so there was nothing to raise my suspicion. I guess the quickest was to fix it would be to display a warning window if the e-mail address list contains semicolon. Though I am not sure why the program evaluated the list of recipients to be "", and then sent it anyway; perhaps this should cause an error message. Please please please do something about it, because for user this is very unpleasant. A mistake easy to be made, and no feedback; the mails are not sent, but the user completely believes they were.
My biggest concern is that the average user will not know that their email was not sent successfully. They never got an error. The reason they use an email program is to send and receive mail. If they believe Thunderbird can not be trusted to send mail they will stop using it. In my case, the SMTP server that Thunderbird contacted, in which to send the message, did not flag the addresses as an error, gave no error message to Thunderbird (?), but dropped the message, but Thunderbird thought all was well and gave no error message itself. The user saw just saw his message was sent, and he only needed to look in the Sent folder for confirmation. If the user went to the Sent folder and looked at the message, he would have seen something like this: Subject: Enclosed is my latest article, hope it's under the deadline! From: Me@whyIwriteIdontknow.com To: "" Date: 3/18/2009 3:10 PM X-Mozilla-Status: 0001 X-Mozilla-Status2: 00800000 ... Notice the empty To field? How is the average user supposed to know this? If Thunderbird is going to go after the average Joe or Jane, it has to report errors because your average user is not any good at reading RFCs. Thunderbird can do a quick fix right now - after sending the message, go into the Sent folder and look at what you sent and if the To field is bogus, flag it as an error to the user so they can at least troubleshoot it.
Seriously, how is this even an issue after five years? It is the epitome of open-source idiocy to force your users to alter totally expected and otherwise trained behavior so your *frontend* follows an RFC. Thunderbird users are making mistakes. Nobody cares about the technical docs - that's for software to handle. The front end, the UI, it should be able to spot user mistakes and just fix them. My recommendation, as the one who open this bug five years ago is to do one of the following: * warn the user via an alert dialog, when a semi-colon is spotted, that it is not a valid separator. It could do this on blur or on send OR * transparently change semi-colons to commas in the to: field on blur or on send OR * just accept semi-colons as a valid separator in the software, and pass it to the SMTP server with commas * least acceptably, just do not send the message. Blanking the To: field seems to be the least acceptable outcome of all. If I relied on Thunderbird and it did this to me, I would immediately ditch it with a grudge, thinking the developers clearly were not coding for me as a user. This is dead simple to fix and one of your team agreed in 2005, so the fact that it's open perplexes me. Why anyone would use the RFC as an argument not to properly capture user behavior is beyond me. C'mon -- just *fix* it.
Sorry for the second email, but on re-read that sounds really harsh. Obviously, I am a Thunderbird user for several years. It just seems like a simple fix for a potentially big problem. Thanks for your time and hard work. But please, let's knock out simple issues like this one.
I know you have lots of bugs to fix, and that few engineers like working on bugs - they rather work on the new stuff instead. You guys and gals are doing the world a big favor by helping us wean away from that Outlook-drug we have been taking all these years :-) I love what you are doing! As we go around and convince others to switch over, we have to be realistic and admit a lot of our "users" are not technically oriented. They trust the software to do the thinking for them, like Adam said. Whatever it takes to send and receive mail reliability has to take priority. You can not control which SMTP or POP servers you talk to, and you can not assume those servers were written to an RFC standard. If my users completely and totally believe that Thunderbird will always send and receive mail without fail, they'll stick with it. The reason one of my users switched is because Outlook sometimes would fail to send some email they had (they managed to insert some weird characters into their mail for some reason). They are very sensitive about sending and receiving email.
cc: exchange dudes and bryan for opinions on changing behavior and/or erroring out (in part because the wanted? has not been addressed) twto other semi-colon bugs bug 210521 bug 258653
Created attachment 408336 [details] [diff] [review] Replace semicolons with commas I appreciate any feedback on this patch and the following. If the patch is fine, I am thinking of doing the same thing for Bug 210521 & Bug 258653. SetCC & SetTo in nsMSgCompFields.cpp will pass the replaced string (semicolns are replaced with commas) to SetUnicodeHeader.
Thanks for working on this. From a quick look that doesn't look correct though. Things like To: mygroup:;, email@example.com, othergroup:firstname.lastname@example.org, email@example.com; are valid syntax. I assume that would break with that patch. So you can't blindly replace semicolons with commas. I think it would be ok if the address isn't in a group (starting with "<label>:" ).
This patch doesn't seem to be in the right place. If the error is with reformatUnquotedAddresses not following the spec, then it should be fixed. If, however, the error is when we're formulating the items to go into the address field then we should be correcting that formatting.
Created attachment 408596 [details] [diff] [review] msg_parse_Header_addresses: add "Group Addresses" support, add ';' as a delimeter
Created attachment 408597 [details] [diff] [review] test cases $ make SOLO_FILE="test_splitRecipients.js" -C mailnews/compose/test/ check-one make: Entering directory `/c/mozilla-build/comm-central-devel/obj-i686-pc-mingw3 2/mailnews/compose/test' /c/mozilla-build/python25/python -u /c/mozilla-build/comm-central-devel/mozilla/ config/pythonpath.py \ -I/c/mozilla-build/comm-central-devel/mozilla/build \ /c/mozilla-build/comm-central-devel/mozilla/testing/xpcshell/runxpcshe lltests.py \ --symbols-path=../../../mozilla/dist/crashreporter-symbols \ --test-path=test_splitRecipients.js \ ../../../mozilla/dist/bin/xpcshell \ ../../../mozilla/_tests/xpcshell/test_compose/unit TEST-PASS | c:\mozilla-build\comm-central-devel\obj-i686-pc-mingw32\mozilla\_tes ts\xpcshell\test_compose\unit\test_splitRecipients.js | test passed INFO | Result summary: INFO | Passed: 1 INFO | Failed: 0
Please request review for the patch and testcase. standard8 might be a suitable reviewer for it.
Created attachment 408985 [details] [diff] [review] msg_parse_Header_addresses: add "Group Addresses" support, add ';' as a delimeter, 1
Created attachment 408986 [details] [diff] [review] test cases
Comment on attachment 408986 [details] [diff] [review] test cases The tests are looking good. Although I think you need a couple of more cases: Undisclosed recipients: firstname.lastname@example.org ;;extra:; Undisclosed recipients:;;extra:email@example.com; The second one passes but the first one doesn't afaict. Whilst these are slightly obscure, they are just a variant on what you are trying to support. Could you also use foo.invalid for the cases you're adding (or something similar) rather than invalid.com as per bug 533314?
Comment on attachment 408985 [details] [diff] [review] msg_parse_Header_addresses: add "Group Addresses" support, add ';' as a delimeter, 1 Sorry for the delay in getting to these. >+ // Find the fist non-empty group address or a non-group address nit: First >+ const char *line_temp = line_end; >+ while (*line_temp >+ && !(*line_temp == ',' && paren_depth <= 0 >+ && (!mailbox_start || mailbox_end)) >+ && !(*line_temp == ';' && paren_depth <= 0 >+ && (!mailbox_start || mailbox_end))) Can you put the operators at the end of lines please e.g. while (*line_temp && !(*line_temp == ',' ... >+ /* Skip over extra whitespace or commas between addresses. */ >+ while (*line_end && (isspace(*line_end) || *line_end == ',')) >+ NEXT_CHAR(line_end); Just like you did above, please use IS_SPACE here. >+ if(!*line_end) >+ break; nit: space before ( I still need to think about the logic a bit more, but if you can do the update to this and the test case first, that would be good.
Created attachment 419950 [details] [diff] [review] fix bug in comment 24, add change per comment 25
Created attachment 419951 [details] [diff] [review] add test cases in comment 24 [Checkin: Comment 31]
Comment on attachment 419950 [details] [diff] [review] fix bug in comment 24, add change per comment 25 >- /* Skip over extra whitespace or commas before addresses. >- */ >- while (*line_end && (IS_SPACE(*line_end) || *line_end == ',')) >+ /* Skip over extra whitespace, commas or semicolons before addresses. */ >+ while (*line_end && (isspace(*line_end) || *line_end == ',' || >+ *line_end == ';')) > NEXT_CHAR(line_end); This should definitely be IS_SPACE to account for non-ASCII characters. r=Standard8 with that fixed.
Created attachment 424565 [details] [diff] [review] comment 28 [Checkin: Comment 30]
Comment on attachment 424565 [details] [diff] [review] comment 28 [Checkin: Comment 30] http://hg.mozilla.org/comm-central/rev/81ba8ae8671d
Comment on attachment 419951 [details] [diff] [review] add test cases in comment 24 [Checkin: Comment 31] http://hg.mozilla.org/comm-central/rev/544cf79ddf8c
Can someone say what the outcome of this bug was? I don't know anything about group addresses, but when I enter semicolon-separated single addresses into one recipient row, then press Enter, I think that should be parsed by TB into single addresses, or not? firstname.lastname@example.org; email@example.com current result: nothing expected result: two single addresses in two recipient rows
(In reply to Thomas D. from comment #32) > firstname.lastname@example.org; email@example.com > current result: nothing the two addresses just stay in one row > expected result: two single addresses in two recipient rows TB 24.0a1 (2013-06-21) on WinXP
Oh, we expand semicolon-separated addresses only *when sending*... So is that by design, to educate users that they should not use ; for separating single addresses because that's illegal?
What? Why only when sending? How can one edit the list before sending?
This seems to have regressed in Thunderbird 31, according to bug 1059988. (See that bug for details.)