902370 - Renaming IMAP folder using non-ASCII characters like Japanese creates .msf and .sbd (and so on) files with system native character string instead of UTF7 encoded string.

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[mkaneda]

User-Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:17.0) Gecko/20130620 Thunderbird/17.0.7

Renaming an IMAP folder using non-ascii characters like japanese creates .msf and .sdb (and so on) files with system locale character string instead of UTF7 encoded string on Thunderbird 17.0.7.

Then renaming to another name, new folder name and old folder name are displayed in the folder pane.

Then clicking the old folder in the pane, the popup message "The current command did not succeed. ... Mailbox doesn't exists: &MEIwQjBC~." appears. I can not delete the old folder until restart the thunderbird.

I also comfirm this issue on Windows 8 and Scientific Linux 6.3 in Japanese.

Reproducible: Always

Steps to Reproduce:
1. create a folder "あ"
   in Profiles\xxxx.default\ImapMail\<imap_server>, "&MEI-.msf" is created.

2. rename the folder to "あああ"
   in Profiles\xxxx.default\ImapMail\<imap_server>, the .msf file is renamed to "あああ.msf" instead of "&MEIwQjBC-.msf"

3. rename the folder to "あああいいい"
   in folder Pane, "あああ" still remains.
   in Profiles\xxxx.default\ImapMail\<imap_server>, the .msf file is renamed to "あああいいい.msf", but "あああ.msf" still remains.

4. right click on the folder "あああ" 

Actual Results:  
The popup message "The current command did not succeed. ... Mailbox doesn't exists: &MEIwQjBC~." appears.
I can not delete the old folder.

Expected Results:  
no error message and .msf file name shuld be corrected.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Attachment: test_for_bug_902370.patch

An xpcshell test for this issue.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Mark, is there any chance to get this fixed in ESR 24?
I've heard that a company uses TB ESR 17 is getting in this trouble. And I've also heard that the reporter  mkaneda is ready to post a patch to fix this issue.

I hope this issue will fix in next ESR and will do anything what I can do.

Thank you,

Comment 3

[mkaneda]

Ikezoe-san, thank you for your help.

I comment additional info I have investigated.

I think this issue is caused by the following line in mailnews/imap/src/nsImapMailFolder.cpp nsImapMailFolder::RenameClient():

rv = CopyMUTF7toUTF16(PromiseFlatCString(newName), newNameString);


In the case of creating folder, a similar procedure is the following in mailnews/imap/src/nsImapMailFolder.cpp nsImapMailFolder::CreateClientSubfolderInfo():

NS_ConvertASCIItoUTF16 leafName(folderName); 


The CopyMUTF7toUTF16() in RenameClient() was introduced in Bug 499278.

Thank you.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: Fix with a unit test

I had misunderstood that  mkaneda had already a patch to fix this issue. So I made a patch on behalf of him here.

mkaneda, thanks for the useful info.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Attachment: Fix with a unit test

Sorry for the garbage in the previous patch.

Comment 6

[WADA:World Anti-bad-Duping Agency]

FYI.
This bug is exposed only when rename was executed twice without folder re-discovery by "collapse/re-expand of account" or "restart of Tb".

1. Rename あ to あああ　=> "あああ.msf" is used instead of "&MEIwQjBC-.msf".
2. Open folder named あああ
   => "あああ.msf" is used.
      If Offline^use=On, "あああ" is used as offline-store file.
msgFolder.URI and msgFolder.folderURI, which is used by message filter, virtual folder etc., is broken.
> msgFolder.URI       = imap://[e-mail]/<UTF-8 binary of あああ>
> msgFolder.folderURI = imap://[e-mail]/%E3%81%82%E3%81%82%E3%81%82
This is caue of this bug.
But access to mail data has no problem, because "what file name is used" is not relevant to file access(once opened, file handle is used for .msf file access and offline-store file access).
3. Collapse account and re-expand account, or restart Tb.
3-1. Folder re-discovery is invoked, then correct Modified-utf-7 is used in URI / folderURI, and is used for .msf file name / offline-store file name.
> msgFolder.URI       = imap://[e-mail]/&MEIwQjBC-
> msgFolder.folderURL = imap://[e-mail]/%26MEIwQjBC-
3-2. Old あああ.msf, あああ(offline-store file) is deleted, because Mbox corresponds to あああ.msf doesn't exist at IMAP server.
3-3. Because "rename in IMAP" won't copy local mail related data,
     message headers of all mails are fetched from server again,
     and entire mail data of all mails are downloaded to "&MEIwQjBC-".
So, user usually is not aware of this bug.
User simply sees phenomenon like "message filter rule is disabled", "target folder of virtual folder is removed", "all mail is re-downloaded by auto-sync, after auto-sync is executed at step 2".
4. If rename of "あああ" to "あああいいい" happens before step 3 occurs, phenomenon of comment #0 is exposed to Tb user.

Comment 7

[WADA:World Anti-bad-Duping Agency]

FYI.
This bug occurs also when "&" is used in Mbox name, because Modified-UTF-7 of "&" is "&-".

Comment 8

[mkaneda]

Ikezoe-san, 

I have verified that the patch works very well.

Thank you so much.

Comment 9

[Mark Banner (:standard8)]

Comment on attachment 787486 [details] [diff] [review]
Fix with a unit test

Redirecting review as I'm going to be afk for a few days.

Comment 10

[[e-mail]]

Comment on attachment 787486 [details] [diff] [review]
Fix with a unit test

Sorry for the delay, I was busier than normal last week and I wasn't in the mood for reviews over the UK bank holiday weekend.

This patch seems reasonable to me by comparison with CreateClientSubfolderInfo but I didn't try running the test.

Comment 11

[Hiroyuki Ikezoe (:hiro)]

Archaeopteryx, could you please push the patch to try server with xpcshell test?

Thank you.

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Pushed to Thunderbird-Try as https://tbpl.mozilla.org/?tree=Thunderbird-Try&showall=1&rev=abeeb120fbd6

The builds will be likely orange or red, but the xpcshell tests seem to work. Compare with earlier pushes if necessary.

Comment 13

[Hiroyuki Ikezoe (:hiro)]

Thanks Archaeopteryx!

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/ff2e9fbb1d73

Comment 15

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 787486 [details] [diff] [review]
Fix with a unit test

[Approval Request Comment]
Regression caused by (bug #): 499278
User impact if declined: loss data in the worst case
Testing completed (on c-c, etc.): yes
Risk to taking this patch (and alternatives if risky): low

Comment 16

[Justin Wood (:Callek)]

Standard8, is this patch safe for 24? Data-loss in worst case [imho] warrants getting it in before our cutoff.

Comment 17

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-aurora/rev/a3a0b7ac87c7
https://hg.mozilla.org/releases/comm-beta/rev/a7ff46d7c192