154000 - Can't move non-ascii subfolder on IMAP

Status: VERIFIED FIXED

(MailNews Core :: Internationalization, defect)

Description

[Jeesun Cho]

Can't move non-ascii subfolder under special folders on IMAP
Platform: JA windows XP
Input: JA chars

Steps:
1. On IMAP account, create a non-ascii folder and subfolder under it.
2. Move them (both at the same time by highlighing the 1st level folder) by
dragging and dropping them to Sent or Drafts folder.
3. Both will appear under Sent folder for example, but when you click on the 2nd
level folder, you'll get an error: The mail server responded: Mailbox does not exist

English folders work fine.

Comment 1

[Jeesun Cho]

This seems related to bug 153982

Comment 2

[marina]

btw: this error can be reproduced with any folder on the IMAP tree (not only
with special folders). Try to drag the newly created non-ascii subfolder to the
top of the tree ([e-mail]) or to any ascii/non-ascii folder of the
first level: you'll get the same error- seems that the copy/move of the
non-ascii subfolder is not allowed.

Comment 3

[Jeesun Cho]

You're right, Marina. Change the summury accordingly.

Comment 4

[marina]

changing qa contact.Thanks!

Comment 5

[nhottanscp]

This is dataloss. Is there anyway to recover the data?
Nominating for nsbeta1. Reassign to naving, cc to putterman

Comment 6

[nhottanscp]

I can reproduce this with Latin1 characters.

parent folder "ссс", subfolder "ррр".

Comment 7

[nhottanscp]

1) In nsImapMailFolder::RenameSubFolders, 
http://lxr.mozilla.org/seamonkey/source/mailnews/imap/src/nsImapMailFolder.cpp#6905

6905      rv = AddSubfolderWithPath(&utf7LeafName, dbFileSpec,
getter_AddRefs(child));
The argument utf7LeafName contains non UTF-7 after the call. Then the following
call to append to CString fails because utf7LeafName contains non ASCII.

2) In nsImapMailFolder::AddSubfolderWithPath,
http://lxr.mozilla.org/seamonkey/source/mailnews/imap/src/nsImapMailFolder.cpp#324
323   uri.Append(*name);
324   char* uriStr = ToNewCString(uri);

this doesn't seem to be right since 'name' may be non ASCII

I got assersions for both places with non ASCII subfolder name.

Naving, could you take a look at this?

Comment 8

[nhottanscp]

>I can reproduce this with Latin1 characters.
>parent folder "ссс", subfolder "ррр".
Parent folder does not have to be non ASCII, subfolder has to be non ASCII.

Comment 9

[marina]

this is an important intl functionality, it is not a regression from 6.2 (is
reproducable in the shipped versoins)

Comment 10

[nhottanscp]

Attachment: Convert subfolder to UTF-7.

The dataloss problem is fixed with this patch. But I still see the assersion
for AddSubfolderWithPath using ToNewCstring.

Comment 11

[Frank Tang]

I suggest we take this as nsbeta1+ and mark it as [adt1] since this could be a
huge data loss (the loss of a whole folder of messages) . Since I am not the
engineer manager of naving, I will leave that to his manager to mark that. 

Comment 12

[Frank Tang]

N6.2 have this problem too. So this is not a regression frrom 6.2 to 7.0

Comment 13

[Navin Gupta]

accepting. working on a fix.

Comment 14

[scottputterman]

If they are really losing their folder then this is a pretty critical bug.

Comment 15

[nhottanscp]

Scott, please also look at bug 153982, similar problem also happens for POP.

Comment 16

[Navin Gupta]

Attachment: proposed fix

I made some changes to nhotta's last patch. We need to use modutf7 for online
name.
Regarding assertion the problem is in AddSubfolderWithPath where it does
ToNewCString. It should actually convert name to modutf7 when constructing
uriStr. I think this patch is ok to fix this bug.

Comment 17

[Navin Gupta]

Cavin, David, Can I get reviews ? thx.

Comment 18

[Navin Gupta]

I think rdf uri's have to be in utf8 format so we cannot do what I suggested to
fix the assertion. 

Comment 19

[Cavin Song]

Comment on attachment 89115 [details] [diff] [review]
proposed fix

r=cavin.

Comment 20

[Navin Gupta]

This fixes the assertion for me in AddSubfolderWithPath. still investigating if
this is correct

@@ -321,7 +321,7 @@
   uri.Append(PRUnichar('/'));
 
   uri.Append(*name);
