1647249 - Address book migration fails on "bad" mailing list name

Status: RESOLVED FIXED

(MailNews Core :: Address Book, defect)

Description

[Jorg K (CEST = GMT+2)]

I have a mailing list called "a, b, c" from ancient times. This made the AB migration from TB 68 to TB 78 fail.

Comment 1

[Geoff Lankow (:darktrojan)]

Attachment: 1647249-migrate-bad-lists-1.diff

I've mitigated this problem in two ways: put the list migration into a try/catch block so that one bad list doesn't prevent a migration, and converting invalid or duplicate list names into valid list names.

Comment 2

[Geoff Lankow (:darktrojan)]

Comment on attachment 9158401 [details] [diff] [review]
1647249-migrate-bad-lists-1.diff

This could break address book migration, and we only really get one shot at that, so it has to be in 78.

Comment 3

[Magnus Melin [:mkmelin]]

Comment on attachment 9158401 [details] [diff] [review]
1647249-migrate-bad-lists-1.diff

Review of attachment 9158401 [details] [diff] [review]:
-----------------------------------------------------------------

r=mkmelin with the changes below

::: mail/base/modules/MailMigrator.jsm
@@ +781,5 @@
>        if (cardMap.size > 0) {
>          await newBook._bulkAddCards(cardMap.values());
>  
>          for (let card of database.enumerateCards(directory)) {
>            if (card.isMailList) {

could just do an early continue here

@@ +785,5 @@
>            if (card.isMailList) {
> +            try {
> +              let listName = card.displayName
> +                .trim()
> +                .replace(/  +/g, " ")

maybe use `.replace(/\s+/g, " ")` - I don't know if that's been (ever) possible to enter throuh

@@ +786,5 @@
> +            try {
> +              let listName = card.displayName
> +                .trim()
> +                .replace(/  +/g, " ")
> +                .replace(/[,;"<>]/g, "");

I think you want to do the double space replacements later, and the trimming last. Otherwise you could get double spaces due to removing junk.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/a8766e0cda5d
Fix address book migration failure with invalid mailing list names. r=mkmelin DONTBUILD

Comment 5

[Wayne Mery (:wsmwk)]

Comment on attachment 9158401 [details] [diff] [review]
1647249-migrate-bad-lists-1.diff

Approved for beta.
good thing there are tests ;)

Comment 6

[Rob Lemley [:rjl]]

Thunderbird 78.0b4:
https://hg.mozilla.org/releases/comm-beta/rev/5ece8ea9b110