24692 - Message is lost after copying to a non-ascii folder name

Status: VERIFIED FIXED

(MailNews Core :: Internationalization, defect, P3)

Description

[marina]

Steps to reproduce:
-go to Yahoo.fr;
-send this page to your account;
-get message, verify mail body is correct;
-now copy this message to two folders (one with ascii name and another one with 
non-ascii name);
//note: message copyed to ascii folder displays all comtents fine;
after selecting message in non-ascii folder there is no body

Comment 1

[marina]

i forgot to mention that no problem occur when moving messages.

Comment 2

[Katsuhiko Momoi]

QA assigned to marina.

Comment 3

[nhottanscp]

Questions:
1) Is this IMAP or local?
2) Is the folder name in non-ascii displayed fine?

Comment 4

[marina]

it is a local folder and non-ascii name is displyed fine.

Comment 5

[nhottanscp]

Reassign to jefft. I think he owns this part.

Comment 6

[Phil Peterson]

Nominate as beta1 stopper

Comment 7

[leger]

Putting on on PDT+ radar for beta1.

Comment 8

[jefft]

David, is this yours? We are having problem locating non-ascii folder database.

02/10/00  01:57p        <DIR>          .
02/10/00  01:57p        <DIR>          ..
02/10/00  01:57p                     0 Aחזה
02/10/00  01:57p                 1,049 Aחזה.msf
02/09/00  01:29p                     0 Drafts
02/10/00  11:04a                 1,133 Drafts.msf
02/10/00  01:35p             1,099,685 Inbox
02/10/00  01:52p                29,954 Inbox.msf
02/10/00  11:04a                   960 popstate.dat
02/10/00  11:03a                     0 rules.dat
02/09/00  01:29p                     0 Sent
02/09/00  01:29p                     0 Templates
02/10/00  01:14p                 8,123 Trash
02/10/00  01:37p                 1,630 Trash.msf
02/09/00  01:29p                     0 Unsent Messages
02/10/00  01:52p                 1,133 Unsent Messages.msf
              16 File(s)      1,143,667 bytes
                          1,240,010,752 bytes free

F:\JeffTsai\work\qatest21\Mail\server1>

+	dbHeap	0x00000000
+	dbName	0x0c2d2788 "F:\JeffTsai\work\qatest21\Mail\server1\Aח.sbd\ה.msf"
+	m_mdbEnv	0x0c2d2518
+	myMDBFactory	0x0aebce98
+	&newFile	0x0012c804
+	newFile	0x00000000
	ret	2153054214
+	this	0x0c2d2900


