Bug 153333 - WebKitCSSMatrix transformList with calculated relative length crashes Safari.
Summary: WebKitCSSMatrix transformList with calculated relative length crashes Safari.
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: Safari 9
Hardware: Mac OS X 10.11
: P2 Normal
Assignee: Dean Jackson
URL:
Keywords: InRadar
Depends on: 154380
Blocks:
  Show dependency treegraph
 
Reported: 2016-01-21 16:29 PST by William Chen
Modified: 2022-09-30 14:56 PDT (History)
9 users (show)

See Also:


Attachments
Test case (111 bytes, text/html)
2016-01-21 16:29 PST, William Chen
no flags Details
Patch (16.03 KB, patch)
2016-02-17 17:40 PST, Dean Jackson
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description William Chen 2016-01-21 16:29:16 PST
Created attachment 269516 [details]
Test case

See attachment.
Comment 1 Alexey Proskuryakov 2016-01-25 15:45:01 PST
<rdar://problem/17198383> 

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x00007fff96e86464 WebCore::RenderStyle::fontDescription() const + 4
1   com.apple.WebCore             	0x00007fff9720ce85 WebCore::CSSPrimitiveValue::computeNonCalcLengthDouble(WebCore::CSSToLengthConversionData const&, unsigned short, double) + 85
2   com.apple.WebCore             	0x00007fff971ceb13 WebCore::CSSCalcValue::computeLengthPx(WebCore::CSSToLengthConversionData const&) const + 19
3   com.apple.WebCore             	0x00007fff97b77a27 WebCore::Length WebCore::CSSPrimitiveValue::convertToLength<26>(WebCore::CSSToLengthConversionData const&) const + 87
4   com.apple.WebCore             	0x00007fff97c6661e WebCore::transformsForValue(WebCore::CSSValue&, WebCore::CSSToLengthConversionData const&, WebCore::TransformOperations&) + 3742
5   com.apple.WebCore             	0x00007fff9701c77e WebCore::WebKitCSSMatrix::setMatrixValue(WTF::String const&, int&) + 270
6   com.apple.WebCore             	0x00007fff9701c4d0 WebCore::JSWebKitCSSMatrixConstructor::constructJSWebKitCSSMatrix(JSC::ExecState*) + 208
Comment 2 Mike Taylor 2016-01-27 11:44:54 PST
Edge throws a Syntax Error and that's what Gecko is planning on doing too, for caclated values in a transformList.
Comment 3 Myles C. Maxfield 2016-02-02 22:14:06 PST
Spec says "If parsing is not successful or any <transform-function> has <length> values without absolute length units, throw a SyntaxError exception." https://drafts.fxtf.org/geometry/#dom-dommatrix-dommatrix-transformlist
Comment 4 Dean Jackson 2016-02-17 17:40:54 PST
Created attachment 271609 [details]
Patch
Comment 5 Simon Fraser (smfr) 2016-02-17 17:49:55 PST
Comment on attachment 271609 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        using absolute length.

lengths.

> Source/WebCore/css/StyleBuilderConverter.h:358
> +    transformsForValue(value, styleResolver.state().cssToLengthConversionData(), false, operations);

boolean trap!

> Source/WebCore/css/WebKitCSSMatrix.cpp:70
> +        if (!transformsForValue(*value, CSSToLengthConversionData(), true, operations)) {

Ick. I think i want an enum here.
Comment 6 Dean Jackson 2016-02-17 18:14:18 PST
Committed r196738: <http://trac.webkit.org/changeset/196738>
Comment 7 WebKit Commit Bot 2016-02-17 21:12:07 PST
Re-opened since this is blocked by bug 154380
Comment 8 Ahmad Saleem 2022-09-30 14:56:35 PDT
Safari 16 does not crash on the test case but show following warning in console:

[Error] SyntaxError: The string did not match the expected pattern.
	DOMMatrix (attachment.cgi:5)
	Global Code (attachment.cgi:5)

While Chrome Canary 108 show following in console:

attachment.cgi?id=269516:5 Uncaught DOMException: Failed to construct 'DOMMatrix': Lengths must be absolute, not relative
    at https://bug-153333-attachments.webkit.org/attachment.cgi?id=269516:5:9

In Firefox Nightly 107, we get following:

Uncaught DOMException: An invalid or illegal string was specified

______

From Comment 01, it seems that it is expected rather than crash. Should we mark this as "RESOLVED CONFIGURATION CHANGED" since now we are doing right thing? Thanks!