nsXPCOMDetector::DoIt didn't call Reset resulting in crash [@ nsSJISProber::HandleData] triggered by Canvas getAsText

VERIFIED FIXED in Firefox 7

Status

()

Core
Internationalization
--
critical
VERIFIED FIXED
7 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: smontagu)

Tracking

(Blocks: 1 bug, 5 keywords)

Trunk
mozilla9
crash, regression, testcase, verified-aurora, verified-beta
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox7 fixed, firefox8 fixed, blocking2.0 -, status1.9.2 unaffected)

Details

(Whiteboard: [tbird topcrash][ccbr][qa!], crash signature)

Attachments

(3 attachments, 3 obsolete attachments)

(Reporter)

Description

7 years ago
Created attachment 480262 [details]
testcase (crashes Firefox when loaded)
(Reporter)

Comment 1

7 years ago
Only crashes on Mac OS X 10.6, not on Mac OS X 10.5?
(Reporter)

Comment 2

7 years ago
Created attachment 480337 [details]
stack trace+
Also crashes on Linux x86-64 (SIGBUS).
'aLen' is zero.  It originates from 'numRead' being zero here:
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDOMFile.cpp#355
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Comment 4

7 years ago
Created attachment 480630 [details] [diff] [review]
universalchardet patch

The actual bug is that nsXPCOMDetector::DoIt wasn't calling Reset, but I've scattered some assertions as well.
Assignee: nobody → smontagu
Attachment #480630 - Flags: review?(VYV03354)
(Assignee)

Comment 5

7 years ago
Created attachment 480632 [details] [diff] [review]
content patch

I guess we want to do this as well
Attachment #480632 - Flags: review?
(Reporter)

Updated

7 years ago
Attachment #480632 - Attachment is patch: true
Attachment #480632 - Attachment mime type: application/octet-stream → text/plain
(Assignee)

Updated

7 years ago
Attachment #480632 - Flags: review? → review?(jonas)
(Assignee)

Comment 6

7 years ago
Created attachment 480634 [details] [diff] [review]
crashtest
Comment on attachment 480630 [details] [diff] [review]
universalchardet patch

Is HandleData guaranteed to be called with a non-zero length buffer?
(Assignee)

Comment 8

7 years ago
nsUniversalDetector::HandleData should be able to handle being called with a non-zero length buffer.
(Assignee)

Updated

