Closed Bug 348706 Opened 14 years ago Closed 13 years ago

Passing null as a value of a DOM attribute causes crash

Categories

(Core Graveyard :: Java to XPCOM Bridge, defect, critical)

defect
Not set
critical

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: vkorenev, Assigned: jhpedemonte)

Details

(Keywords: crash, testcase, verified1.8.1.1)

Attachments

(2 files, 1 obsolete file)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.5) Gecko/20060802 Firefox/1.5.0.5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.5) Gecko/20060802 Firefox/1.5.0.5

The following code causes JVM crash:

nsIDOMAttr attr = ... // Get or create an attribute
attr.setValue(null);  // Mozilla crashes here


Reproducible: Always
Attached patch Patch (obsolete) — Splinter Review
Attached file Test case
Attachment #233745 - Flags: review?(pedemonte)
Severity: normal → critical
Keywords: crash, testcase
Attachment #233745 - Flags: review?(pedemonte) → review?(jhpedemonte)
Status: UNCONFIRMED → NEW
Ever confirmed: true
This bug finally spurred me to create some unit tests (bug 350886).  In addition to not handling the 'null' case (as described in this bug), I was also doing too much.  This patch fixes the crash and cleans up the code.
Attachment #233745 - Attachment is obsolete: true
Attachment #233745 - Flags: review?(jhpedemonte)
Comment on attachment 236252 [details] [diff] [review]
comprehensive patch

Vladimir, since you've opened some JavaXPCOM bugs and attached good patches, you seem to be somewhat familiar with the JavaXPCOM source code.  Do you mind reviewing this patch?  If you don't have the privileges to set the review flag on the attachment, then just post a comment.
Attachment #236252 - Flags: review?(vkorenev)
Attachment #236252 - Flags: review?(vkorenev) → review+
Comment on attachment 236252 [details] [diff] [review]
comprehensive patch

I ran some tests that had caused crash earlier. XULRunner with this patch works fine.
Checked in to trunk. ->FIXED
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Is it possible to check in this patch to 1.8.1 branch?
Comment on attachment 236252 [details] [diff] [review]
comprehensive patch

Asking for 1.8.1 branch approval.  This patch is XULRunner only.
Attachment #236252 - Flags: approval1.8.1?
Comment on attachment 236252 [details] [diff] [review]
comprehensive patch

1.8.1 is out, moving request so we notice it
Attachment #236252 - Flags: approval1.8.1? → approval1.8.1.1?
Comment on attachment 236252 [details] [diff] [review]
comprehensive patch

approved for 1.8 branch, a=dveditz for drivers
Attachment #236252 - Flags: approval1.8.1.1? → approval1.8.1.1+
Checked in to MOZILLA_1_8_BRANCH.
Keywords: fixed1.8.1.1
Vladminir:  Can you help us verify this fix on the 1.8.1 branch?  Please run your testcase against a recent 1.8.1 nightly build (preferably the latest 2.0.0.1 candidate build) and replace the "fixed1.8.1.1" keyword with "verified1.8.1.1".  Thanks!
Works fine for me.
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.