Closed Bug 1707408 Opened 3 years ago Closed 3 years ago

browser.accounts.list() fails if a Gmail/IMAP folder contains a '%' character

Categories

(Thunderbird :: Add-Ons: Extensions API, defect)

Thunderbird 89
defect

Tracking

(thunderbird_esr78? fixed, thunderbird89 fixed, thunderbird90?)

RESOLVED FIXED
90 Branch
Tracking Status
thunderbird_esr78 ? fixed
thunderbird89 --- fixed
thunderbird90 ? ---

People

(Reporter: dreadnaut, Assigned: TbSync)

References

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:87.0) Gecko/20100101 Firefox/87.0

Steps to reproduce:

In the options page of the "BorderColors-D"¹ add-on, I use browser.accounts.list() to retrieve all identities. A user found an issue² where the call would fail. Eventually, we tracked this down to the existence of a folder named "%Something".

I can reproduce this on TB89.0b1, if the folder is part of a Gmail account connected via IMAP. (Note: I don't have a non-Gmail IMAP account to test). No trouble when the folder is under Local Folder.

In the error console I can find this message:

malformed URI sequence 2 ext-mail.js:1509
    map self-hosted:224
    folderURIToPath chrome://messenger/content/parent/ext-mail.js:1509
    convertFolder chrome://messenger/content/parent/ext-mail.js:1575
    traverseSubfolders chrome://messenger/content/parent/ext-mail.js:1597
    traverseSubfolders chrome://messenger/content/parent/ext-mail.js:1600
    traverseSubfolders chrome://messenger/content/parent/ext-mail.js:1600
    convertAccount chrome://messenger/content/parent/ext-mail.js:1459
    list chrome://messenger/content/parent/ext-accounts.js:18
    list self-hosted:1178
    result resource://gre/modules/ExtensionParent.jsm:935
    withPendingBrowser resource://gre/modules/ExtensionParent.jsm:491
    result resource://gre/modules/ExtensionParent.jsm:935
    callAndLog resource://gre/modules/ExtensionParent.jsm:897
    recvAPICall resource://gre/modules/ExtensionParent.jsm:934
    InterpretGeneratorResume self-hosted:1485
    AsyncFunctionNext self-hosted:695

Which points to the decodeURIComponent call in this function:

/**
 * Convert a folder URI to a human-friendly path.
 * @return {String}
 */
function folderURIToPath(uri) {
  let path = Services.io.newURI(uri).filePath;
  return path
    .split("/")
    .map(decodeURIComponent)
    .join("/");
}

[1] https://addons.thunderbird.net/en-GB/thunderbird/addon/bordercolors-d/
[2] https://github.com/dreadnaut/bordercolors-d/issues/18#issuecomment-824395128

Assignee: nobody → john
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/21d62e67ea80
Do not encode/decodeURIComponent the folderURI of IMAP folders (which fails if % are used). r=darktrojan

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch

Thank you for getting this fixed in such a short time!

Short time? I've filed this bug about 4 month ago. See Bug 1684327. But thanks anyway.

The test for this is simple, and it also seems to fix bug 1688612, so we should backport this to 78, as it fixes two important API methods (messages.query and messages.list)

Comment on attachment 9219031 [details]
Bug 1707408 - Do not encode/decodeURIComponent the folderURI of IMAP folders (which fails if % are used). r=darktrojan

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined:
This fixed multiple annoying bugs.

Testing completed (on c-c, etc.):
https://treeherder.mozilla.org/jobs?repo=try-comm-central&test_paths=comm%2Fmail%2Fcomponents%2Fextensions%2Ftest%2Fbrowser&revision=21d540f356137e043836bbc0505dfc5f84426029

Risk to taking this patch (and alternatives if risky):
Low.

Attachment #9219031 - Flags: approval-comm-beta?

Comment on attachment 9219031 [details]
Bug 1707408 - Do not encode/decodeURIComponent the folderURI of IMAP folders (which fails if % are used). r=darktrojan

[Triage Comment]
Approved for beta

Attachment #9219031 - Flags: approval-comm-beta? → approval-comm-beta+
Flags: needinfo?(john)

[Approval Request Comment]
Regression caused by (bug #):
User impact if declined:
This fixed multiple annoying bugs.

Testing completed (on c-c, etc.):
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=433531c52d4f4e40b5599004fb9e41fac0233341

Risk to taking this patch (and alternatives if risky):
Low

Flags: needinfo?(john)
Attachment #9221517 - Flags: approval-comm-esr78?

Comment on attachment 9221517 [details] [diff] [review]
ESR78_D113697_2.diff

[Triage Comment]
Approved for esr78

Attachment #9221517 - Flags: approval-comm-esr78? → approval-comm-esr78+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: