[webkit-dev] SVG Stabilization

[Nikolas Zimmermann]

Hey Maciej,

sorry for the really late reply. But I forgot to send the reply :/
First of all I'm very happy to see there is real interesst in a WebKit release
with SVG included - after all that's why me/Rob started to work on WebKit
as our main code base :-)

> To address these concerns, a few of us came up with a plan which I'd
> like to propose now for discussion. I would especially like to hear
> from WebKit SVG hackers on this.
>
> 1) Disable newer/experimental features   (we'd probably guard these
> with a single SVG_EXPERIMENTAL_FEATURES ifdef)
>    * SVGImage -- already disabled
>    * Animation -- not tested anywhere enough yet, and still noticably
> unstable
>    * Filters -- still unstable, and can cause problems with buggy
> graphics drivers which take down the whole machine
>    * Use -- untested/unstable
>    * foreignObject -- might have issues with html inside svg,
> particularly for hit testing, etc
>

I totally agree with disabling SVGImage/Animation/Filters/foreignObject, for 
several reasons:

Animation is not even finished from a designed perspective, nor would I call
it a reliable implementation. Disabling is a must for a release.

Filters are "fishy". They don't work correctly, they leak, have SVG DOM 
updating issues, and relies on some dirty hacks. Needs to be disabled, too.

SVGImage was a quick hack to show "it works", and isn't really meant to
be enabled yet in a stable build.

foreignObject is rather untested - at least from my side. I never touched
it at all, and we already have several bug reports about problems with it.
So disabling is also a must here.

For <use> I see it a bit different. You're right that the code was a source
of crashes lately - but mostly because of the stupid fact that I forgot the
toNode() implementation (I think you've fixed it in ToT some weeks ago).

I have tested it extensively when implementing it - as it's one of the most
important features in SVG. We have Andreas Neumann around who stated
to be happy doing a lot of more <use> testing in the future.
All the carto.net examples involving <use> work excellent already.
The SVGElementInstance support is something most viewers do NOT have,
and it would be nice to have us including it in a release :-)

I hope I can get you convinced that <use> is a must - I'd be happy
to fix all issues that are reported (already looking forward to Andreas mail
flood ;-)

> 2) Additional testing
>    * Fuzz-test for custom parsers - the biggest security risk is
> buffer overruns in some of the custom parsers, so we'd like to
> develop a fuzz-testing tool for attributes that trigger these, and
> fix resulting crashes.
Definately. There have been too many crashes around involving the svg
path parsing code etc. Any idea how to develop such a fuzz-testing tool?
Simple perl/ruby/<whatever> script producing static test files?

>    * XSS testing - SVG has many elements that reference external
> content, these should be tested for cross-site scripting risk.
Fortunately we don't allow ie <tref> / <use> external references so far.
I think there is no support at all for external referencing, except the usual:
external JS scripts / external CSS sheets. Though that is using WebCore
code itself - no additional SVG trickery - so that should be safe.
SVGImage is disabled, so referencing external SVG content is not possible
- shouldn't be a problem.

Did I miss anything?

>    * Additional ad-hoc testing - we will need community help with this
Agreed.

>    * Look into generating new results for pixel tests and keep them
> passing, once experimental features are off on trunk.
Agreed.

Thanks for getting the discussion started, Maciej!
Short resume: I agree with all points of your mail, except for <use>.
That needs further discussion :-)

Niko