88746 - Mozilla appears to hang because of bad charset in javascript.

Status: VERIFIED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[David Einstein]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.2+)
Gecko/20010701
BuildID:    2001070108

Attempting to load the details on the debian woody freeze causes mozilla to
appear to never stop loading.  Running in a debug build one sees that an
assertion is triggered in nsScriptLoader.cpp line 715 "Could not convert
external JavaScript to Unicode!". Tracing this back we see that the cause is the
following line in the generated html
<meta HTTP-EQUIV="Content-Type" CONTENT="text/html; charset=">
which causes the nsDocument to have a charset of ">  I am not sure whether this
is a parse error in the parser, or in nsDocument, but I do not know which
component nsDocument lives in, so parser has got it.

Reproducible: Always
Steps to Reproduce:
1. Load suggested URL
2. Wait
3.

Actual Results:  White screen and loading behavior.

Expected Results:  Interesting informative much awaited article.

Comment 1

[David Einstein]

Ok, it is at least partially the parser.  
The code in nsParser::DetectMetaTag is the culprit in this case.  Also, the code
in nsParser::DetectMetaTag seems strange,

do {
...
  tagStart = tagEnd;
  tagEnd = end;
} while (tagStart != end);
return PR_FALSE;
Is mozilla intentionally checking only the first meta tag? When I get time, I'll
see what happens with a malformed charset in other than the first meta tag.

Looking at the other end of this problem, shouldn't the javascript people do
something other than fall over dead in the presence of a charset that they don't
understand?  UTF-8 or ISO-8859-1 seem to be popular guesses.

Comment 2

[harishd]

Reassigning bug to vidur.

Comment 3

[David Einstein]

Please ignore my comments about the do while. It was 1:30 am.  There is a bug
with parsing malformed <meta Content-Type tags though.  

Comment 4

[harishd]

Taking back.

Comment 5

[David Einstein]

Attachment: Javascript for testcase

Comment 6

[David Einstein]

Attachment: testcase

Comment 7

[David Einstein]

Attachment: testcase2

Comment 8

[David Einstein]

Attachment: patch for second bug

Comment 9

[David Einstein]

There are two bugs, and two testcases.
The first testcase appears to freeze because the charset was parsed as "> and
javascript fell over and died.
The second testcase does not appear tof freeze.  This could be considered a good
thing, but it doesn't freeze because only the first meta tag was being parsed.
 
The bug causing only one meta tag to be parsed is fixed in the accompanying patch.
I have a fix for the stringTokenizer, but I want to ensure that it doesnt
destroy anything else.

Comment 10

[David Einstein]

Attachment: patch for the first bug

Comment 11

[David Einstein]

Ok, the patch for the first bug does two things.  It keeps the tokenizer from
reading past the end of the buffer, which I am prettty sure is correct, and it
ignores the trailing quote that is given to us by the tokenizer.  It may be
better to have the tokenizer report the shorter length in the case that taget is
a quote, kSkipQuotes is true and the last character read is a quote.  This is
the only place that it seems to matter, so that's where I fixed it.  I'm going
to submit another bug about the behavior of the scriptLoader.

Comment 12

[harishd]

Attachment: patch v1.3

Comment 13

[David Einstein]

Ok, It is simple will and work I will grant you that. As somone who has to read
the code with little to no preparation the idea of pushing orselves past the end
of the buffer by mistaking a trailing quote for a leading one is a bit
unsettling though. 

Comment 14

[vidur (gone)]

r/sr=vidur

Comment 15

[vidur (gone)]

Fix checked in to the TRUNK.

Comment 16

[Heikki Toivonen (remove -bugzilla when emailing directly)]

And by the way, this was r=heikki...

Comment 17

[harishd]

Marking FIXED.

Comment 18

[bsharma]

Verified on:
build: 2001-07-02-04-Trunk
platform: WinNT

The above url loads fine.
The browser does not load 1st test case and keeps trying.
The 2nd test case loads fine.