Closed
Bug 867058
Opened 10 years ago
Closed 10 years ago
java.lang.IllegalArgumentException: bytes.length 0 must be a positive number at org.mozilla.gecko.gfx.BitmapUtils.decodeByteArray(BitmapUtils.java) at org.mozilla.gecko.AllPagesTab.storeFaviconsInMemCache(AllPagesTab.java)
Categories
(Firefox for Android Graveyard :: Toolbar, defect)
Tracking
(firefox22+ fixed, firefox23+ fixed, firefox24 fixed, fennec22+)
RESOLVED
FIXED
Firefox 24
People
(Reporter: scoobidiver, Assigned: bnicholson)
References
Details
(Keywords: crash, regression, topcrash, Whiteboard: [native-crash][startupcrash])
Crash Data
Attachments
(1 file)
3.87 KB,
patch
|
mfinkle
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
There's one crash in 23.0a1/20130429: bp-9c1d579a-f996-4e15-9ecc-4c5042130429. java.lang.IllegalArgumentException: bytes.length 0 must be a positive number at org.mozilla.gecko.gfx.BitmapUtils.decodeByteArray(BitmapUtils.java:25) at org.mozilla.gecko.gfx.BitmapUtils.decodeByteArray(BitmapUtils.java:20) at org.mozilla.gecko.AllPagesTab.storeFaviconsInMemCache(AllPagesTab.java:900) at org.mozilla.gecko.AllPagesTab$10.doInBackground(AllPagesTab.java:922) at org.mozilla.gecko.AllPagesTab$10.doInBackground(AllPagesTab.java:918) at org.mozilla.gecko.util.UiAsyncTask$BackgroundTaskRunnable.run(UiAsyncTask.java:48) at android.os.Handler.handleCallback(Handler.java:605) at android.os.Handler.dispatchMessage(Handler.java:92) at android.os.Looper.loop(Looper.java:137) at org.mozilla.gecko.util.GeckoBackgroundThread.run(GeckoBackgroundThread.java:32) More reports at: https://crash-stats.mozilla.com/report/list?signature=java.lang.IllegalArgumentException%3A+bytes.length+0+must+be+a+positive+number+at+org.mozilla.gecko.gfx.BitmapUtils.decodeByteArray%28BitmapUtils.java%29
Reporter | ||
Updated•10 years ago
|
status-firefox23:
--- → affected
Reporter | ||
Updated•10 years ago
|
Reporter | ||
Comment 1•10 years ago
|
||
It's currently #1 top crasher in 22.0b1 with many duplicates.
tracking-fennec: --- → ?
tracking-firefox22:
--- → ?
Keywords: topcrash
Whiteboard: [native-crash] → [native-crash][startupcrash]
Comment 2•10 years ago
|
||
Lots of dupes with 20 crashes - let's see if this persists. If so, it would be great to check for device correlations. mfinkle, who might be able to perform a code inspection?
Comment 3•10 years ago
|
||
I see one URL on crash-stats : http://www.dmi.dk/dmi/danmark/regionaludsigten/kbhnsj.htm
Reporter | ||
Comment 4•10 years ago
|
||
(In reply to Alex Keybl [:akeybl] from comment #2) > Lots of dupes with 20 crashes - let's see if this persists. If so, it would > be great to check for device correlations. It persists indeed. It occurs from Froyo to JB 4.2 on any kind of devices: Asus Nexus 7 38 Samsung Nexus 10 13 Samsung SCH-I535 11 Acer A500 11 Samsung GT-I9100 10 Samsung GT-P3110 9 ASUS Transformer Pad TF300T 8 Samsung SC-06D 6 Samsung GT-P7510 4 NEC LT-NA7 4 Samsung GT-P1000 4 ... For Aurora, it first showed up in 22.0a2/20130428 and happens one build out of four in the worst case. So the Aurora regression is: http://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=d7352cdf5e91&tochange=bf17ffb21f5e It's likely a regression from bug 863288.
Blocks: 863288
Comment 5•10 years ago
|
||
Some notes: * Fx22 (on Beta) seems to be to greatest number of recent crashes * The exception is thrown on purpose, if we find a Bytes array of zero length: http://mxr.mozilla.org/mozilla-beta/source/mobile/android/base/gfx/BitmapUtils.java#33 * The crash reports I looked at seem to be caused by bogus favicons, saved in our database: http://mxr.mozilla.org/mozilla-beta/source/mobile/android/base/awesomebar/AllPagesTab.java#834 A simple fix for this is to make the null check a bit more robust here: http://mxr.mozilla.org/mozilla-beta/source/mobile/android/base/awesomebar/AllPagesTab.java#835 if (b != null && b.length > 0) A different approach might be to scan the DB and clean out bogus favicons.
Assignee: nobody → bnicholson
Comment 6•10 years ago
|
||
I just wanted to check on how we save favicons to the DB. We appear to check for "empty" favicons: http://mxr.mozilla.org/mozilla-beta/source/mobile/android/base/Favicons.java#318 But we could be more robust by doing some checks deeper. We just save the Byte array without looking first: http://mxr.mozilla.org/mozilla-beta/source/mobile/android/base/db/LocalBrowserDB.java#768
Assignee | ||
Comment 7•10 years ago
|
||
Note that we had width/height checks before, but they were removed in bug 863288: diff --git a/mobile/android/base/awesomebar/AwesomeBarTab.java b/mobile/android/base/awesomebar/AwesomeBarTab.java --- a/mobile/android/base/awesomebar/AwesomeBarTab.java +++ b/mobile/android/base/awesomebar/AwesomeBarTab.java - Bitmap bitmap = BitmapFactory.decodeByteArray(b, 0, b.length); - if (bitmap != null && bitmap.getWidth() > 0 && bitmap.getHeight() > 0) { + Bitmap bitmap = BitmapUtils.decodeByteArray(b); + if (bitmap != null) { diff --git a/mobile/android/base/awesomebar/HistoryTab.java b/mobile/android/base/awesomebar/HistoryTab.java --- a/mobile/android/base/awesomebar/HistoryTab.java +++ b/mobile/android/base/awesomebar/HistoryTab.java - Bitmap bitmap = BitmapFactory.decodeByteArray(b, 0, b.length); - if (bitmap != null && bitmap.getWidth() > 0 && bitmap.getHeight() > 0) { + Bitmap bitmap = BitmapUtils.decodeByteArray(b); + if (bitmap != null) { etc. Looks like these checks were also guarding against bytes.length == 0 (which would make sense, since a 0x0 image should have no bytes). We still check for 0 width or height in decodeByteArray(), but we do it after the bytes.length check which is too late.
Updated•10 years ago
|
tracking-firefox23:
--- → +
Assignee | ||
Comment 8•10 years ago
|
||
Attachment #752921 -
Flags: review?(mark.finkle)
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Brian Nicholson (:bnicholson) from comment #7) > Looks like these checks were also guarding against bytes.length == 0 (which > would make sense, since a 0x0 image should have no bytes). We still check > for 0 width or height in decodeByteArray(), but we do it after the > bytes.length check which is too late. Actually, I don't think we were guarding against bytes.length == 0 at all. Before, we directly called BitmapFactory.decodeByteArray() in Android, which doesn't throw an exception for a zero-length array. This gave us back a 0x0 bitmap, which is why we needed getWidth()/getHeight() checks on the result.
Comment 10•10 years ago
|
||
Comment on attachment 752921 [details] [diff] [review] Check bytes.length before decoding favicons Note to self: These are the place where we could be getting a bogus favicon.
Attachment #752921 -
Flags: review?(mark.finkle) → review+
Updated•10 years ago
|
tracking-fennec: ? → 22+
Assignee | ||
Comment 11•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b86033ece2e4
status-firefox24:
--- → fixed
Comment 12•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b86033ece2e4
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
Should we nom this for beta/aurora?
Flags: needinfo?(bnicholson)
Assignee | ||
Comment 14•10 years ago
|
||
Comment on attachment 752921 [details] [diff] [review] Check bytes.length before decoding favicons [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 863288 User impact if declined: crashes with 0-size favicons Testing completed (on m-c, etc.): m-c Risk to taking this patch (and alternatives if risky): very low risk String or IDL/UUID changes made by this patch: none
Attachment #752921 -
Flags: approval-mozilla-beta?
Attachment #752921 -
Flags: approval-mozilla-aurora?
Flags: needinfo?(bnicholson)
Updated•10 years ago
|
Attachment #752921 -
Flags: approval-mozilla-beta?
Attachment #752921 -
Flags: approval-mozilla-beta+
Attachment #752921 -
Flags: approval-mozilla-aurora?
Attachment #752921 -
Flags: approval-mozilla-aurora+
Comment 15•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/d13ef195810b https://hg.mozilla.org/releases/mozilla-beta/rev/ae657be3a4f1
Reporter | ||
Updated•10 years ago
|
Summary: java.lang.IllegalArgumentException: bytes.length 0 must be a positive number at org.mozilla.gecko.gfx.BitmapUtils.decodeByteArray(BitmapUtils.java) → java.lang.IllegalArgumentException: bytes.length 0 must be a positive number at org.mozilla.gecko.gfx.BitmapUtils.decodeByteArray(BitmapUtils.java) at org.mozilla.gecko.AllPagesTab.storeFaviconsInMemCache(AllPagesTab.java)
Updated•2 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•