Closed Bug 119335 Opened 23 years ago Closed 22 years ago

nsXMLContentSerializer::AppendTextData() causes assertion in nsDependentString::Rebind()

Categories

(Core :: DOM: Serializers, defect, P3)

x86
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla1.2alpha

People

(Reporter: kinmoz, Assigned: t_mutreja)

References

Details

(Whiteboard: [needs a=])

Attachments

(2 files)

I've been seeing this the last few weeks with a trunk build ...

Sometimes when saving an HTML doc in composer I see several assertions like:

###!!! ASSERTION: nsDependentString must wrap only null-terminated strings: '!*a
EndPtr', file ..\..\dist/include/string\nsDependentString.h, line 86
###!!! Break: at file ..\..\dist/include/string\nsDependentString.h, line 86

The assertion seems to be triggered by the following code in
nsXMLContentSerializer::AppendTextData():


    if (frag->Is2b()) {
      AppendToString(nsDependentString(frag->Get2b()+aStartOffset, length),
                     aStr,
                     aTranslateEntities,
                     aIncrColumn);
    }
    else {
      AppendToString(NS_ConvertASCIItoUCS2(frag->Get1b()+aStartOffset, length),
                     aStr,
                     aTranslateEntities,
                     aIncrColumn);
    }

The 1b and 2b pointers in the text fragment are not null terminated, so you may
want to use something other than a Dependent string, or make a local copy of the
fragment contents and null terminate it before using the dependent string.

Here's the partial stack trace that shows how I'm hitting the assertion:


NTDLL! 77f7629c()
nsDebug::Assertion(const char * 0x1010ce38
??_C@_0DJ@PCFA@nsDependentString?5must?5wrap?5only@, const char * 0x1010ce7c
??_C@_09OKBI@?$CB?$CKaEndPtr?$AA@, const char * 0x1010ce00
??_C@_0CO@HHMI@?4?4?2?4?4?2dist?1include?1string?2nsDepe@, int 86) line 290 + 13
bytes
nsDependentString::Rebind(const unsigned short * 0x0613d060, const unsigned
short * 0x0613d1ba) line 86 + 34 bytes
nsDependentString::Rebind(const unsigned short * 0x0613d060, unsigned int 173)
line 96
nsDependentString::nsDependentString(const unsigned short * 0x0613d060, unsigned
int 173) line 99 + 51 bytes
nsXMLContentSerializer::AppendTextData(nsIDOMNode * 0x0613d37c, int 0, int -1,
nsAString & {...}, int 1, int 0) line 120 + 40 bytes
nsHTMLContentSerializer::AppendText(nsHTMLContentSerializer * const 0x06749670,
nsIDOMText * 0x0613d37c, int 0, int -1, nsAString & {...}) line 161 + 31 bytes
nsDocumentEncoder::SerializeNodeStart(nsIDOMNode * 0x0613d37c, int 0, int -1,
nsAString & {...}) line 318
nsDocumentEncoder::SerializeToStringRecursive(nsIDOMNode * 0x0613d37c, nsAString
& {...}) line 373 + 20 bytes
nsDocumentEncoder::SerializeToStringRecursive(nsIDOMNode * 0x0613d43c, nsAString
& {...}) line 394 + 21 bytes
nsDocumentEncoder::SerializeToStringRecursive(nsIDOMNode * 0x0613d5a0, nsAString
& {...}) line 394 + 21 bytes
nsDocumentEncoder::SerializeToStringRecursive(nsIDOMNode * 0x0613da5c, nsAString
& {...}) line 394 + 21 bytes
nsDocumentEncoder::SerializeToStringRecursive(nsIDOMNode * 0x0613cefc, nsAString
& {...}) line 394 + 21 bytes
nsDocumentEncoder::SerializeToStringRecursive(nsIDOMNode * 0x0613b05c, nsAString
& {...}) line 394 + 21 bytes
nsDocumentEncoder::SerializeToStringRecursive(nsIDOMNode * 0x0606406c, nsAString
& {...}) line 394 + 21 bytes
nsDocumentEncoder::SerializeToStringRecursive(nsIDOMNode * 0x05f9f21c, nsAString
& {...}) line 394 + 21 bytes
nsDocumentEncoder::SerializeToStringRecursive(nsIDOMNode * 0x06052d54, nsAString
& {...}) line 394 + 21 bytes
nsDocumentEncoder::EncodeToString(nsDocumentEncoder * const 0x06749760,
nsAString & {...}) line 934 + 21 bytes
nsDocumentEncoder::EncodeToStream(nsDocumentEncoder * const 0x06749760,
nsIOutputStream * 0x06749bb0) line 975 + 19 bytes
nsDocument::SaveFile(nsDocument * const 0x06052d78, nsIURI * 0x06748440, int 1,
int 0, const unsigned short * 0x0674c9f0, const unsigned short * 0x016c0f84,
unsigned int 258, unsigned int 4294967295) line 3450 + 44 bytes
nsEditor::SaveFile(nsEditor * const 0x05fe0d40, nsIURI * 0x06748440, int 1, int
0, const nsAString & {...}) line 1009 + 104 bytes
This looks similar to bug 109813 and bug 115075.
jst suggested Substring(). I did that, and my own bug 121897 seemed to work
after this change. So I am pretty confident all 3 of those other bugs are dupes
of this. Taking and marking the dupes.
Assignee: peterv → heikki
Priority: -- → P3
Whiteboard: [fixinhand]
Target Milestone: --- → mozilla0.9.9
*** Bug 109813 has been marked as a duplicate of this bug. ***
*** Bug 115075 has been marked as a duplicate of this bug. ***
*** Bug 121897 has been marked as a duplicate of this bug. ***
Comment on attachment 67801 [details] [diff] [review]
Use Substring

