Last Comment Bug 601261 - nsXPCOMDetector::DoIt didn't call Reset resulting in crash [@ nsSJISProber::HandleData] triggered by Canvas getAsText
: nsXPCOMDetector::DoIt didn't call Reset resulting in crash [@ nsSJISProber::H...
Status: VERIFIED FIXED
[tbird topcrash][ccbr][qa!]
: crash, regression, testcase, verified-aurora, verified-beta
Product: Core
Classification: Components
Component: Internationalization (show other bugs)
: Trunk
: All All
: -- critical (vote)
: mozilla9
Assigned To: Simon Montagu :smontagu
:
Mentors:
: 668570 (view as bug list)
Depends on:
Blocks: 326633 680396
  Show dependency treegraph
 
Reported: 2010-10-01 15:03 PDT by Jesse Ruderman
Modified: 2011-09-23 06:17 PDT (History)
18 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
-
unaffected


Attachments
testcase (crashes Firefox when loaded) (100 bytes, text/html)
2010-10-01 15:03 PDT, Jesse Ruderman
no flags Details
stack trace+ (8.10 KB, text/plain)
2010-10-01 18:06 PDT, Jesse Ruderman
no flags Details
universalchardet patch (7.38 KB, patch)
2010-10-04 09:05 PDT, Simon Montagu :smontagu
VYV03354: review-
Details | Diff | Splinter Review
content patch (821 bytes, patch)
2010-10-04 09:09 PDT, Simon Montagu :smontagu
jonas: review+
Details | Diff | Splinter Review
crashtest (683 bytes, patch)
2010-10-04 09:12 PDT, Simon Montagu :smontagu
no flags Details | Diff | Splinter Review
universalchardet patch v2 (4.62 KB, patch)
2011-08-20 19:58 PDT, Makoto Kato [:m_kato]
VYV03354: review+
asa: approval‑mozilla‑aurora+
asa: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Jesse Ruderman 2010-10-01 15:03:33 PDT
Created attachment 480262 [details]
testcase (crashes Firefox when loaded)
Comment 1 Jesse Ruderman 2010-10-01 15:07:21 PDT
Only crashes on Mac OS X 10.6, not on Mac OS X 10.5?
Comment 2 Jesse Ruderman 2010-10-01 18:06:35 PDT
Created attachment 480337 [details]
stack trace+
Comment 3 Mats Palmgren (:mats) 2010-10-03 10:49:37 PDT
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
Comment 4 Simon Montagu :smontagu 2010-10-04 09:05:19 PDT
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.
Comment 5 Simon Montagu :smontagu 2010-10-04 09:09:05 PDT
Created attachment 480632 [details] [diff] [review]
content patch

I guess we want to do this as well
Comment 6 Simon Montagu :smontagu 2010-10-04 09:12:22 PDT
Created attachment 480634 [details] [diff] [review]
crashtest
Comment 7 Masatoshi Kimura [:emk] 2010-10-04 09:34:17 PDT
Comment on attachment 480630 [details] [diff] [review]
universalchardet patch

Is HandleData guaranteed to be called with a non-zero length buffer?
Comment 8 Simon Montagu :smontagu 2010-10-04 09:57:09 PDT
nsUniversalDetector::HandleData should be able to handle being called with a non-zero length buffer.
Comment 9 Vladimir Vukicevic [:vlad] [:vladv] 2010-10-04 12:27:57 PDT
(doesn't seem like a canvas bug?)
Comment 10 Benjamin Smedberg [:bsmedberg] 2010-10-14 11:45:11 PDT
I really don't think this needs to block, but I will certainly approve as necessary.
Comment 11 Wayne Mery (:wsmwk, NI for questions) 2011-06-14 11:53:21 PDT
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
Comment 12 Joshua Cranmer [:jcranmer] 2011-06-30 11:06:54 PDT
*** Bug 668570 has been marked as a duplicate of this bug. ***
Comment 13 Wayne Mery (:wsmwk, NI for questions) 2011-07-26 12:54:14 PDT
is review request on VYV03354's radar?

#7 crash for Thunderbird v5 =>  no longer small potatoes
Comment 14 Makoto Kato [:m_kato] 2011-07-31 18:40:33 PDT
Ccing to emk-san.
Comment 15 Wayne Mery (:wsmwk, NI for questions) 2011-08-09 14:50:26 PDT
crash does not exist in Thunderbird 3, nor firefox prior to v4
Comment 16 Masatoshi Kimura [:emk] 2011-08-12 10:31:56 PDT
Comment on attachment 480630 [details] [diff] [review]
universalchardet patch

This patch didn't build with "ac_add_options --enable-debug".
Comment 17 Wayne Mery (:wsmwk, NI for questions) 2011-08-19 07:17:58 PDT
*** Bug 680396 has been marked as a duplicate of this bug. ***
Comment 18 Makoto Kato [:m_kato] 2011-08-20 19:58:49 PDT
Created attachment 554690 [details] [diff] [review]
universalchardet patch v2

This needs LIBXUL_LIBRARY = 1 to use NS_ASSERTION
Comment 19 Makoto Kato [:m_kato] 2011-08-22 21:59:40 PDT
Comment on attachment 480632 [details] [diff] [review]
content patch

charset detection into nsDOMFile is removed by But 661876.
Comment 20 Makoto Kato [:m_kato] 2011-08-22 22:00:36 PDT
Comment on attachment 480634 [details] [diff] [review]
crashtest

Test case doesn't work by bug 661876
Comment 21 Makoto Kato [:m_kato] 2011-08-22 23:55:06 PDT
Also, FileReaderSync has no this bug because it checks numRead.
Comment 22 Masatoshi Kimura [:emk] 2011-08-23 00:04:18 PDT
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.
Comment 23 Makoto Kato [:m_kato] 2011-08-23 00:13:15 PDT
(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.
Comment 24 Makoto Kato [:m_kato] 2011-08-24 00:31:26 PDT
hg.mozilla.org/integration/mozilla-inbound/rev/0d1dd45a06fc
Comment 25 Ed Morley [:emorley] 2011-08-24 17:17:57 PDT
http://hg.mozilla.org/mozilla-central/rev/0d1dd45a06fc
Comment 26 Makoto Kato [:m_kato] 2011-08-25 03:03:54 PDT
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.
Comment 28 Trif Andrei-Alin[:AlinT] 2011-08-26 00:02:17 PDT
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 Vlad [QA] 2011-09-09 07:21:44 PDT
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.
Comment 30 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-09-19 15:43:34 PDT
Still needs verification on aurora, qa+
Comment 31 Vlad [QA] 2011-09-23 06:16:54 PDT
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

Note You need to log in before you can comment on or make changes to this bug.