Bug 49564 - createSVGTransformFromMatrix(undefined) => NULL ptr
Summary: createSVGTransformFromMatrix(undefined) => NULL ptr
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows Vista
: P1 Normal
Assignee: Rob Buis
URL: http://code.google.com/p/chromium/iss...
Keywords:
Depends on:
Blocks:
 
Reported: 2010-11-15 14:53 PST by Berend-Jan Wever
Modified: 2010-12-06 06:06 PST (History)
5 users (show)

See Also:


Attachments
Repro (153 bytes, text/html)
2010-11-15 14:53 PST, Berend-Jan Wever
no flags Details
Patch (4.95 KB, patch)
2010-12-05 09:33 PST, Rob Buis
zimmermann: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Berend-Jan Wever 2010-11-15 14:53:00 PST
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
                ...
Comment 1 Rob Buis 2010-12-03 13:11:40 PST
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.
Comment 2 Nikolas Zimmermann 2010-12-05 04:48:41 PST
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..
Comment 3 Rob Buis 2010-12-05 04:52:12 PST
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.
Comment 4 Rob Buis 2010-12-05 09:33:12 PST
Created attachment 75630 [details]
Patch
Comment 5 WebKit Review Bot 2010-12-05 09:35:21 PST
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.
Comment 6 Nikolas Zimmermann 2010-12-05 11:54:30 PST
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.
Comment 7 Rob Buis 2010-12-05 12:41:46 PST
Landed in r73345.
Also notified chromium.
Comment 8 Berend-Jan Wever 2010-12-06 06:06:01 PST
Thanks!