createSVGTransformFromMatrix(undefined) => NULL ptr

• Component: SVG
• Status: RESOLVED FIXED

[Berend-Jan Wever]

Created attachment 73932 [details]
Repro

Repro:
<script>
document.createElementNS("http://www.w3.org/2000/svg", "pattern").
  patternTransform.animVal.createSVGTransformFromMatrix(undefined);
</script>

id:             chrome.dll!WebCore::SVGTransformListPropertyTearOff::createSVGTransformFromMatrix ReadAV@NULL (0fa2b124867b86a588efe302a0d5ce30)
description:    Attempt to read from unallocated NULL pointer+0xC in chrome.dll!WebCore::SVGTransformListPropertyTearOff::createSVGTransformFromMatrix
stack:          chrome.dll!WebCore::SVGTransformListPropertyTearOff::createSVGTransformFromMatrix
                chrome.dll!WebCore::SVGTransformListInternal::createSVGTransformFromMatrixCallback
                chrome.dll!v8::internal::HandleApiCallHelper<...>
                chrome.dll!v8::internal::Builtin_HandleApiCall
                chrome.dll!v8::internal::Invoke
                chrome.dll!v8::internal::Execution::Call
                ...

[Rob Buis]

Some remarks on the bug.

- happens with Safari as well
- spec does not indicate what to do AFAICS
- tried with FF and Opera and both throw an exception

possible solutions:
1 have something similar to StrictTypeChecking flag but make it also
   throw on null/undefined
2 replace the assert with if (!matrix) return 0 in SVGTransformListPropertyTearO.h, fixes
   the crash but is not what other implementations do.
3 hack the bindings to do something special for SVGTransformList? Sounds more hacky than 1.

Waiting for feedback from the bindings experts :)
Cheers,

Rob.

[Nikolas Zimmermann]

Good repo, I was aware of the problem, but forgot to file a bug about it.
The ASSERTION should turn in a throw.

createSVGTransformFromMatrix has to take an ExceptionCode& parameter, that's then set to a TYPE_MISMATCH_ERR.

Sth. like
if (!matrix) { ec = TYPE_MISMATCH_ERR; return 0; }

That should fix the problem.
Just required adding "raises (DOMException)" to the createSVGTransformFromMatrix method in the SVGTransformList.idl.

Rob, do you want to fix it? I'm busy this weekend..

[Rob Buis]

Moin Niko,

(In reply to comment #2)
> Good repo, I was aware of the problem, but forgot to file a bug about it.
> The ASSERTION should turn in a throw.
> 
> createSVGTransformFromMatrix has to take an ExceptionCode& parameter, that's then set to a TYPE_MISMATCH_ERR.
> 
> Sth. like
> if (!matrix) { ec = TYPE_MISMATCH_ERR; return 0; }
> 
> That should fix the problem.
> Just required adding "raises (DOMException)" to the createSVGTransformFromMatrix method in the SVGTransformList.idl.
> 
> Rob, do you want to fix it? I'm busy this weekend..

Yep I have some time and will have a go at it :)
Cheers,

Rob.

[Rob Buis]

Created attachment 75630 [details]
Patch

[WebKit Review Bot]

Attachment 75630 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/svg/dom/SVGTransformList-expected.txt', u'LayoutTests/svg/dom/script-tests/SVGTransformList.js', u'WebCore/ChangeLog', u'WebCore/svg/SVGTransformList.idl', u'WebCore/svg/properties/SVGTransformListPropertyTearOff.h']" exit_code: 1
WebCore/svg/properties/SVGTransformListPropertyTearOff.h:43:  More than one command on the same line in if  [whitespace/parens] [4]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Nikolas Zimmermann]

Comment on attachment 75630 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=75630&action=review

Excellent, please fix the style issues before landing, r=me.

> WebCore/svg/properties/SVGTransformListPropertyTearOff.h:43
> +        if (!matrix) { ec = TYPE_MISMATCH_ERR; return 0; }

Each new statement should go into its own line.

[Rob Buis]

Landed in r73345.
Also notified chromium.

[Berend-Jan Wever]

Thanks!