NTDLL! 77f7629c()
nsDebug::Assertion(const char * 0x0b42cb28, const char * 0x0b42ca34, const char 
* 0x0b42ca0c, int 78) line 189 + 13 bytes
mork_assertion_signal(const char * 0x0b42cb28) line 78 + 31 bytes
morkEnv::NewError(const char * 0x0b42cb6c) line 369 + 19 bytes
morkEnv::CantMakeWhenBadError() line 404
morkHandle::morkHandle(morkEnv * 0x0c2d25d0, morkHandleFace * 0x0c2d2250, 
morkObject * 0x0c2d2430, unsigned long 1181314149) line 101
orkinFile::orkinFile(morkEnv * 0x0c2d25d0, morkHandleFace * 0x0c2d2250, morkFile 
* 0x0c2d2430) line 67 + 32 bytes
orkinFile::MakeFile(morkEnv * 0x0c2d25d0, morkFile * 0x0c2d2430) line 81 + 43 
bytes
morkFile::AcquireFileHandle(morkEnv * 0x0c2d25d0) line 144 + 13 bytes
orkinFactory::CreateNewFile(nsIMdbEnv * 0x0c2d2518, nsIMdbHeap * 0x0aebcfa8, 
const char * 0x0c2d2788, nsIMdbFile * * 0x0012c804) line 350 + 12 bytes
nsMsgDatabase::OpenMDB(nsMsgDatabase * const 0x0c2d2900, const char * 
0x0c2d2788, int 1) line 626 + 30 bytes
nsMailDatabase::Open(nsMailDatabase * const 0x0c2d2b20, nsIFileSpec * 
0x0c495d00, int 1, int 0, nsIMsgDatabase * * 0x0c491454) line 89 + 29 bytes
nsMsgLocalMailFolder::GetDatabase(nsIMsgWindow * 0x0be92650) line 463 + 66 bytes
nsMsgLocalMailFolder::UpdateFolder(nsMsgLocalMailFolder * const 0x0c4913e8, 
nsIMsgWindow * 0x0be92650) line 509 + 19 bytes
XPTC_InvokeByIndex(nsISupports * 0x0c4913e8, unsigned int 32, unsigned int 1, 
nsXPTCVariant * 0x0012cb28) line 139
nsXPCWrappedNativeClass::CallWrappedMethod(JSContext * 0x01d9c090, 
nsXPCWrappedNative * 0x0c495360, const XPCNativeMemberDescriptor * 0x0b8d94fc, 
nsXPCWrappedNativeClass::CallMode CALL_METHOD, unsigned int 1, long * 
0x00f008d0, long * 0x0012cce8) line 898 + 43 bytes
WrappedNative_CallMethod(JSContext * 0x01d9c090, JSObject * 0x0b831890, unsigned 
int 1, long * 0x00f008d0, long * 0x0012cce8) line 200 + 34 bytes
js_Invoke(JSContext * 0x01d9c090, unsigned int 1, unsigned int 0) line 665 + 26 
bytes
js_Interpret(JSContext * 0x01d9c090, long * 0x0012d5d4) line 2292 + 15 bytes
js_Invoke(JSContext * 0x01d9c090, unsigned int 3, unsigned int 0) line 681 + 13 
bytes
js_Interpret(JSContext * 0x01d9c090, long * 0x0012de7c) line 2292 + 15 bytes
js_Invoke(JSContext * 0x01d9c090, unsigned int 1, unsigned int 0) line 681 + 13 
bytes
js_Interpret(JSContext * 0x01d9c090, long * 0x0012e724) line 2292 + 15 bytes
js_Invoke(JSContext * 0x01d9c090, unsigned int 0, unsigned int 0) line 681 + 13 
bytes
js_Interpret(JSContext * 0x01d9c090, long * 0x0012efcc) line 2292 + 15 bytes
js_Invoke(JSContext * 0x01d9c090, unsigned int 1, unsigned int 2) line 681 + 13 
bytes
js_InternalInvoke(JSContext * 0x01d9c090, JSObject * 0x0b8c15d0, long 193733968, 
unsigned int 0, unsigned int 1, long * 0x0012f158, long * 0x0012f104) line 754 + 
19 bytes
JS_CallFunctionValue(JSContext * 0x01d9c090, JSObject * 0x0b8c15d0, long 
193733968, unsigned int 1, long * 0x0012f158, long * 0x0012f104) line 2772 + 31 
bytes
nsJSContext::CallEventHandler(nsJSContext * const 0x01d9ed60, void * 0x0b8c15d0, 
void * 0x0b8c2550, unsigned int 1, void * 0x0012f158, int * 0x0012f154) line 562 
+ 33 bytes
nsJSEventListener::HandleEvent(nsIDOMEvent * 0x0c2d3594) line 128 + 57 bytes
nsEventListenerManager::HandleEventSubType(nsListenerStruct * 0x0bcd5aa0, 
nsIDOMEvent * 0x0c2d3594, unsigned int 8) line 677 + 19 bytes
nsEventListenerManager::HandleEvent(nsIPresContext * 0x01d9eb70, nsEvent * 
0x0012f6b0, nsIDOMEvent * * 0x0012f650, unsigned int 7, nsEventStatus * 
0x0012f6d0) line 1171 + 31 bytes
nsXULElement::HandleDOMEvent(nsXULElement * const 0x0bbf0720, nsIPresContext * 
0x01d9eb70, nsEvent * 0x0012f6b0, nsIDOMEvent * * 0x0012f650, unsigned int 1, 
nsEventStatus * 0x0012f6d0) line 2928
nsXULTreeElement::FireOnSelectHandler(nsXULTreeElement * const 0x0beb471c) line 
556
nsXULTreeElement::SelectCell(nsXULTreeElement * const 0x0beb4718, 
nsIDOMXULElement * 0x0c2791b4) line 207
nsTreeFrame::SetSelection(nsIPresContext * 0x01d9eb70, nsTreeCellFrame * 
0x0b90d4b4) line 130
nsTreeCellFrame::HandleMouseDownEvent(nsIPresContext * 0x01d9eb70, nsGUIEvent * 
0x0012fb14, nsEventStatus * 0x0012fa20) line 246
nsTreeCellFrame::HandleEvent(nsTreeCellFrame * const 0x0b90d4b4, nsIPresContext 
* 0x01d9eb70, nsGUIEvent * 0x0012fb14, nsEventStatus * 0x0012fa20) line 199
PresShell::HandleEvent(PresShell * const 0x0af523a4, nsIView * 0x0bd75310, 
nsGUIEvent * 0x0012fb14, nsEventStatus * 0x0012fa20) line 2875 + 38 bytes
nsView::HandleEvent(nsView * const 0x0bd75310, nsGUIEvent * 0x0012fb14, unsigned 
int 8, nsEventStatus * 0x0012fa20, int & 0) line 799
nsView::HandleEvent(nsView * const 0x0af52860, nsGUIEvent * 0x0012fb14, unsigned 
int 28, nsEventStatus * 0x0012fa20, int & 0) line 784
nsViewManager::DispatchEvent(nsViewManager * const 0x0af51040, nsGUIEvent * 
0x0012fb14, nsEventStatus * 0x0012fa20) line 1704
HandleEvent(nsGUIEvent * 0x0012fb14) line 69
nsWindow::DispatchEvent(nsWindow * const 0x0af52744, nsGUIEvent * 0x0012fb14, 
nsEventStatus & nsEventStatus_eIgnore) line 493 + 10 bytes
nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012fb14) line 514
nsWindow::DispatchMouseEvent(unsigned int 302, nsPoint * 0x00000000) line 3038 + 
21 bytes
ChildWindow::DispatchMouseEvent(unsigned int 302, nsPoint * 0x00000000) line 
3256
nsWindow::ProcessMessage(unsigned int 513, unsigned int 1, long 13631562, long * 
0x0012fda0) line 2335 + 24 bytes
nsWindow::WindowProc(HWND__ * 0x58f20222, unsigned int 513, unsigned int 1, long 
13631562) line 673 + 27 bytes
USER32! 77e71820()
00d0004a()