sr=jag
Attachment #67801 - Flags: superreview+
Comment on attachment 67801 [details] [diff] [review]
Use Substring

r=jst
Attachment #67801 - Flags: review+
Checked in.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Whiteboard: [fixinhand]
Not fixed.  Go to:
http://www.mozilla.org/quality/browser/front-end/testcases/xpapps-gui/find-on-page/find-in-page.html
Scroll down to "Long almost-match" and doubleclick on "Test" in italics right
beneath it, and you'll see the assert (at least on linux in a build from today).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I've been asked to reassign this to Tanu.
Assignee: heikki → tmutreja
Status: REOPENED → NEW
Repeating the same steps as Akkana mentioned above, I'm not getting the 
assertions on Linux with Feb24'02 build.
In fact on Linux I'm getting following ASSERTION failure message:

NS_ASSERTION(aStartOffset == 0,"a start offset is beyond the end of the text 
fragment!");
at nsXMLContentSerializer.cpp line 110. 

I feel this is not related to the original bug mentioned there.
Status: NEW → ASSIGNED
OS: Windows NT → Linux
Target Milestone: mozilla0.9.9 → mozilla1.0
Moving Netscape owned 0.9.9 and 1.0 bugs that don't have an nsbeta1, nsbeta1+,
topembed, topembed+, Mozilla0.9.9+ or Mozilla1.0+ keyword.  Please send any
questions or feedback about this to adt@netscape.com.  You can search for
"Moving bugs not scheduled for a project" to quickly delete this bugmail.
Target Milestone: mozilla1.0 → mozilla1.2
On Linux, double clicking on the HTML content generates a copy event. On double
clicking a word, here selection range is including end of the last tag too (I
don't understand the intent of that! ). In the test case mentioned by Akkana,
last tag here is an <h2> containing "Long almost-match:".Hence aStartOffset is
pointing to the last position of this text and has the value greater than 0
i.e. 18.

In existing code I could not see any reason for asserting "aStartOffset == 0".
It may be greater than 0 but should be less than ((aEndOffset == -1) ?
frag->GetLength() : aEndOffset).  Here I'm including generic assertions that I
find reasonable and removing the earlier one.
Keywords: patch
Whiteboard: [needs review]
Comment on attachment 72777 [details] [diff] [review]
Patch to fix "Assertion Failure" mentioned in comment#13. 

sr=jst
Attachment #72777 - Flags: superreview+
Comment on attachment 72777 [details] [diff] [review]
Patch to fix "Assertion Failure" mentioned in comment#13. 

r=akkana
Attachment #72777 - Flags: review+
Whiteboard: [needs review] → [needs a=]
Comment on attachment 72777 [details] [diff] [review]
Patch to fix "Assertion Failure" mentioned in comment#13. 

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #72777 - Flags: approval+
Checkin in for tmutreja
Fixed with checkin
D:\mozilla\content\base\src>cvs commit
cvs commit: Examining .
Checking in nsXMLContentSerializer.cpp;
/cvsroot/mozilla/content/base/src/nsXMLContentSerializer.cpp,v  <--  nsXMLConten
tSerializer.cpp
new revision: 1.22; previous revision: 1.21
done
Status: ASSIGNED → RESOLVED
Closed: 23 years ago22 years ago
Resolution: --- → FIXED
Reporter: please verify and mark verified-fixed...thanks!
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: