Closed Bug 1664039 Opened 4 years ago Closed 4 years ago

When pasted comma-separated email addresses get converted to pills, the address immediately after an error pill goes missing (dataloss)

Categories

(Thunderbird :: Message Compose Window, defect, P3)

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1674650

People

(Reporter: thomas8, Unassigned)

References

Details

+++ This bug was initially created as a clone of Bug #1663041 +++

STR

  1. compose, and paste the following list of 5 comma-separated addresses into To-field, with one erroneous address in the middle (e.g. on German keyboard, if you don't hold AltGr long enough and press q key where the @ is third-level character, you'll just get q):
    john <john@example.com>, error <fooqbar.com>, jane <jane@example.com>, foo <foo@bar.com>, bar@baz.com
  2. press Enter and count resulting pills

Actual result:

  • 1 address lost, jane deleted! 4 out of 5 remain. Cunningly, only one good address after the erroneous address gets deleted, others remain. You may never notice that one is missing somewhere in the middle.

Expected result:

  • Please don't lose pills when converting csv

Something's wrong here. Just having an error pill shouldn't eat random pills...
Can we please find the problem here and make sure that an erroneous address doesn't cause dataloss.

Summary: Pills created after pasting comma-separated email addresses truncated at first error pill (dataloss) → When pasted comma-separated email addresses get converted to pills, the address immediately after the error pill is missing (dataloss)
Summary: When pasted comma-separated email addresses get converted to pills, the address immediately after the error pill is missing (dataloss) → When pasted comma-separated email addresses get converted to pills, the address immediately after the error pill goes missing (dataloss)
Summary: When pasted comma-separated email addresses get converted to pills, the address immediately after the error pill goes missing (dataloss) → When pasted comma-separated email addresses get converted to pills, the address immediately after an error pill goes missing (dataloss)

This goes through recipientAddPill() which calls MailServices.headerParser.makeFromDisplayAddress()

https://searchfox.org/comm-central/rev/421c8e4ca45dee96a86a73bb00cec4e7552f4f93/mailnews/mime/src/MimeJSComponents.jsm#401
_makeSingleAddress will get sent input of "error <fooqbar.com>, jane <jane@example.com>"

Basically every part is working as it should, and ultimately it's our internal maillist syntax which is to blame :/
Not sure if this is easily solvable. Luckily I don't think it's a common thing for people to enter "foo <foo@example.com>" type addresses.

Priority: P2 → P3

(In reply to Magnus Melin [:mkmelin] from comment #1)

Basically every part is working as it should, and ultimately it's our internal maillist syntax which is to blame :/
Not sure if this is easily solvable. Luckily I don't think it's a common thing for people to enter "foo <foo@example.com>" type addresses.

Well, if every part was working as it should, this bug should not happen. Clearly our algorithm is wrong whichever way we look at it. I understand that "error <fooqbar.com>" could be mistaken as a mailing list, but that's no reason to dump the next address, which is complete and correct. Looking at the syntax of the CSV input string, "error <fooqbar.com>, jane <jane@example.com>" , there's no doubt syntactically that these are intended as two comma-separated addresses, so we're supposed to split at the comma first and then look at the two parts.

If we took the whole thing as one address, we'd have to consider the left side ("error <fooqbar.com>, jane") as display name, which we can't because it has unquoted angle brackets. Furthermore, at least we should create an error pill which has the full original string, instead of randomly cutting out an entire correct address just because the previous one is lacking an @.

I've tried to explain that the risk of accidentally typing another character for @ is much higher on keyboard layouts where @ is a third-level key requiring modifier keys to be pressed.

(In reply to Thomas D. (:thomas8) from comment #2)

no reason to dump the next address, which is complete and correct. Looking at the syntax of the CSV input string, "error <fooqbar.com>, jane <jane@example.com>" , there's no doubt syntactically that these are intended as two comma-separated addresses, so we're supposed to split at the comma first and then look at the two parts.

We can't do that, at least easily. That's why I blame the maillist syntax.
After we encounter an address, we must discard everything after it, or we expose users to odd attack scenarios (e.g. bug 1526456).

I think bug 1674650 would have fixed this. Please retest with today's nightly and dupe it if resolved.

I tested this and there's no more dataloss.
Bug 1674650 fixed it.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.