Comment 9

[David :Bienvenu]

accepting, but it'll have to wait until I finish the AOL IMAP stuff, I believe.

Comment 10

[David :Bienvenu]

No, this is not me. I think the first part about the message getting lost is 
probably you, Jeff, not catching the error trying to copy to the destination 
folder (I haven't debugged that part, however). The assertion opening the db is 
because the mPath that we create in nsMsgFolder::parseURI(PRBool needServer) is 
bogus, perhaps because file spec is rejecting the illegal characters. I stepped 
through the following loop

    while (token) {
      // skip leading '/' (and other // style things)
      if (nsCRT::strcmp(token, "")!=0) {

        // add .sbd onto the previous path
        if (haveFirst) {
          newPath+=".sbd";
          newPath += "/";
        }
        
        hashedToken = token;
        NS_MsgHashIfNecessary(hashedToken);
        newPath += hashedToken;
        haveFirst=PR_TRUE;
      }
      
      token = nsCRT::strtok(newStr, "/", &newStr);
    }

and the newPath created is garbage. I know Scott has been looking at similar 
problems, especially with regard to double byte chars, so I'm cc'ing him.

Comment 11

[scottputterman]

cc'ing Alec.

I'm not sure what to do about this.  Perhaps GetPath shouldn't be based on 
parsing the uri.  Or perhaps as Alec has mentioned we need to get the uri 
parsing code to not eliminate whitespace.

Comment 12

[Alec Flett]

ugh. 
What if we nsEscaped the rest of the URI before we passed it to SetSpec?

Comment 13

[David :Bienvenu]

I think we need to do something like utf-7 encode the folder name in the uri, 
and the db name, and set the online name for display purposes only. It's a scary 
change, however.

Comment 14

[David :Bienvenu]

Attachment: patch for fix

Comment 15

[David :Bienvenu]

OK, I found the place where we're not checking an error code in the msg/copy 
code, so I should be able to prevent the data loss. I'll attached that diff. 
Also, I looked at 4.x, and it's able to create folders and database files with 
non-ascii names, so I suspect we're doing something evil in the file spec code.

Comment 16

[David :Bienvenu]

the msg is now no longer lost, but non-ascii folder names still don't work, so 
I'm leaving this open.

Comment 17

[David :Bienvenu]

this turns out to be what looks like a bug in nsCRT::strtok - it's not handling 
characters with the high bit set correctly.  The ח in Aחזה looks like '/' to 
nsCRT::strtok, which messes up our parsing routines. I'll probably switch to 
using nsCString and FindChar instead. Should have a fix tomorrow.

Comment 18

[Alec Flett]

if strtok is busted, we should fix it, no?

Comment 19

[David :Bienvenu]

Yes, I think we should. It's used all over the place. I think some of those 
places can have 8 bit data.

Comment 20

[David :Bienvenu]

moving out ETA - after getting around the strtok bug, I've now run into a 
problem with the way we encode uri's - some code expects uri's to be ut8 
(NS_MakeAbsoluteURI uses ut8 encoding) and other code expects uri's to be 
straight 8 bit characters. Perhaps converting to utf7 everywhere would get 
around that, but it's a scary change.

Comment 21

[Warren Harris]

David: What code expects URLs to be straight 8 bit characters? We need to fix 
that. Can you file a separate bug against [e-mail], and cc 
me? Thanks.

Comment 22

[David :Bienvenu]

It's mailnews code that creates url's (and uri's) with 8 bit characters. Should 
we utf-7 encode them, or utf-8 encode them?

Comment 23

[Warren Harris]

URLs are supposed to be UTF8 encoded when handed to necko (although I really 
regret not changing this to PRUnichar* a long time ago).

Comment 24

[David :Bienvenu]

ah, the fun just never stops with this bug. utf8 seems to have the joyous 
property that if you utf8 encode a string that is already utf8-encoded, you get 
a different string. So even if I utf8 encode the local mail folder uri's, I run 
into grief because the xul utf8 encodes the utf8-encoded folder name that it 
gets out of the uri. So, unless I'm more than usually confused, I think utf8 is 
not the way to go here (despite the fact that it's relatively convenient with 
nsStrings and nsTextFormatter to convert back and forth between them). So I 
might be stuck with the relatively expensive utf7 encoding/decoding.

Comment 25

[Warren Harris]

Shouldn't we know at all points in the code whether a char* represents UTF8 or 
ascii? If this is getting confused along the way causing us to UTF8 encode 
twice, we need to fix that, not work around it with a double encoding. But 
perhaps I'm not understanding you...

Comment 26

[David :Bienvenu]

we should, but I don't, at least. Are URI's supposed to be utf8 encoded? If so,
nsXULContentUtils::GetElementRefResource is broken, or the code that stores
uri's is broken; nsXULContentUtils::GetelementRefResource seems to assume that
they're not utf8 encoded. If URI's are NOT supposed to be utf8-encoded, then I
need to come up another way of storing 8 bit data safely so that utf8 encoding
won't touch it.

Comment 27

[Warren Harris]

Users of nsStdURL (which is what I think mail/news uses) are supposed to convert 
their strings to UTF8 before initializing the url (with SetSpec). SetSpec just 
says it takes a char*, but it's supposed to be UTF8. Sounds like this isn't 
happening somewhere. Similarly, callers of GetSpec should assume that they're 
getting UTF8. 

Cc'ing Andreas just in case I've got something wrong here.

Comment 28

[Chris Waterson]

We can certainly change the XUL/RDF stuff to do UTF8 encoding & docoding of 
URLs (although I'm not exactly sure all of the places where we'd need to do 
that). I was under the assumption that all URLs were 7-bit ascii when I wrote 
the code.

Comment 29

[David :Bienvenu]

I changed the local mail code to convert to utf8 before calling setspec and 
convert from utf8 when calling getspec, but that doesn't do any good unless 
everyone does the same. Also, the problem I'm having now is with URI's, not 
URL's. Do you think this utf8 convention applies to URI's as well?

Comment 30

[David :Bienvenu]

To go into a little more detail, rdf_MakeAbsoluteURI(nsIURI* aURL, nsString& 
aURI) calls rdf_MakeAbsoluteURI(nsIURI* aURL, nsString& aURI) which calls 
NS_MakeAbsoluteURI - the first thing that does is take the spec and convert it 
to utf8, so it seems to believe that the spec is NOT utf8 encoded. If this is 
correct, then I can't UTF8 encode URI's, and should utf7 encode them, or 
something.

Comment 31

[Warren Harris]

When I mentioned converting to utf8, I meant at the point where the nsIURI 
methods (particularly SetSpec and Resolve) gets called. Since the 
NS_MakeAbsoluteURI takes an nsString, this string is expected to be in unicode, 
and the conversion to utf8 happens at the point we start needing char* data.

If mail/news and/or rdf only deal with nsStrings, you shouldn't have to worry 
about utf8 conversion.

Comment 32

[David :Bienvenu]

We deal with URI's, The nstring in question came from a URI, so it's not real 
unicode, just a const char * stuck into an nsString. That's because we're using 
GetResource. Perhaps if I use GetUnicodeResource, I'll be happier. I'm going to 
try that.

Comment 33

[Warren Harris]

This is the second case today where I've seen a char* stuck into an nsString and 
bad things happen. I think we need to use a charset converter when we do that. 
If you got the string from a URL then it's supposed to be utf8. Putting utf8 
directly into an nsString will mangle things.

Frank: Do you agree that we need to use a charset converter here?

Comment 34

[Alec Flett]

not sure if this is what you were talking about, but it seems like basically we
should never allow someone to put a char* into an nsString without telling us
what charset it's in. this would force people to do their research before
blindly dropping in the char*.. the nsString could do the charset conversion
automatically.

Comment 35

[Warren Harris]

My comments exactly in bug 28474.

Comment 36

[scottputterman]

i think this is also part of the problem for 7844.  I can get folder names to 
show up, but the uris are incorrect.

Comment 37

[David :Bienvenu]

OK, waterson and I came up with a fix for this - rdf_MakeAbsoluteURI needs to 
stop converting to utf8, basically, and take a const char * instead of an 
nsString. This will save some data allocation and copying as well. I'm going to 
code it up. The equivalent change (converting back to straight unicode from utf8 
)also works, but is inneficient.

Comment 38

[David :Bienvenu]

I have a fix for this - waiting for checkin approval. Won't make tomorrow's 
build because I don't have approval yet, so moving to 2/24

Comment 39

[David :Bienvenu]

fix checked in.

Comment 40

[marina]

can not be verify because of a new bug # 28987

Comment 41

[marina]

verified as such in 2000-02-24 Win build.

Comment 42

[marina]

marked as verified