Last Comment Bug 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.
: Renaming IMAP folder using non-ASCII characters like Japanese creates .msf an...
Status: RESOLVED FIXED
: dataloss, regression
Product: MailNews Core
Classification: Components
Component: Networking: IMAP (show other bugs)
: Trunk
: All All
: -- critical (vote)
: Thunderbird 26.0
Assigned To: Hiroyuki Ikezoe (:hiro)
:
Mentors:
Depends on:
Blocks: 499278
  Show dependency treegraph
 
Reported: 2013-08-07 02:34 PDT by mkaneda
Modified: 2013-09-09 14:28 PDT (History)
6 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed


Attachments
test_for_bug_902370.patch (2.71 KB, patch)
2013-08-07 03:28 PDT, Hiroyuki Ikezoe (:hiro)
no flags Details | Diff | Splinter Review
Fix with a unit test (6.54 KB, patch)
2013-08-08 06:22 PDT, Hiroyuki Ikezoe (:hiro)
no flags Details | Diff | Splinter Review
Fix with a unit test (5.88 KB, patch)
2013-08-08 06:24 PDT, Hiroyuki Ikezoe (:hiro)
neil: review+
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑beta+
Details | Diff | Splinter Review

Description mkaneda 2013-08-07 02:34:16 PDT
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) 2013-08-07 03:28:45 PDT
Created attachment 786839 [details] [diff] [review]
test_for_bug_902370.patch

An xpcshell test for this issue.
Comment 2 Hiroyuki Ikezoe (:hiro) 2013-08-07 03:36:12 PDT
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 2013-08-08 05:42:25 PDT
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) 2013-08-08 06:22:27 PDT
Created attachment 787485 [details] [diff] [review]
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) 2013-08-08 06:24:33 PDT
Created attachment 787486 [details] [diff] [review]
Fix with a unit test

Sorry for the garbage in the previous patch.
Comment 6 WADA 2013-08-09 19:23:02 PDT
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://yatter.king%40gmail.com@imap.gmail.com/<UTF-8 binary of あああ>
> msgFolder.folderURI = imap://yatter.king%40gmail.com@imap.gmail.com/%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://yatter.king%40gmail.com@imap.gmail.com/&MEIwQjBC-
> msgFolder.folderURL = imap://yatter.king%40gmail.com@imap.gmail.com/%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 2013-08-09 19:34:42 PDT
FYI.
This bug occurs also when "&" is used in Mbox name, because Modified-UTF-7 of "&" is "&-".
Comment 8 mkaneda 2013-08-13 23:51:56 PDT
Ikezoe-san, 

I have verified that the patch works very well.

Thank you so much.
Comment 9 Mark Banner (:standard8) 2013-08-15 14:42:10 PDT
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 neil@parkwaycc.co.uk 2013-08-29 07:12:48 PDT
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) 2013-08-29 19:00:26 PDT
Archaeopteryx, could you please push the patch to try server with xpcshell test?

Thank you.
Comment 12 Sebastian Hengst [:aryx][:archaeopteryx] 2013-08-30 00:10:40 PDT
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) 2013-09-01 16:31:56 PDT
Thanks Archaeopteryx!
Comment 14 Ryan VanderMeulen [:RyanVM] 2013-09-01 19:04:08 PDT
https://hg.mozilla.org/comm-central/rev/ff2e9fbb1d73
Comment 15 Hiroyuki Ikezoe (:hiro) 2013-09-02 16:37:26 PDT
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) 2013-09-08 11:31:33 PDT
Standard8, is this patch safe for 24? Data-loss in worst case [imho] warrants getting it in before our cutoff.

Note You need to log in before you can comment on or make changes to this bug.