"ASSERTION: Attempting to allocate excessively large array" with execCommand("outdent"), combining acute accent

RESOLVED FIXED

Status

()

Core
Layout: Text
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Ehsan, Assigned: roc)

Tracking

(Blocks: 2 bugs, 5 keywords)

Trunk
x86
Mac OS X
assertion, fixed1.9.0.19, testcase, verified1.9.1, verified1.9.2
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.0.19 +
wanted1.9.0.x +
in-testsuite ?

Firefox Tracking Flags

(blocking1.9.2 .2+, status1.9.2 .2-fixed, blocking1.9.1 .9+, status1.9.1 .9-fixed)

Details

(Whiteboard: [sg:critical?], URL)

Attachments

(1 attachment)

+++ This bug was initially created as a clone of Bug #499844 +++

Testcase: https://bugzilla.mozilla.org/attachment.cgi?id=384527

###!!! ASSERTION: Attempting to allocate excessively large array: 'Error', file nsTArray.cpp, line 69

See bug 499844 comment 14, 16 and 17.
Can someone with security access make this block on bug 377438 as well?  I couldn't do it because I don't have the access.
(In reply to bug 499844 comment #17)
> If you can explain the results of your debugging in more detail, we can
> probably work out what to do. But it might be simpler just to reassign the
> second assertion to me.

Well, there were merely observations, no results per se.

After the FindClusterStart call, the properties object's iterator start becomes 0, whereas it's 1 for iter.  Therefore, the GetSkippedDistance call returns -1.  I think that it's kind of meaningless to call GetSkippedDistance with those parameters after the FindClusterStart call, but I might be wrong.  Or maybe the code is silently assuming that FindClusterStart can never modify the iterator's start in this way, but reading its implementation, it seems sort of obvious that it does.
OK. FindClusterStart walks backwards looking for the start of a cluster. The problem here is that it's walking backwards beyond the start of the text frame. We need to make it stop moving backwards when it reaches the start of the text frame.
Created attachment 427246 [details] [diff] [review]
fix
Attachment #427246 - Flags: review?(smontagu)
Whiteboard: [sg:critical?] → [sg:critical?][needs review]
Comment on attachment 427246 [details] [diff] [review]
fix

>@@ -2460,17 +2460,17 @@ PropertyProvider::GetSpacingInternal(PRU

>         PRInt32 originalOffset = run.GetOriginalOffset() + i;
>         if (IsJustifiableCharacter(mFrag, originalOffset, isCJK)) {
>           iter.SetOriginalOffset(originalOffset);
>-          FindClusterStart(mTextRun, &iter);
>+          FindClusterStart(mTextRun, run.GetOriginalOffset(), &iter);
>           PRUint32 clusterFirstChar = iter.GetSkippedOffset();
>           FindClusterEnd(mTextRun, run.GetOriginalOffset() + run.GetRunLength(), &iter);

If you're already touching this code ... I found it hard to follow which was which of originalOffset and run.GetOriginalOffset(). Can you rename the local to iterOriginalOffset or something (or maybe use two locals, runOriginalOffset and iterOriginalOffset?)
Attachment #427246 - Flags: review?(smontagu) → review+
Sure.
Whiteboard: [sg:critical?][needs review] → [sg:critical?][needs landing]
http://hg.mozilla.org/mozilla-central/rev/006e1e7dd7c1
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Whiteboard: [sg:critical?][needs landing] → [sg:critical?]
Comment on attachment 427246 [details] [diff] [review]
fix

Should be easy and safe to backport to branches.
Attachment #427246 - Flags: approval1.9.2.2?
Attachment #427246 - Flags: approval1.9.1.9?
Blocks: 377438
Do we need this in 1.9.0 too like bug 499844?
blocking1.9.1: --- → .9+
blocking1.9.2: --- → .2+
status1.9.1: --- → wanted
status1.9.2: --- → wanted
Flags: wanted1.9.0.x?
Flags: blocking1.9.0.19?
Comment on attachment 427246 [details] [diff] [review]
fix

Approved for 1.9.1.9 and 1.9.2.2, a=dveditz for release-drivers
Attachment #427246 - Flags: approval1.9.2.2?
Attachment #427246 - Flags: approval1.9.2.2+
Attachment #427246 - Flags: approval1.9.1.9?
Attachment #427246 - Flags: approval1.9.1.9+
Whiteboard: [sg:critical?] → [sg:critical?][needs landing]
Whiteboard: [sg:critical?][needs landing] → [sg:critical?][needs 192 landing][needs 191 landing]
Whiteboard: [sg:critical?][needs 192 landing][needs 191 landing] → [sg:critical?][needs 192 landing][needs 191 landing][needs answer to comment 9 from roc]
Comment on attachment 427246 [details] [diff] [review]
fix

Yes.
Attachment #427246 - Flags: approval1.9.1.10?
Whiteboard: [sg:critical?][needs 192 landing][needs 191 landing][needs answer to comment 9 from roc] → [sg:critical?][needs 192 landing][needs 191 landing]
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/36b685937fc6
status1.9.2: wanted → .2-fixed
Whiteboard: [sg:critical?][needs 192 landing][needs 191 landing] → [sg:critical?][needs 191 landing]
Comment on attachment 427246 [details] [diff] [review]
fix

a=beltzner for 1.9.0.19
Attachment #427246 - Flags: approval1.9.1.10? → approval1.9.0.19+
Flags: wanted1.9.0.x?
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.19?
Flags: blocking1.9.0.19+
Whiteboard: [sg:critical?][needs 191 landing] → [sg:critical?][needs 191/190 landing]
Keywords: checkin-needed
The patch does not apply cleanly on either 1.9.1 or 1.9.0.
Whiteboard: [sg:critical?][needs 191/190 landing] → [sg:critical?][needs 191 landing][needs 190 landing]
There's a hunk in there that was cleaning up warnings. It's not needed for 1.9.0/1.9.1.

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a0091c234b2b
Also checked into 1.9.0 CVS.
Keywords: checkin-needed → fixed1.9.0.19
Whiteboard: [sg:critical?][needs 191 landing][needs 190 landing] → [sg:critical?]
status1.9.1: wanted → .9-fixed
Verified for 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.9pre) Gecko/20100312 Shiretoko/3.5.9pre. 

Verified for 1.9.2 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.2pre) Gecko/20100312 Namoroka/3.6.2pre. 

One 1.9.0, built today, when I run the testcase (https://bugzilla.mozilla.org/attachment.cgi?id=384527), I see:

###!!! ASSERTION: Attempting to allocate excessively large array: 'Error', file nsTArray.cpp, line 69

So, it looks like it isn't fixed on 1.9.0.
Keywords: verified1.9.1, verified1.9.2
Group: core-security
You need to log in before you can comment on or make changes to this bug.