87110 - Browser is hung when viewing a forwarded chinese mail

Status: VERIFIED FIXED

(MailNews Core :: MIME, defect, P1)

Description

[ji]

This problem is originally reported from one of our customers: [e-mail]
And I can reproduce with the testing mail he sent to us.
Below is the report from him:
Zhang Lu wrote:

  got an mail in Chinese. When click on it, the status bar shows "loading 
program" and my task manager (w2k) shows
  netscape is using 90MB of memmory and total memory goes up to 400MB. have to 
kill the program from task

I'll attach the test mail.
  manager. 

  Zhang

Comment 1

[ji]

Another email from him:
I  tested same mail on Win 98, it also hung there by showing "loading document". 
But I cannot tell how many virtual memory it was using. The machine was very 
slow at that time. 
That was a desktop PIII 800 with 256 MB memory. 

Zhang 

Comment 2

[ji]

Attachment: The testing mail from Zhang

Comment 3

[ji]

Nominating for rtm.

Comment 4

[nhottanscp]

I cannot reproduce. I used NS6.1 PR1 win32 on Window2000 US.
I used the attached data, it was loaded fine (altough the last part was shown as
garbage). Browser did not hung after viewing that message.

Comment 5

[marina]

i tested this mail on Win2K and it is slow but i was able to open it and in the
new window: i didn't hang nor crash, besides some ??? it shows fine.

Comment 6

[nhottanscp]

After Xianglan forwarded the original mail (not the attached one), I can
reproduce it. Cc to putterman, ducaroz, this could be non i18n (Xianglan will
forward that mail to them).

Comment 7

[nhottanscp]

Attachment: Callstack, used the debugger 'break'.

Comment 8

[nhottanscp]

Reassign to putterman, does not seem to be i18n specific (please see the
attached callstack), cc to mscott.

Comment 9

[scottputterman]

reassigning to ducarroz.  I also freeze with the message.  Putting on nsbranch list.

Comment 10

[scottputterman]

adding the milestone as well.

Comment 11

[scottputterman]

adding PDT+

Comment 12

[Marek Z. Jeziorek]

Jean-Francois, this is PDT+ bug. Tomorrow, on Tuesday, we'll try to build the
first RTM candidate. It would be good, if this could be resolved ASAP.

Comment 13

[Jean-Francois Ducarroz]

Accepting and looking at it...

Comment 14

[varada]

Incidental information.
I copied the message to a local folder and then changed the charset of the
attached chinese message to GB2312,GBK,Big5 and in all three cases they seemed
to load properly(the first two seemed to have decoded more of the message than
the last charset). When I tried using the US-ASCII that was the existing charset
in the message I went back to the freeze.

Comment 15

[Jean-Francois Ducarroz]

I am able to reproduce it on Mac. I put a printf in mime_display_stream_write.
For some reason, despite we are writing each time about 1533 bytes, the interval
between calls become exponentialy longer, and stop after the 27th. After the
27th call, I get "WARNING: Found that temp mem is low..." evey n secondes(n also
become longer each time).

Comment 16

[ji]

