348706 - Passing null as a value of a DOM attribute causes crash

Status: VERIFIED FIXED

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

Description

[Vladimir Korenev]

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

Comment 1

[Vladimir Korenev]

Attachment: Patch

Comment 2

[Vladimir Korenev]

Attachment: Test case

Comment 3

[jhp (no longer active)]

Attachment: comprehensive patch

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.

Comment 4

[jhp (no longer active)]

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.

Comment 5

[Vladimir Korenev]

Comment on attachment 236252 [details] [diff] [review]
comprehensive patch

I ran some tests that had caused crash earlier. XULRunner with this patch works fine.

Comment 6

[jhp (no longer active)]

Checked in to trunk. ->FIXED

Comment 7

[Vladimir Korenev]

Is it possible to check in this patch to 1.8.1 branch?

Comment 8

[jhp (no longer active)]

Comment on attachment 236252 [details] [diff] [review]
comprehensive patch

Asking for 1.8.1 branch approval.  This patch is XULRunner only.

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 236252 [details] [diff] [review]
comprehensive patch

1.8.1 is out, moving request so we notice it

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 236252 [details] [diff] [review]
comprehensive patch

approved for 1.8 branch, a=dveditz for drivers

Comment 11

[jhp (no longer active)]

Checked in to MOZILLA_1_8_BRANCH.

Comment 12

[Jay Patel [:jay]]

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!

Comment 13

[Vladimir Korenev]

Works fine for me.