519839 - SVG fails to render correctly

Status: VERIFIED FIXED

(Core :: SVG, defect, P2)

Description

[Damian Shaw [Quan]]

Attachment: Wikimedia SVG

STR:

1. Open attached SVG
2. Observe Europe, Africa, the Americas and part of Asia display.

Actual result:

1. Only Europe renders correctly.

Could reproduce on:

Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9.2b1pre) Gecko/20090930 Namoroka/3.6b1pre (.NET CLR 3.5.30729) ID:20090930051755

Could not reproduce on Firefox 3.5 on Windows XP x64.

Comment 1

[Damian Shaw [Quan]]

Attachment: Incorrect rendering

Comment 2

[Damian Shaw [Quan]]

Attachment: Correct rendering

Comment 3

[Damian Shaw [Quan]]

Ignore the horizontal line at the top of the 2 images, just my poor screenshot taking ability.

Comment 4

[Carlos Alén Silva]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3

Same problem on Gecko 1.9.1.3/4pre on Windows XP SP 3.

Comment 5

[Takeshi Kurosawa :takenspc]

Works:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20090925 Minefield/3.7a1pre

Fails:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.3a1pre) Gecko/20090926 Minefield/3.7a1pre

Regression range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=def6be3d8b6b&tochange=149c3820e8a8

Comment 6

[Takeshi Kurosawa :takenspc]

1.9.2 branch has same regression range.
Works:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2b1pre) Gecko/20090925 Namoroka/3.6b1pre
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/37595a72841c

Fails:
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2b1pre) Gecko/20090926 Namoroka/3.6b1pre

Regression range:
http://hg.mozilla.org/releases/mozilla-1.9.2/pushloghtml?fromchange=37595a72841c&tochange=7a6b4aea8917

Perhaps, this is a regression from  Bug 414772 or bug 516396
http://hg.mozilla.org/mozilla-central/rev/ed8abff562ad
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/7a6b4aea8917

Comment 7

[Robert Longson [:longsonr]]

PR_strtod no longer accepts strings with >= 64K bytes following the check in for bug 516396

--- a/nsprpub/pr/src/misc/prdtoa.c	Fri Sep 25 18:22:48 2009 -0700
+++ b/nsprpub/pr/src/misc/prdtoa.c	Fri Sep 25 18:36:35 2009 -0700
@@ -1644,6 +1640,13 @@ PR_strtod
 
 	if (!_pr_initialized) _PR_ImplicitInitialization();
 
+	for(s = s00, i = 0; *s && i < 64 * 1024; s++, i++)
+		;
+	if (*s) {
+		PR_SetError(PR_INVALID_ARGUMENT_ERROR, 0);
+		return 0.0;
+		}
+

Here 64 * 1024 is 64 Kb and if the end of the string isn't found by then it gives up. In actual fact it needs to give up if anything that can't be part of a number (in this case a comma) is found.

Comment 8

[Jonathan Watt [:jwatt]]

Attachment: testcase with unnecessary stuff removed

Comment 9

[Wan-Teh Chang]

Robert: thanks for the explanation.  I got it now.  I will check in a fix soon.

Does SVG need to support a number longer than 64 * 1024 characters?

Comment 10

[Jonathan Watt [:jwatt]]

SVG has the same definition or <number> as CSS does:

http://www.w3.org/TR/CSS2/syndata.html#value-def-number

There are no special requirements for the number of digits in a number that should be supported, and I've not seen any content out there with stupidly large numbers of digits. There is a lot of tool generated content with crazy long "path" data though, which it seems reasonable to be able to throw that at PR_strtod.

Comment 11

[Wan-Teh Chang]

Could you test the NSPR patch in bug 516396 comment 57?  Thanks.

I just wanted to make sure PR_strtod can apply the 64K character limit
to just the number part of the input string.

Comment 12

[Jonathan Watt [:jwatt]]

That works great, thanks!

Comment 13

[Robert Longson [:longsonr]]

(In reply to comment #9)
> Robert: thanks for the explanation.  I got it now.  I will check in a fix soon.
> 
> Does SVG need to support a number longer than 64 * 1024 characters?

No, but a path is a collection of numbers. x, y, z, etc with seaparators that may be commas or letters. We parse the first number by sending the whole string to strtod then using the second strtod argument to find where to start to consume the next element.

Comment 14

[Daniel Veditz [:dveditz]]

This regression should block the 1.9.1/1.9.0 releases

Comment 15

[Daniel Veditz [:dveditz]]

This should be fixed, the change that caused this was (partially) backed-out from mozilla-central, 1.9.2 and 1.9.1 (bug 516396 comment 71). Marking "fixed1.9.0.15" because although the original patch hadn't yet landed there, we do want QA to verify that the revised patch doesn't cause this regression.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed on trunk, 1.9.2, and 1.9.1 with builds on all platforms like:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20091006 Minefield/3.7a1pre ID:20091006031035

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2b1pre) Gecko/20091006 Namoroka/3.6b1pre ID:20091006034008

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.4pre) Gecko/20090924 Shiretoko/3.5.4pre ID:20090924030717

Comment 17

[Al Billings [:abillings - ex-MoCo]]

Comparing 1.9.0.14 and 1.9.0.15 (build 3) on Windows Vista, the SVG image renders exactly the same on both.

Comment 18

[Daniel Veditz [:dveditz]]

Calling this one "fixed1.8.1.24" to get some verification love: it was a regression from an early version of bug 516396's nspr patch which we have now taken on the 1.8.1 branch.

Comment 19

[Al Billings [:abillings - ex-MoCo]]

Verified for 1.8.1.24 using Seamonkey and attached SVG. It renders correctly now.

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.24pre) Gecko/20100225 SeaMonkey/1.1.19pre