100% cpu svg file in tag <animateTransform> parameter "begin"

RESOLVED FIXED in mozilla11

Status

()

Core
SVG
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: sec search, Assigned: birtles)

Tracking

7 Branch
mozilla11
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Created attachment 564186 [details]
svg0.svg

User Agent: Opera/9.80 (Windows NT 5.1; U; ru) Presto/2.9.168 Version/11.51

Steps to reproduce:

open .svg file in browser


Actual results:

100% cpu


Expected results:

browser should not load the CPU 100%

Updated

6 years ago
Component: General → SVG
Product: Firefox → Core
QA Contact: general → general

Comment 1

6 years ago
Looks like the begin time comes out as LL_MININT. This wraps around so that when you do 0 > LL_MININT and LL_MININT = 0 - LL_MININT. Perhaps SMIL needs to use LL_MININT + 1 in some places.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Updated

6 years ago
Assignee: nobody → birtles
Status: NEW → ASSIGNED
(Assignee)

Comment 2

6 years ago
Created attachment 574220 [details] [diff] [review]
Proposed patch v1a

Here's a proposed patch to catch situations where the time we're generating overflows the range of nsSMILTime and just ignore the time.

I've also tweaked the parsing method to drop the isValid flag and just return early. (When I first wrote it I was working for a company whose coding standard discouraged early returns and I mistakenly applied that here.) I've also converted a few NS_ASSERTIONs to NS_ABORT_IF_FALSE.
Attachment #574220 - Flags: review?(longsonr)

Comment 3

6 years ago
Comment on attachment 574220 [details] [diff] [review]
Proposed patch v1a

>-      if (sign != 0) {
>-        // sign has already been set
>-        isValid = false;
>-        break;
>-      }
>+      // check sign has not already been set
>+      if (sign != 0)
>+        return NS_ERROR_FAILURE;

I'd rather you kept the braces even for one line tests, and fortunately the coding style agrees with me https://developer.mozilla.org/En/Mozilla_Coding_Style_Guide :-)
Attachment #574220 - Flags: review?(longsonr) → review+
(Assignee)

Comment 4

6 years ago
(In reply to Robert Longson from comment #3)
> I'd rather you kept the braces even for one line tests, and fortunately the
> coding style agrees with me
> https://developer.mozilla.org/En/Mozilla_Coding_Style_Guide :-)

Fair enough. I was trying to match the existing style (which was written at a time where there was a specific exception to that rule for early returns. In fact, the coding style still gives an example of an un-braced early return under "Return from errors immediately" snippet #4).

I'm going to go ahead and break with the existing style and trust that we'll gradually add braces elsewhere (as well as replacing a number of uses of nsresult with bool).
(Assignee)

Comment 5

6 years ago
Pushed to m-i:
https://hg.mozilla.org/integration/mozilla-inbound/rev/36b19dca7f91
Whiteboard: [inbound]

Comment 6

6 years ago
https://hg.mozilla.org/mozilla-central/rev/36b19dca7f91
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
(Assignee)

Updated

6 years ago
Flags: in-testsuite+
OS: Windows XP → All
Hardware: x86 → All
Whiteboard: [inbound]
Depends on: 703992
You need to log in before you can comment on or make changes to this bug.