-  char* uriStr = ToNewCString(uri);
+  char* uriStr = nsCRT::strdup(NS_ConvertUCS2toUTF8(uri.get()).get());
   if (uriStr == nsnull) 
     return NS_ERROR_OUT_OF_MEMORY;
 

Comment 21

[Navin Gupta]

Attachment: complete patch fixing assertion

Comment 22

[David :Bienvenu]

Comment on attachment 89115 [details] [diff] [review]
proposed fix

sr=bienvenu

Comment 23

[Navin Gupta]

fixed on trunk

Comment 24

[Jaime Rodriguez, Jr.]

jeesun - cna you pls verify this fix on the trunk, then  mark it as resolved?
thanks!

Comment 25

[Jeesun Cho]

I'll pick up tomorrow's trunk build and verify this.

Comment 26

[Jeesun Cho]

Reopen this. Try this slightly different scenario. I'm still experiencing data
loss. See the attached screenshot.
1. Create a non-ascii folder and a subfolder under it (ゆき、　ゆき２）
2. Move both to a 1st level folder (ifolder)
3. Then create ゆき３ under ゆき２
4. Notice the tree structure looks weird (The original ゆき２ is grayed-out and
the same folder was created again. See the screenshot.)
5. Move the whole ゆき１，２，３　folders back to the top of the tree, out of
ifolder
6. ゆき３ disappeared

Comment 27

[Jeesun Cho]

Attachment: non-ascii folder screenshot

Please see this page in Shift_JIS

Comment 28

[nhottanscp]

Please create a reproducible test case for your comment #26, so everyone can
test/debug.

Comment 29

[nhottanscp]

Jeesun, plesae use Latin1 characters (e.g. á) for your test case (when the
problem is generic for non ASCII characters), so pepole who do not have Japanese
system support can reproduce and debug, thanks.

Comment 30

[Jeesun Cho]

Sure! This is reproducible using latin-1 chars too. Just use latin-1 chars where
I mention JA chars.
My intention was to give you an example to describe the problem and reproducible
steps in more detail. 

Comment 31

[Navin Gupta]

I don't know what is latin-1 and what is japanese. Can you give a list of
reproducible steps with folder names in latin-1.

Comment 32

[Jeesun Cho]

1. Create a latin-1 folder, for example, tàmü and a subfolder, tàmü2 under it 
2. Move both to a 1st level folder, say its name is ifolder.
3. Then create yet another subfolder, tàmü3 under tàmü2
4. Notice the tree structure looks weird (The original tàmü2 is grayed-out and
the same folder was created again.)
5. Move the whole tàmü, tàmü2, tàmü3 folders back to the top of the tree, out of
ifolder
6. tàmü3 disappeared

Comment 33

[Jaime Rodriguez, Jr.]

removing the nomination because this bug has been reopened.

Comment 34

[Navin Gupta]

working on a fix. 

Comment 35

[Navin Gupta]

I have a fix for problem described in comment #32. But I am seeing other
problems where LSUB command doesn't tell us all the children so when we
*restart* tàmü3 disappears. This is relevant portion of the imap log for the
last rename command
when doing comment #32. Remember we do lsub command before we do rename. Relevant
code to look at is RenameHierarchyByHand in nsImapProtocol. David, what do you 
think ?

2512[4ffa200]: nsmail-2:S-ifolder/t&AOA-m&APw-:SendData: 6 lsub ""
"ifolder/t&AOA-m&APw-/*"

2512[4ffa200]: nsmail-2:S-ifolder/t&AOA-m&APw-:CreateNewLineFromSocket: * LSUB
(\HasNoChildren) "/" ifolder/t&AOA-m&APw-/t&AOA-m&APw-1/t&AOA-m&APw-2

2512[4ffa200]: nsmail-2:S-ifolder/t&AOA-m&APw-:CreateNewLineFromSocket: 6 OK
Completed

2512[4ffa200]: nsmail-2:S-ifolder/t&AOA-m&APw-:SendData: 7 close

2512[4ffa200]: nsmail-2:S-ifolder/t&AOA-m&APw-:CreateNewLineFromSocket: 7 OK
Completed

2512[4ffa200]: nsmail-2:A:SendData: 8 rename "ifolder/t&AOA-m&APw-" "t&AOA-m&APw-"

2512[4ffa200]: nsmail-2:A:CreateNewLineFromSocket: 8 OK Completed

2512[4ffa200]: nsmail-2:A:SendData: 9 subscribe "t&AOA-m&APw-"

2512[4ffa200]: nsmail-2:A:CreateNewLineFromSocket: 9 OK Completed

2512[4ffa200]: nsmail-2:A:SendData: 10 unsubscribe "ifolder/t&AOA-m&APw-"

2512[4ffa200]: nsmail-2:A:CreateNewLineFromSocket: 10 OK Completed

2512[4ffa200]: nsmail-2:A:SendData: 11 subscribe
"t&AOA-m&APw-w-/t&-AOA-m&-APw-1/t&-AOA-m&-APw-2"

2512[4ffa200]: nsmail-2:A:CreateNewLineFromSocket: 11 NO Mailbox does not exist

2512[4ffa200]: nsmail-2:A:SendData: 12 unsubscribe
"ifolder/t&-AOA-m&-APw-/t&-AOA-m&-APw-1/t&-AOA-m&-APw-2"

2512[4ffa200]: nsmail-2:A:CreateNewLineFromSocket: 12 OK Completed

2716[50e0c50]: nsmail-2:NA:CreateNewLineFromSocket: * OK dredd.mcom.com IMAP4
service (Netscape Messaging Server 4.15 Patch 4 (built Dec  7 2000))

2716[50e0c50]: nsmail-2:NA:SendData: 1 authenticate plain

2716[50e0c50]: nsmail-2:NA:CreateNewLineFromSocket: + 

2716[50e0c50]: nsmail-2:NA:SendData: Logging suppressed for this command (it
probably contained authentication information)
2716[50e0c50]: nsmail-2:NA:CreateNewLineFromSocket: 1 OK User logged in (no
protection)

2716[50e0c50]: nsmail-2:A:SendData: 2 XSERVERINFO MANAGEACCOUNTURL
MANAGELISTSURL MANAGEFILTERSURL

2716[50e0c50]: nsmail-2:A:CreateNewLineFromSocket: * XSERVERINFO
MANAGEACCOUNTURL {60}

2716[50e0c50]: nsmail-2:A:CreateNewLineFromSocket:
http://[e-mail]:4444/bin/user/admin/bin/enduser MANAGELISTSURL NIL
MANAGEFILTERSURL NIL

2716[50e0c50]: nsmail-2:A:CreateNewLineFromSocket: 2 OK Completed

Comment 36

[Navin Gupta]

If I try to break up the hierarchy and subscribe for each children folder then 
also it doesn't fix it. cc jgmyers for input as well. 

3040[51627b0]: nsmail-2:S-ifolder/t&AOA-m&APw-:SendData: 6 
lsub "" "ifolder/t&AOA-m&APw-/*"

3040[51627b0]: nsmail-2:S-ifolder/t&AOA-m&APw-:CreateNewLineFromSocket: * LSUB 
(\HasNoChildren) "/" ifolder/t&AOA-m&APw-/t&AOA-m&APw-2/t&AOA-m&APw-3

3040[51627b0]: nsmail-2:S-ifolder/t&AOA-m&APw-:CreateNewLineFromSocket: 6 OK 
Completed

3040[51627b0]: nsmail-2:S-ifolder/t&AOA-m&APw-:SendData: 7 close

3040[51627b0]: nsmail-2:S-ifolder/t&AOA-m&APw-:CreateNewLineFromSocket: 7 OK 
Completed

3040[51627b0]: nsmail-2:A:SendData: 8 rename "ifolder/t&AOA-m&APw-" "t&AOA-
m&APw-"

3040[51627b0]: nsmail-2:A:CreateNewLineFromSocket: 8 OK Completed

3040[51627b0]: nsmail-2:A:SendData: 9 subscribe "t&AOA-m&APw-"

3040[51627b0]: nsmail-2:A:CreateNewLineFromSocket: 9 OK Completed

3040[51627b0]: nsmail-2:A:SendData: 10 unsubscribe "ifolder/t&AOA-m&APw-"

3040[51627b0]: nsmail-2:A:CreateNewLineFromSocket: 10 OK Completed

3040[51627b0]: nsmail-2:A:SendData: 11 subscribe "t&AOA-m&APw-w-/t&-AOA-m&-APw-
2"

3040[51627b0]: nsmail-2:A:CreateNewLineFromSocket: 11 NO Mailbox does not exist

3040[51627b0]: nsmail-2:A:SendData: 12 unsubscribe "ifolder/t&-AOA-m&-APw-/t&-
AOA-m&-APw-2"

3040[51627b0]: nsmail-2:A:CreateNewLineFromSocket: 12 OK Completed

3040[51627b0]: nsmail-2:A:SendData: 13 subscribe "t&AOA-m&APw-w-/t&-AOA-m&-APw-
2/t&-AOA-m&-APw-3"

3040[51627b0]: nsmail-2:A:CreateNewLineFromSocket: 13 NO Mailbox does not exist

3040[51627b0]: nsmail-2:A:SendData: 14 unsubscribe "ifolder/t&-AOA-m&-APw-/t&-
AOA-m&-APw-2/t&-AOA-m&-APw-3"

3040[51627b0]: nsmail-2:A:CreateNewLineFromSocket: 14 OK Completed

2624[518f7b0]: nsmail-2:A:SendData: 12 select "t&AOA-m&APw-"

2624[518f7b0]: nsmail-2:A:CreateNewLineFromSocket: * FLAGS (\Answered \Flagged 
\Draft \Deleted \Seen)

2624[518f7b0]: nsmail-2:A:CreateNewLineFromSocket: * OK [PERMANENTFLAGS 
(\Answered \Flagged \Draft \Deleted \Seen \*)]  

2624[518f7b0]: nsmail-2:A:CreateNewLineFromSocket: * 0 EXISTS

2624[518f7b0]: nsmail-2:A:CreateNewLineFromSocket: * 0 RECENT

2624[518f7b0]: nsmail-2:A:CreateNewLineFromSocket: * OK [UIDVALIDITY 
1025145230]  

2624[518f7b0]: nsmail-2:A:CreateNewLineFromSocket: 12 OK [READ-WRITE] Completed

Comment 37

[Navin Gupta]

Attachment: patch to fix the case in comment #32

This patch fixes the bug but there are some problems as described in my earlier

comments. AddSubfolderWithPath should actually take char* instead of
nsAutoString if we want to avoid converting back and forth from unicode to utf7

and vice-versa. But we can do that in a separate bug.

Comment 38

[David :Bienvenu]

this subscribe failed:
2512[4ffa200]: nsmail-2:A:SendData: 11 subscribe
"t&AOA-m&APw-w-/t&-AOA-m&-APw-1/t&-AOA-m&-APw-2"

2512[4ffa200]: nsmail-2:A:CreateNewLineFromSocket: 11 NO Mailbox does not exist

and here:
3040[51627b0]: nsmail-2:A:SendData: 11 subscribe "t&AOA-m&APw-w-/t&-AOA-m&-APw-
2"

3040[51627b0]: nsmail-2:A:CreateNewLineFromSocket: 11 NO Mailbox does not exist

presumably, that's the cause of the problem. I can't tell if your patch fixes
that problem or not.

Comment 39

[Navin Gupta]

The last patch doesn't fix the problem of tàmü3 disappearing on restart. 

>This patch fixes the bug but there are some problems as described in my earlier
>comments.

Actually if I try the same thing with ascii folder names it works fine. So this 
problem (tàmü3 disappearing on restart) appears to be a *server issue* because
the server doesn't tell us the children correctly on doing lsub command. 
For a comparison please look at comment #35 and this comment, thx. 


2624[4c02a10]: nsmail-2:S-ifolder/A:SendData: 7 lsub "" "ifolder/A/*"

2624[4c02a10]: nsmail-2:S-ifolder/A:CreateNewLineFromSocket: * LSUB 
(\HasChildren) "/" ifolder/A/B

2624[4c02a10]: nsmail-2:S-ifolder/A:CreateNewLineFromSocket: * LSUB 
(\HasNoChildren) "/" ifolder/A/B/C

2624[4c02a10]: nsmail-2:S-ifolder/A:CreateNewLineFromSocket: 7 OK Completed

2624[4c02a10]: nsmail-2:S-ifolder/A:SendData: 8 close

2624[4c02a10]: nsmail-2:S-ifolder/A:CreateNewLineFromSocket: 8 OK Completed

2624[4c02a10]: nsmail-2:A:SendData: 9 rename "ifolder/A" "A"

2624[4c02a10]: nsmail-2:A:CreateNewLineFromSocket: 9 OK Completed

2624[4c02a10]: nsmail-2:A:SendData: 10 subscribe "A"

2624[4c02a10]: nsmail-2:A:CreateNewLineFromSocket: 10 OK Completed

2624[4c02a10]: nsmail-2:A:SendData: 11 unsubscribe "ifolder/A"

2624[4c02a10]: nsmail-2:A:CreateNewLineFromSocket: 11 OK Completed

2624[4c02a10]: nsmail-2:A:SendData: 12 subscribe "A/B"

2624[4c02a10]: nsmail-2:A:CreateNewLineFromSocket: 12 OK Completed

2624[4c02a10]: nsmail-2:A:SendData: 13 unsubscribe "ifolder/A/B"

2624[4c02a10]: nsmail-2:A:CreateNewLineFromSocket: 13 OK Completed

2624[4c02a10]: nsmail-2:A:SendData: 14 subscribe "A/B/C"

2624[4c02a10]: nsmail-2:A:CreateNewLineFromSocket: 14 OK Completed

2624[4c02a10]: nsmail-2:A:SendData: 15 unsubscribe "ifolder/A/B/C"

2624[4c02a10]: nsmail-2:A:CreateNewLineFromSocket: 15 OK Completed

2980[4d68730]: nsmail-2:NA:CreateNewLineFromSocket: * OK dredd.mcom.com IMAP4 
service (Netscape Messaging Server 4.15 Patch 4 (built Dec  7 2000))

2980[4d68730]: nsmail-2:NA:SendData: 1 authenticate plain

Comment 40

[nhottanscp]

Cc to taka for the server problem.

Comment 41

[David :Bienvenu]

Navin, did you look in the subscribe UI and see if your subscribe was
successful? It looks to me from the log that it failed.

Comment 42

[John G. Myers]

From comment 35:

2512[4ffa200]: nsmail-2:A:SendData: 11 subscribe
"t&AOA-m&APw-w-/t&-AOA-m&-APw-1/t&-AOA-m&-APw-2"

This appears to have been garbled by the client.  Shouldn't it instead be:

11 subscribe "t&AOA-m&APw-w-/t&-AOA-m&-APw-1/t&-AOA-m&APw-2"

to match what the server reported in the previous LSUB response?

Comment 43

[Navin Gupta]

yes, subscribe UI failed. Good point, jgmyers. 

Comment 44

[David :Bienvenu]

a point that I tried to make twice :-(

Comment 45

[Navin Gupta]

The difference was so subtle I couldn't make out (until it was explicitly 
pointed out). Sorry about that!

patch upcoming.

Comment 46

[David :Bienvenu]

sorry, I should have been more clear - the subscribe should not fail, and the
reason it failed was that we were subscribing to a folder that didn't exist
because the name was wrong - but that imap mod-utf7 is a bear to read.

Comment 47

[Takayuki Tei]

The cause of garbled folder name could be in charset convertor.
Just my guess.

Comment 48

[Navin Gupta]

Attachment: proposed fix

There are two parts to the fix. One is to pass unicode string converted from
mod-utf7 to AddSubfolderWithPath. The next part is in imapProtocol code where
the server is giving us names in modutf7 so we don't need to convert the server
name again to mod-utf7. That was causing subscribe to fail.

Cavin, David, Can I get reviews ? thx.

Comment 49

[David :Bienvenu]

Comment on attachment 89444 [details] [diff] [review]
proposed fix

sr=bienvenu - however, this does require a lot of testing of mod-utf7 folder
names (e.g., A&B), creating, renaming, deleting, subscribe/unsubscribing, etc.

Comment 50

[Navin Gupta]

This code change will just affect renaming/deleting/moving imap folders and 
that is all it would affect. I have tested these three, looks ok. 

Comment 51

[Navin Gupta]

*** Bug 154446 has been marked as a duplicate of this bug. ***

Comment 52

[Cavin Song]

Comment on attachment 89444 [details] [diff] [review]
proposed fix

r=cavin.

Comment 53

[Takayuki Tei]

If I am reading comments correctly, there is a bug in charset
convertor from Modified UTF-7 to UCS-2.  We should double check
the convertor works okay and open a new bug against it if it doesn't.

Comment 54

[Navin Gupta]

fixed on trunk. 

Comment 55

[David :Bienvenu]

yes, it does seem like the round trip might not quite be working from imap
mod-utf7 -> ucs2 -> imap mod-utf7, unless somewhere we're just converting to
utf7 and not imap mod-utf7

Comment 56

[Navin Gupta]

we were passing modutf7 to CreateUtf7ConvertedString when it was expecting 8 
bit ascii, so I think it is ok now unless we have some other places doing the 
same thing. 

Comment 57

[Jeesun Cho]

Verified on 0628 trunk build.

Comment 58

[Jaime Rodriguez, Jr.]

adt1.0.1+ (on ADT's behalf) approval for checkin to the 1.0 branch. pls check
this in asap, then add the "fixed1.0.1" keyword.

Comment 59

[chris hofmann]

Comment on attachment 89444 [details] [diff] [review]
proposed fix

a=chofmann for 1.0.1 add the fixed1.0.1 keyword after checking in to the
branch.

Comment 60

[Navin Gupta]

fixed on branch.

Comment 61

[Jeesun Cho]

Verified on branch build.