WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
6890
SVG - invalid polyline data causes hang
https://bugs.webkit.org/show_bug.cgi?id=6890
Summary
SVG - invalid polyline data causes hang
Mark Vakoc
Reported
2006-01-28 09:33:58 PST
The attached SVG will cause WebKit (lastest nightly ~1/27/2006) to hang indefinitely. It has invalid polyline data (INF as one of the coordinates). Firefox and Adobe/IE report the problem, but WebKit goes to pinwheel. WARNING: Viewing this actually causes the entire computer to hang, and had to manually power off (perhaps another issue for apple?), so I wouldn't view it outside a debugger.
Attachments
sample svg with invalid polyline data
(393 bytes, image/svg+xml)
2006-01-28 09:36 PST
,
Mark Vakoc
no flags
Details
testcases for 6890 and 6951
(2.23 KB, patch)
2006-02-13 12:21 PST
,
Alexander Kellett
no flags
Details
Formatted Diff
Diff
fix
(2.09 KB, patch)
2006-02-13 12:22 PST
,
Alexander Kellett
eric
: review-
Details
Formatted Diff
Diff
cleaned version of the method
(2.12 KB, patch)
2006-03-02 16:09 PST
,
Alexander Kellett
eric
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Mark Vakoc
Comment 1
2006-01-28 09:36:46 PST
Created
attachment 6051
[details]
sample svg with invalid polyline data
Eric Seidel (no email)
Comment 2
2006-01-28 13:12:02 PST
Reproducible crasher = SVG HitList. Addign Keyword. This shoudl be simple to fix.
Darin Adler
Comment 3
2006-02-10 09:31:53 PST
Reproducible crashers, even in SVG, are P1.
Alexander Kellett
Comment 4
2006-02-13 12:21:53 PST
Created
attachment 6463
[details]
testcases for 6890 and 6951
Alexander Kellett
Comment 5
2006-02-13 12:22:28 PST
Created
attachment 6464
[details]
fix
Eric Seidel (no email)
Comment 6
2006-02-14 03:21:27 PST
Comment on
attachment 6464
[details]
fix A couple comments: 1. I think it's better to declare optr on its own line. (make the declaration less confusing). 2. I think optr deserves a better name. Honestly, I think ptr deserves a better name... The code is hard enough to read as is... giving the variables cryptic names only hurts :( 3. While you're in there, you might as well remove the commented out //std::cout lines. Three little nits, if you were landing your own patches you could just fix them as you landed, but since someone else has to land this, I think it's best to get one more round of patches.
Eric Seidel (no email)
Comment 7
2006-02-14 03:24:34 PST
Comment on
attachment 6463
[details]
testcases for 6890 and 6951 These are fine. Ideally we would remove more of the extra junk when reducing, such as: xmlns:my="
http://mark.vakoc.com/
" xmlns:fn="
http://www.w3.org/2005/02/xpath-functions
", etc. Also no need to have svg:polyline, just polyline The second test is another example. You could remove the <![CDATA[ block as well as the xlink namespace. the tests look fine otherwise. Whoever lands these will have to actually generate the final expected results.
Eric Seidel (no email)
Comment 8
2006-02-15 01:44:27 PST
Comment on
attachment 6463
[details]
testcases for 6890 and 6951 Removing review flag from test cases. They're fine to land. Waiting still for revised patch.
Alexander Kellett
Comment 9
2006-03-02 16:09:41 PST
Created
attachment 6815
[details]
cleaned version of the method
Eric Seidel (no email)
Comment 10
2006-03-03 01:24:16 PST
Comment on
attachment 6815
[details]
cleaned version of the method svgPolyTo(xPos, xPos, segmentNum++); looks wrong. It should be svgPolyTo(xPos, yPos, segmentNum++); I think. Otherwise this looks fine. Please be sure to add the layout tests, results, and change log when landing. With that fixed, this is good to land. r=me.
Darin Adler
Comment 11
2006-03-06 09:11:38 PST
Comment on
attachment 6815
[details]
cleaned version of the method This code looks to me like it can look at a character past the end of the string. Where's the check that currSegment < eoString before looking at *currSegment?
Darin Adler
Comment 12
2006-03-06 15:08:30 PST
Alex landed this change already.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug