309599 - crash in nsIHTMLEditor::insertHTML

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[alexander :surkov (:asurkov)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4

nsIHTMLEditor.insertHTML("") crashes mozilla.

Reproducible: Always

Comment 1

[alexander :surkov (:asurkov)]

Attachment: testcase

Comment 2

[Frank Wein [:mcsmurf]]

The insertHTML dialog for Composer does a null-check for this case, see
http://lxr.mozilla.org/mozilla/source/editor/ui/dialogs/content/EdInsSrc.js#72.
If you mess around with interfaces which directly interact with the engine, i
think you should do things like null-check yourself...

Comment 3

[alexander :surkov (:asurkov)]

(In reply to comment #2)
> The insertHTML dialog for Composer does a null-check for this case, see
> http://lxr.mozilla.org/mozilla/source/editor/ui/dialogs/content/EdInsSrc.js#72.
> If you mess around with interfaces which directly interact with the engine, i
> think you should do things like null-check yourself...

When I got a crash then I begun to check incorrect values. But I belive mozilla
should not be crashed in any case.

Comment 4

[Martijn Wargers (dead)]

Attachment: testcase2

Same as previous testcase, but with enableprivilege for easy testing the crash.

Comment 5

[Martijn Wargers (dead)]

According to bug 309459, comment 4:
"Crashing is always a bug. In this case, visitEntries should be null-checking its
argument."
So that would make this a genuine bug, not?

Comment 6

[Martijn Wargers (dead)]

Attachment: patch

This fixes it for me.

Comment 7

[Martijn Wargers (dead)]

Not sure, but I guess some code in EdInsSrc.js could be removed also, if the patch is fine.

Comment 8

[[e-mail]]

Comment on attachment 215754 [details] [diff] [review]
patch

Sorry, I don't really understand this code; try brade or glazou perhaps?

Comment 9

[Kathleen :Brade]

Comment on attachment 215754 [details] [diff] [review]
patch

r=brade

Comment 10

[Johnny Stenback (:jst)]

Comment on attachment 215754 [details] [diff] [review]
patch

sr=jst (and please land on the 1.8.1 branch too)

Comment 11

[Martijn Wargers (dead)]

Checking in editor/libeditor/html/nsHTMLDataTransfer.cpp;
/cvsroot/mozilla/editor/libeditor/html/nsHTMLDataTransfer.cpp,v  <--  nsHTMLData
Transfer.cpp
new revision: 1.115; previous revision: 1.114
done

Fixed on trunk.

Comment 12

[:Gavin Sharp [email: [e-mail]]]

Checked in on the 1.8 branch.
mozilla/editor/libeditor/html/nsHTMLDataTransfer.cpp; new revision: 1.110.2.2;

Comment 13

[Martijn Wargers (dead)]

Johnny, should I ask for approval 1.8.0.3?
Bug 331275 is a recent regression in the 1.8 branch, that gets fixed by this patch.
See bug 331275, comment 4, 306 crashes found in the old database for Thunderbird.
I think the patch is safe, not?

Comment 14

[Jay Patel [:jay]]

Comment on attachment 215754 [details] [diff] [review]
patch

Please check in promptly on the 1.8.0 branch.  Thanks!

Comment 15

[:Gavin Sharp [email: [e-mail]]]

Checked in on the 1.8.0 branch.
mozilla/editor/libeditor/html/nsHTMLDataTransfer.cpp 	1.110.2.1.4.1

Comment 16

[Marcia Knous [:marcia]]

verified using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4. No crash using either testcase.

Comment 17

[(not reading, please use [e-mail] instead)]

*** Bug 334883 has been marked as a duplicate of this bug. ***