(To answer PDT's question) Although the problem is only reproducible with a 
forwarded Chinese mail so far, from attached callstack it doesn't seem to be 
i18n specific.( Please see nhotta's comments on 2001-06-21 10:45). It reveals 
a flaw of attachment handling. And the symptom of the bug is very bad. It eats a 
lot of memories and the program is hung, the user has to kill it from task.

Comment 17

[lchiang]

JF - Added "No ETA" to the Status Whiteboard.

Do you have an ETA (since we'd like this fix checked in as soon as you have r,
sr)? If not, do you need help reproducing, diagnosing or fixing?

Comment 18

[Jean-Francois Ducarroz]

I am still looking at it and so far I still no understand what going wrong. 
Therefore no ETA yet!

Comment 19

[scottputterman]

We're not having much luck figuring this out.  How likely is it to get a message
like this?  One thing I would like to know is what was the original mailer. The
other strange this is that it's a Chinese mail sent with a US Ascii charset.  Is
that going to be common?  

Comment 20

[Jean-Francois Ducarroz]

After playing with message for a while and debugging the problem on both Mac and Windows, I have discovered that 
if at the last stage of mime, instead of passing the data to the browser, I just eat it, everything is fine (except that 
we are not displaying the message anymore). Therefore, I thing the problem if more into the parser than into mime.

I'll try to generate an html file with the mime output to backup this early conclusion...

Comment 21

[Jean-Francois Ducarroz]

Attachment: html data generated by mime

Comment 22

[Jean-Francois Ducarroz]

Attachment: Stack trace (mac)

Comment 23

[Jean-Francois Ducarroz]

When I look at the last stack trace, looks like something goes wrong in the parser. Unfortunatly, I cannot reproduce 
the problem using the HTML file generated by mime!

Comment 24

[ji]

I'm not sure what mailer was used to send out this original mail. It was 
reported from outside customer. I'll send out an email to ask. I guess it must 
be common to receive this kind of mails which are automatically sent out from 
e-magzine servers. 

Comment 25

[scottputterman]

My main concern is whether this was a rare occurrence or if all emails from
Chinese magazines are going to come out like this.  If it's just one email I
wouldn't hold the release for this.  If it's a lot of emails then that's a
different story.  I'm not sure how to assess this, but is there someone on I18N
who could help with that?

Comment 26

[msanz]

adding momoi to the list, he may have some ideas

Comment 27

[Jean-Francois Ducarroz]

After debugging this problem for couple of days, I think the problem is in the
parser. Here is what seems to append:

Mime read a chinese message which has an incorrect charset (US-ASCII). When the
parser read the data received from mime, it pass it to the utf8ToUnicode decoder
which failed because of "bad" characters (0xA3, 0xAA, etc...). When the decoder
failed, the parser replace the bad character and try again to convert the data.

Looks like we are failing on a lot of characters causing the parser to allocate
a lot of memory which cause use to run of it quicjly which cause the all app to
hang.

The problem seems to occurs in nsScanner::Append(), in the part that take care
of the convertion.

At this point, I would like to hand this bug to either layout team (Harish) or
to I18N team (Frank). Anybody?

Comment 28

[nhottanscp]

As you pointed out, there is a problem in nsScanner::Append().
Frank and I talked to Vidur, he is going to look at this.
Reassign to harishd, cc to vidur.

Comment 29

[harishd]

mmm, I saved attachment id 39507 in my Local Folder and was able to read the
mail without any problem. How else can I reproduce this problem?

Comment 30

[ji]

Harishd, I'll forward you the original mail sent from the customer. I didn't
attach the original one since it contains the customer's telephone number.

Comment 31

[harishd]

Attachment: patch v1.1 [ Someone please try this patch and let me know if the problem still exists ]

Comment 32

[Jean-Francois Ducarroz]

I have tried the patch and it does something but we still hang but not at the 
same level.

Mime now can correctly finish to process the message, it even fire the 
onStopRequest. However, looks like layout loop for ever when it display the 
page! the vertical scrollbar grow up indefinitely until the app hang again.

The problem has moved deeper in layout!

Comment 33

[harishd]

Jean-Francois: Could you please post the stack?

Comment 34

[ji]

I got the following reply from the editor of the E-Magazine.

"We use Majordomo and linux sendmail to send
our magazine. I do not know the versions,
because the Majordomo is on the machine
other than that one we use, and we do not
have privilege to directly access that machine.
 
The codes are plain 8 bit Chinese GB 2312-1980.
No MIME header in message header."

Comment 35

[Jean-Francois Ducarroz]

Attachment: stack trace #1, using patch v1

Comment 36

[Jean-Francois Ducarroz]

Attachment: stack trace #2, using patch v1

Comment 37

[harishd]

Jean-Francois: What testcase did you use and how to reproduce it?

Comment 38

[Frank Tang]

nhotta- can you code reveiew that patch?
I have one question, it seems the origional code does not change mTotalRead but
the new code does. What does that count mean?
What is the unit? Byte? PRUnichar?

Comment 39

[Jean-Francois Ducarroz]

Attachment: mailbox file that contains the message that cause the problem

Comment 40

[Jean-Francois Ducarroz]

Harish, copy the attachment id 41956 into your profile/mail/local folders, name 
it something like bug87110. Then restart mozilla. Now you should have a folder 
into your local folders mail account which contains the chinese message I am 
using for testing. Just click on it and wait...

Comment 41

[harishd]

ok, I think I see the problem..though the stack is not exactly the same..

Comment 42

[harishd]

How can I get the HTML markup for testcase id 41956?

Comment 43

[nhottanscp]

I applied the patch to today's trunk and tried pages in international smoketest
(win32, auto-detection off).
http://www.mozilla.org/quality/smoketests/index.html
And a couple of Japanese pages,
http://www.yomiuri.co.jp, http://www.ebay.co.jp.
I did not see any problem viewing those pages.
I also tried international smoke test for message view. The messages are shown
correctly.

For the problem message, I saw the same thing as Jean-Francois. But one good
thing is that it's not a complete freeze so I can change charset to Chinese then
the message is shown fine.

Comment 44

[Jean-Francois Ducarroz]

Harish: I would guess <pre wrap>. All the chinese data is inside a <pre wrap> 
tag.

Comment 45

[nhottanscp]

(ftang using nhotta's account)
cc attinasi and waterson
attinasi and waterson:
can you take a look at the stack trace #1 and #2? Who may help us to figure out
what happened inside layout?

Comment 46

[harishd]

I think I know what the problem is. 

Looks like the mime code does not know how to handle the "error" message
propagated by mUnicodeDecoder->Convert(). So, what I noticed is that the same
chunk of data ( that happened to contain LINK tag ) is fed into the parser
several times and therefore causing the layout to process the LINK tag endlessly. 

FYI: The problem goes away if I don't propagate the error message.

Will attach a couple of patches for testing purpose.

Comment 47

[harishd]

Attachment: patch v1.2 [ watch your std output ]

Comment 48

[harishd]

Attachment: patch v1.3 [ stop propagating error message from mUnicodeDecoder->Convert() ]

Comment 49

[Frank Tang]

r=ftang,please add NS_ASSERTION((NS_OK==res),"xxx"); right before we change res
to NS_OK.

nhotta, please try the new patch 1.3 . Thanks.

Comment 50

[harishd]

Attachment: patch v1.4 [ with ftang's suggestion. Note: Expect multiple assertions! ]

Comment 51

[Jean-Francois Ducarroz]

I have tested the last patch (v1.4) and it solve the problem with this chinese 
email. However, I don't think it's a good idea to put an assert and the end of 
the while loop. That will just bother more that it would be usefull.

Comment 52

[vidur (gone)]

Requested changes:
1) Remove NS_ASSERTION based on the error code. It's reasonable to reach the end
of the loop with an error code (the decoding error occurs for the last
character), so an assertion isn't appropriate.
2) Change the comment associated with the line setting the return code of
nsScanner::Append() to "don't propagate return code of unicode decoder, since it
doesn't reflect on our success or failure".

After these changes are made, sr=vidur.

Comment 53

[harishd]

Attachment: patch v1.5 [ addressing vidur's concerns ]

Comment 54

[nhottanscp]

I applied patch v1.5 to today's trunk locally and tried pages in international 
smoketest (win32, auto-detection off).
http://www.mozilla.org/quality/smoketests/index.html
And a couple of Japanese pages,
http://www.yomiuri.co.jp, http://www.ebay.co.jp.
I did not see any problem viewing those pages.
I also tried international smoke test for message view. The messages are shown
correctly.
The problem Chinese message was also shown without the hang then I was able to 
set charset to Chinese and the message shown correctly.

Teruko, ylong, are there any additional tests available to test international 
HTML pages?

Comment 55

[harishd]

I ran browser buster ( top60 international sites ) and did not see any problem.

Comment 56

[harishd]

Fix checked in to the trunk.

Comment 57

[Scott MacGregor]

adding vtrunk keyword so QA will QA this on the trunk.

Comment 58

[harishd]

Fixed on the branch too.

Comment 59

[Blake Ross]

Harish, is there any reason this needs to remain open now?

Comment 60

[ji]

Verified with win32, linux and mac 07/16 branch builds. It's fixed.
I also ran intl browser smoketests on all the three platforms. No 
regressions have been found.

Comment 61

[ji]

Marked it as verified. It's fixed.