Closed Bug 585185 Opened 14 years ago Closed 11 years ago

ABORT: negative lengths and percents should be rejected by parser: 'sizeValue->IsCalcUnit()' setting text-indent, negative font size and huge line-height

Categories

(Core :: CSS Parsing and Computation, defect)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla24

People

(Reporter: posidron, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase, Whiteboard: [fuzzblocker])

Attachments

(3 files)

callstack
(deleted), text/plain
Details
testcase
(deleted), text/html
Details
fix v1
(deleted), patch
dbaron
: review+
Details | Diff | Splinter Review
Attached file callstack (deleted) —
<html> <body> <span style="text-indent:21cm;font:700 -1%/4294967295pt Helvetica Arial monospace"> </body> </html>
See Also: → https://bugzilla.mozilla.org/show_bug.cgi?id=576927
Summary: *((int *) 0) [@TouchBadMemory] → "ABORT: negative lengths and percents should be rejected by parser" setting text-indent, negative font size and huge line-height
QA Contact: general → style-system
Confirmed on a Win 7 PC as well. Setting just a huge line-height (>40000000px works) causes a hang.
Component: DOM: CSS Object Model → Style System (CSS)
Keywords: assertion, testcase
<a style="font: -2px Verdana;"> is sufficient to get ABORT: negative lengths and percents should be rejected by parser: 'sizeValue->IsCalcUnit()', file /work/mozilla/builds/nightly/mozilla/layout/style/nsRuleNode.cpp, line 2559 I think the original abort ABORT: negative lengths and percents should be rejected by parser: 'aFontData.mSize.IsCalcUnit()' has been replaced with this abort.
OS: Mac OS X → All
Hardware: x86_64 → All
Summary: "ABORT: negative lengths and percents should be rejected by parser" setting text-indent, negative font size and huge line-height → ABORT: negative lengths and percents should be rejected by parser: 'sizeValue->IsCalcUnit()' setting text-indent, negative font size and huge line-height
Attached file testcase (deleted) —
ABORT: negative lengths and percents should be rejected by parser: 'sizeValue->IsCalcUnit()' still reproducible on Beta/11, Aurora/12, Nightly/13 On Linux, Mac, Windows.
Blocks: fuzzing-fonts
Whiteboard: [fuzzblocker]
Attached patch fix v1 (deleted) — — Splinter Review
Looks like we're inconsistent on whether font-size has to be non-negative or not. In nsCSSPropList, we say it's non-negative: http://mxr.mozilla.org/mozilla-central/source/layout/style/nsCSSPropList.h?rev=ae6f8ea61f33&mark=1739-1739#1734 ...but in the font shorthand parsing code, we don't currently have that restriction for font-size (we call ParseVariant instead of ParseNonNegativeVariant): http://mxr.mozilla.org/mozilla-central/source/layout/style/nsCSSParser.cpp?rev=ae6f8ea61f33&mark=8613-8613#8611 I think that last part is the bug here. Here's a patch to fix it -- let's see how Try likes it: https://tbpl.mozilla.org/?tree=Try&rev=9e6ab6065aba
Comment on attachment 753123 [details] [diff] [review] fix v1 Hooray, try likes it! The code has been this way since forever, which makes me slightly wonder if this change will be web-compatible. I suppose we can optimistically hope that it will be, for consistency / correctness. Just as more backup for this being correct -- note that the CSS 2.1 spec does explicitly say "Negative values are not allowed" for the font-size property: http://www.w3.org/TR/CSS21/fonts.html#font-size-props
Attachment #753123 - Flags: review?(bzbarsky)
(In reply to Daniel Holbert [:dholbert] from comment #6) > The code has been this way since forever, which makes me slightly wonder if > this change will be web-compatible. I suppose we can optimistically hope > that it will be, for consistency / correctness. (...and if we end up finding that it's not web-compatible, it's a trivial enough patch that we can easily back it out in aurora/beta without too much worry)
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Comment on attachment 753123 [details] [diff] [review] fix v1 Good catch. Thanks for fixing this. r=dbaron
Attachment #753123 - Flags: review?(bzbarsky) → review+
Thanks! (I just noticed I'd used slightly wonky indentation in the c++ code for some reason. I fixed it before landing.) Pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/f857b6882f4b
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/f857b6882f4b
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: