Assert applying First() to empty string in HTMLContentSink::AddDocTypeDecl

VERIFIED FIXED

Status

()

P3
normal
VERIFIED FIXED
19 years ago
17 years ago

People

(Reporter: roc, Assigned: dbaron)

Tracking

Trunk
x86
Windows NT
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta2+])

Attachments

(2 attachments)

An assertion is triggered in HTMLContentSink::AddDocTypeDecl, when First() is 
applied to an empty string. I hit it on line 3123 but there are other unguarded 
uses of First().

I have a simple patch to fix this stuff. I will also attach a test case.
Created attachment 10325 [details] [diff] [review]
Patch to nsHTMLContentSink to prevent First() on empty strings
Created attachment 10326 [details]
Document with DOCTYPE that triggers assertion (systemId is empty)
Reassigning to Harish.
Assignee: clayton → harishd

Comment 4

19 years ago
adding keyword nsbeta2 since I run into this almost constantly while I surf.  We
have a patch and it should be reviewed as soon as possible to save developers
time (I ran into it many times doing my smoketesting; I assume other developers
are also constantly bumping into this annoying assert).
Keywords: nsbeta2
Or roc+moz@cs.cmu.edu could just check his patch in, with approval by me, if 
given an r= by harishd or equivalent.  And assuming roc+moz has his CVS access 
set up acceptably!

/be
I don't. CVS over SSH to cvs.mozilla.org isn't working and in this environment I 
will not use cleartext passwords.

Comment 7

19 years ago
This is jst's bug...and he sent a patch to me for a review.  I've reviewed his 
patch and will be checking it in for him. Sorry for my delayed response folks.
Status: NEW → ASSIGNED

Comment 8

19 years ago
Index: html/document/src/nsHTMLContentSink.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/html/document/src/nsHTMLContentSink.cpp,v
retrieving revision 3.365
diff -u -r3.365 nsHTMLContentSink.cpp
--- nsHTMLContentSink.cpp       2000/06/16 14:54:57     3.365
+++ nsHTMLContentSink.cpp       2000/06/16 23:51:57
@@ -3059,7 +3059,7 @@
       publicId.Trim(" \t\n\r");
 
       // Strip quotes
-      PRUnichar ch = publicId.First();
+      PRUnichar ch = publicId.Length() ? publicId.First() : '\0';
 
       if (ch == '"' || ch == '\'') {
         publicId.Cut(0, 1);
@@ -3120,7 +3120,7 @@
       systemId.Trim(" \t\n\r");
 
       // Strip quotes
-      PRUnichar ch = systemId.First();
+      PRUnichar ch = systemId.Length() ? systemId.First() : '\0';
 
       if (ch == '"' || ch == '\'') {
         systemId.Cut(0, 1);
@@ -3168,7 +3168,7 @@
    */
   PRInt32 nameEnd = 0;
 
-  if (name.First() != '"' && name.First() != '\'') {
+  if (name.Length() && name.First() != '"' && name.First() != '\'') {
     nameEnd = name.FindCharInSet(" \n\r\t");
   }
 
@@ -3180,7 +3180,7 @@
     name.Mid(publicId, nameEnd, name.Length() - nameEnd);
     publicId.Trim(" \t\n\r");
 
-    PRUnichar ch = publicId.First();
+    PRUnichar ch = publicId.Length() ? publicId.First() : '\0';
 
     if (ch == '"' || ch == '\'') {
       publicId.Cut(0, 1);

Comment 9

19 years ago
Making nsbeta2+

Updated

19 years ago
Whiteboard: [nsbeta2+]

Comment 10

19 years ago
David is willing to own this.  Over to dbaron.
Assignee: harishd → dbaron
Status: ASSIGNED → NEW
Fixes checked in (jst's patch and some additional ones I hit).  I might want to 
change the 3d of the 4 changes above from jst, though, to 

if (name.IsEmpty() || (name.First() != '"' && name.First() != '\''))

so it behaves exactly like it used to (I think).
Status: NEW → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED

Comment 12

19 years ago
Marking verified per last comments.
Status: RESOLVED → VERIFIED
SPAM. HTML Element component deprecated, changing component to Layout. See bug
88132 for details.
Component: HTML Element → Layout
You need to log in before you can comment on or make changes to this bug.