7 years ago
status1.9.2: --- → unaffected
(doesn't seem like a canvas bug?)
Component: Canvas: 2D → DOM
QA Contact: canvas.2d → general
blocking2.0: --- → ?
Attachment #480632 - Flags: review?(jonas) → review+

Updated

7 years ago
Component: DOM → Internationalization
QA Contact: general → i18n
Summary: Canvas getAsText crash [@ nsSJISProber::HandleData] → nsXPCOMDetector::DoIt didn't call Reset resulting in crash [@ nsSJISProber::HandleData] triggered by Canvas getAsText

Comment 10

7 years ago
I really don't think this needs to block, but I will certainly approve as necessary.
blocking2.0: ? → -
Crash Signature: [@ nsSJISProber::HandleData]

Updated

6 years ago
Crash Signature: [@ nsSJISProber::HandleData] → [@ nsSJISProber::HandleData ]
Also seen in small quantity for Thunderbird users. Mac and linux only, starting with build 20110510 TB3.3a4pre.  bp-c24077fe-be6d-4989-b60b-d512a2110614 has an email address
Whiteboard: [tbird crash]
Duplicate of this bug: 668570
is review request on VYV03354's radar?

#7 crash for Thunderbird v5 =>  no longer small potatoes
Whiteboard: [tbird crash] → [tbird topcrash][ccbr]
Ccing to emk-san.
crash does not exist in Thunderbird 3, nor firefox prior to v4
Keywords: regression
Comment on attachment 480630 [details] [diff] [review]
universalchardet patch

This patch didn't build with "ac_add_options --enable-debug".
Attachment #480630 - Flags: review?(VYV03354) → review-
Duplicate of this bug: 680396
Created attachment 554690 [details] [diff] [review]
universalchardet patch v2

This needs LIBXUL_LIBRARY = 1 to use NS_ASSERTION
Attachment #480630 - Attachment is obsolete: true
Attachment #554690 - Flags: review?(VYV03354)
Attachment #554690 - Flags: review?(VYV03354) → review+
Comment on attachment 480632 [details] [diff] [review]
content patch

charset detection into nsDOMFile is removed by But 661876.
Attachment #480632 - Attachment is obsolete: true
Comment on attachment 480634 [details] [diff] [review]
crashtest

Test case doesn't work by bug 661876
Attachment #480634 - Attachment is obsolete: true
Also, FileReaderSync has no this bug because it checks numRead.
We can create a test case which calls nsICharsetDetector::DoIt with a zero length buffer deliberately.
And nsMsgAttachmentHandler::PickCharset doesn't check whether the buffer is empty or not. It should be fixed before landing this.
(In reply to Masatoshi Kimura [:emk] from comment #22)
> We can create a test case which calls nsICharsetDetector::DoIt with a zero
> length buffer deliberately.
> And nsMsgAttachmentHandler::PickCharset doesn't check whether the buffer is
> empty or not. It should be fixed before landing this.

Thanks, emk-san.  I reopen bug 680396 for comm-central.

Updated

6 years ago
Blocks: 680396
hg.mozilla.org/integration/mozilla-inbound/rev/0d1dd45a06fc
Whiteboard: [tbird topcrash][ccbr] → [tbird topcrash][ccbr][inbound]
http://hg.mozilla.org/mozilla-central/rev/0d1dd45a06fc
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [tbird topcrash][ccbr][inbound] → [tbird topcrash][ccbr]
Target Milestone: --- → mozilla9
Comment on attachment 554690 [details] [diff] [review]
universalchardet patch v2

This crash is top 7 on Thunderbird 6.  So I request for aurora and beta.
Attachment #554690 - Flags: approval-mozilla-beta?
Attachment #554690 - Flags: approval-mozilla-aurora?

Updated

6 years ago
Attachment #554690 - Flags: approval-mozilla-beta?
Attachment #554690 - Flags: approval-mozilla-beta+
Attachment #554690 - Flags: approval-mozilla-aurora?
Attachment #554690 - Flags: approval-mozilla-aurora+
http://hg.mozilla.org/releases/mozilla-aurora/rev/3aba81100f4f
http://hg.mozilla.org/releases/mozilla-beta/rev/1a349d6447ee
status-firefox7: --- → fixed
status-firefox8: --- → fixed
Mozilla/5.0 (Windows NT 6.1; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Windows NT 6.1; rv:7.0a2) Gecko/20110816 Firefox/7.0a2
Mozilla/5.0 (Windows NT 6.1; rv:9.0a1) Gecko/20110823 Firefox/9.0a1
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0) Gecko/20100101 Firefox/7.0

I've runned the testcase on Firefox 7.0b2, Aurora and Nightly, but Firefox did not crash.
Is running the testcase sufficient? or is there another way to verify this?
Thanks.

Comment 29

6 years ago
Considering comment28, setting resolution to Verified Fixed also on Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0) Gecko/20100101 Firefox/7.0 beta 5

Firefox didn't crash when running the test case provided.
Status: RESOLVED → VERIFIED
Still needs verification on aurora, qa+
Whiteboard: [tbird topcrash][ccbr] → [tbird topcrash][ccbr][qa+]
Keywords: verified-beta

Comment 31

6 years ago
Verified  - No crash on loading the test case on:
Mozilla/5.0 (Windows NT 6.1; rv:8.0a2) Gecko/20110921 Firefox/8.0a2
Mozilla/5.0 (Windows NT 5.1; rv:8.0a2) Gecko/20110921 Firefox/8.0a2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:8.0a2) Gecko/20110921 Firefox/8.0a2
Mozilla/5.0 (X11; Linux x86_64; rv:8.0a2) Gecko/20110921 Firefox/8.0a2

Updated

6 years ago
Keywords: verified-aurora
Whiteboard: [tbird topcrash][ccbr][qa+] → [tbird topcrash][ccbr][qa!]
You need to log in before you can comment on or make changes to this bug.