Changeset 200423 in webkit


Ignore:
Timestamp:
May 4, 2016 12:23:41 PM (8 years ago)
Author:
mark.lam@apple.com
Message:

ES6 Function.name inferred from property names of literal objects can break some websites.
​https://bugs.webkit.org/show_bug.cgi?id=157246

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

Specifically, the library mathjs (see ​http://mathjs.org and ​https://github.com/josdejong/mathjs)
uses an idiom where it created literal objects with property names that look like
this: 'number | BigNumber | Unit'. Later, this name is used in a string to create
function source code that gets eval'ed. Since 'number | BigNumber | Unit' is not
a valid function name, we get a syntax error.

Here are the details:

  1. mathjs uses object literals with the funky property names for its function members. For example,

helper function to type check the middle value of the array
var middle = typed({

'number | BigNumber | Unit': function (value) {

return value;

}

});

  1. mathjs' getName() uses Function.name to get the name of functions (hence, picks up the property name as inferred value of Function.name as specified by ES6):

/

  • Retrieve the function name from a set of functions, and check
  • whether the name of all functions match (if given) ... */

function getName (fns) {

var name = ;

for (var i = 0; i < fns.length; i++) {

var fn = fns[i];
...

name = fn.name;

...

return name;

}

  1. mathjs uses that name to assembler new function source code that gets eval'ed:

/

  • Compose a function from sub-functions each handling a single type signature. ... */

function _typed(name, signatures) {

...
generate code for the typed function
var code = [];

var _name = name
;

...
code.push('function ' + _name + '(' + _args.join(', ') + ') {');
code.push(' "use strict";');
code.push(' var name = \ + _name + '\';');
code.push(node.toCode(refs, ' '));
code.push('}');

generate body for the factory function
var body = [

refs.toCode(),
'return ' + code.join('\n')

].join('\n');

evaluate the JavaScript code and attach function references
var factory = (new Function(refs.name, 'createError', body)); <== Syntax Error here!
var fn = factory(refs, createError);
...
return fn;

}

Until mathjs (and any other frameworks that does similar things) and sites that
uses mathjs has been updated to work with ES6, we'll need a compatibility hack to
work around it.

Here's what we'll do:

  1. Introduce a needsSiteSpecificQuirks flag in JSGlobalObject.
  2. Have WebCore's JSDOMWindowBase set that flag if the browser's needsSiteSpecificQuirks is enabled in its settings.
  3. If needsSiteSpecificQuirks is enabled, have JSFunction::reifyName() check for ' ' or '|' in the name string it will use to reify the Function.name property. If those characters exists in the name, we'll replace the name string with a null string.
  • runtime/JSFunction.cpp:

(JSC::JSFunction::reifyName):

  • runtime/JSGlobalObject.h:

(JSC::JSGlobalObject::needsSiteSpecificQuirks):
(JSC::JSGlobalObject::GlobalPropertyInfo::GlobalPropertyInfo):
(JSC::JSGlobalObject::setNeedsSiteSpecificQuirks):

Source/WebCore:

Test: js/dom/regress-157246.html

  • bindings/js/JSDOMWindowBase.cpp:

(WebCore::JSDOMWindowBase::finishCreation):

  • Set the needsSiteSpecificQuirks flag in the JSGlobalObject if needed.

Tools:

  • WebKitTestRunner/TestController.cpp:

(WTR::TestController::resetPreferencesToConsistentValues):
(WTR::updateTestOptionsFromTestHeader):

  • WebKitTestRunner/TestOptions.h:
  • WebKitTestRunner/ios/PlatformWebViewIOS.mm:

(WTR::PlatformWebView::viewSupportsOptions):

  • WebKitTestRunner/mac/PlatformWebViewMac.mm:

(WTR::PlatformWebView::viewSupportsOptions):

  • Add needsSiteSpecificQuirks to WKTR options that can be set.

LayoutTests:

  • js/dom/regress-157246-expected.txt: Added.
  • js/dom/regress-157246.html: Added.
  • js/dom/script-tests/regress-157246.js: Added.
  • platform/ios-simulator-wk1/TestExpectations:
  • platform/mac-wk1/TestExpectations:
  • Skip js/dom/regress-157246.html for wk1 because it relies on a WKTR feature to enable the needsSiteSpecificQuirks settings before running the test.
Location:
trunk
Files:
3 added
13 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r200422 r200423  
     12016-05-04  Mark Lam  <mark.lam@apple.com>
     2
     3        ES6 Function.name inferred from property names of literal objects can break some websites.
     4        https://bugs.webkit.org/show_bug.cgi?id=157246
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        * js/dom/regress-157246-expected.txt: Added.
     9        * js/dom/regress-157246.html: Added.
     10        * js/dom/script-tests/regress-157246.js: Added.
     11
     12        * platform/ios-simulator-wk1/TestExpectations:
     13        * platform/mac-wk1/TestExpectations:
     14        - Skip js/dom/regress-157246.html for wk1 because it relies on a WKTR feature to
     15          enable the needsSiteSpecificQuirks settings before running the test.
     16
    1172016-05-04  Keith Miller  <keith_miller@apple.com>
    218

  • trunk/LayoutTests/platform/ios-simulator-wk1/TestExpectations

    r200146 r200423  
    13601360
    13611361webkit.org/b/137572 scrollbars/scrollbar-iframe-click-does-not-blur-content.html [ Failure ]
     1362
     1363# This test relies on a settings option that we can only set with WKRT.
     1364js/dom/regress-157246.html

  • trunk/LayoutTests/platform/mac-wk1/TestExpectations

    r200199 r200423  
    214214
    215215webkit.org/b/157007 fast/layers/no-clipping-overflow-hidden-added-after-transform.html [ Pass ImageOnlyFailure ]
     216
     217# This test relies on a settings option that we can only set with WKRT.
     218js/dom/regress-157246.html

  • trunk/Source/JavaScriptCore/ChangeLog

    r200422 r200423  
     12016-05-04  Mark Lam  <mark.lam@apple.com>
     2
     3        ES6 Function.name inferred from property names of literal objects can break some websites.
     4        https://bugs.webkit.org/show_bug.cgi?id=157246
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Specifically, the library mathjs (see http://mathjs.org and https://github.com/josdejong/mathjs)
     9        uses an idiom where it created literal objects with property names that look like
     10        this: 'number | BigNumber | Unit'.  Later, this name is used in a string to create
     11        function source code that gets eval'ed.  Since 'number | BigNumber | Unit' is not
     12        a valid function name, we get a syntax error.
     13
     14        Here are the details:
     15
     16        1. mathjs uses object literals with the funky property names for its function members.
     17           For example,
     18
     19              // helper function to type check the middle value of the array
     20              var middle = typed({
     21                'number | BigNumber | Unit': function (value) {
     22                  return value;
     23                }
     24              });
     25
     26        2. mathjs' getName() uses Function.name to get the name of functions (hence, picks
     27           up the property name as inferred value of Function.name as specified by ES6):
     28
     29                /**
     30                 * Retrieve the function name from a set of functions, and check
     31                 * whether the name of all functions match (if given)
     32                 ...
     33                 */
     34                function getName (fns) {
     35                  var name = '';
     36
     37                  for (var i = 0; i < fns.length; i++) {
     38                    var fn = fns[i];
     39                    ...
     40                        name = fn.name;
     41                    ...
     42                  return name;
     43                }
     44
     45        3. mathjs uses that name to assembler new function source code that gets eval'ed:
     46
     47                /**
     48                 * Compose a function from sub-functions each handling a single type signature.
     49                 ...
     50                 */
     51                function _typed(name, signatures) {
     52                  ...
     53                  // generate code for the typed function
     54                  var code = [];
     55                  var _name = name || '';
     56                  ...
     57                  code.push('function ' + _name + '(' + _args.join(', ') + ') {');
     58                  code.push('  "use strict";');
     59                  code.push('  var name = \'' + _name + '\';');
     60                  code.push(node.toCode(refs, '  '));
     61                  code.push('}');
     62
     63                  // generate body for the factory function
     64                  var body = [
     65                    refs.toCode(),
     66                    'return ' + code.join('\n')
     67                  ].join('\n');
     68
     69                  // evaluate the JavaScript code and attach function references
     70                  var factory = (new Function(refs.name, 'createError', body));  // <== Syntax Error here!
     71                  var fn = factory(refs, createError);
     72                  ...
     73                  return fn;
     74                }
     75
     76        Until mathjs (and any other frameworks that does similar things) and sites that
     77        uses mathjs has been updated to work with ES6, we'll need a compatibility hack to
     78        work around it.
     79
     80        Here's what we'll do:
     81        1. Introduce a needsSiteSpecificQuirks flag in JSGlobalObject.
     82        2. Have WebCore's JSDOMWindowBase set that flag if the browser's
     83           needsSiteSpecificQuirks is enabled in its settings.
     84        3. If needsSiteSpecificQuirks is enabled, have JSFunction::reifyName() check for
     85           ' ' or '|' in the name string it will use to reify the Function.name property.
     86           If those characters exists in the name, we'll replace the name string with a
     87           null string.
     88
     89        * runtime/JSFunction.cpp:
     90        (JSC::JSFunction::reifyName):
     91        * runtime/JSGlobalObject.h:
     92        (JSC::JSGlobalObject::needsSiteSpecificQuirks):
     93        (JSC::JSGlobalObject::GlobalPropertyInfo::GlobalPropertyInfo):
     94        (JSC::JSGlobalObject::setNeedsSiteSpecificQuirks):
     95
    1962016-05-04  Keith Miller  <keith_miller@apple.com>
    297

  • trunk/Source/JavaScriptCore/runtime/JSFunction.cpp

    r200416 r200423  
    632632    const Identifier& propID = exec->propertyNames().name;
    633633
     634    if (exec->lexicalGlobalObject()->needsSiteSpecificQuirks()) {
     635        auto illegalCharMatcher = [] (UChar ch) -> bool {
     636            return ch == ' ' || ch == '|';
     637        };
     638        if (name.find(illegalCharMatcher) != notFound)
     639            name = String();
     640    }
     641   
    634642    if (jsExecutable()->isGetter())
    635643        name = makeString("get ", name);

  • trunk/Source/JavaScriptCore/runtime/JSGlobalObject.h

    r200422 r200423  
    718718    UnlinkedModuleProgramCodeBlock* createModuleProgramCodeBlock(CallFrame*, ModuleProgramExecutable*);
    719719
     720    bool needsSiteSpecificQuirks() const { return m_needsSiteSpecificQuirks; }
     721
    720722protected:
    721723    struct GlobalPropertyInfo {
    … …  
    735737    JS_EXPORT_PRIVATE static JSC::JSValue toThis(JSC::JSCell*, JSC::ExecState*, ECMAMode);
    736738
     739    void setNeedsSiteSpecificQuirks(bool needQuirks) { m_needsSiteSpecificQuirks = needQuirks; }
     740
    737741private:
    738742    friend class LLIntOffsetsExtractor;
    … …  
    746750
    747751    JS_EXPORT_PRIVATE static void clearRareData(JSCell*);
     752
     753    bool m_needsSiteSpecificQuirks { false };
    748754};
    749755

  • trunk/Source/WebCore/ChangeLog

    r200417 r200423  
     12016-05-04  Mark Lam  <mark.lam@apple.com>
     2
     3        ES6 Function.name inferred from property names of literal objects can break some websites.
     4        https://bugs.webkit.org/show_bug.cgi?id=157246
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        Test: js/dom/regress-157246.html
     9
     10        * bindings/js/JSDOMWindowBase.cpp:
     11        (WebCore::JSDOMWindowBase::finishCreation):
     12        - Set the needsSiteSpecificQuirks flag in the JSGlobalObject if needed.
     13
    1142016-05-04  Konstantin Tokarev  <annulen@yandex.ru>
    215

  • trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp

    r200121 r200423  
    8686
    8787    addStaticGlobals(staticGlobals, WTF_ARRAY_LENGTH(staticGlobals));
     88
     89    if (m_wrapped && m_wrapped->frame() && m_wrapped->frame()->settings().needsSiteSpecificQuirks())
     90        setNeedsSiteSpecificQuirks(true);
    8891}
    8992

  • trunk/Tools/ChangeLog

    r200415 r200423  
     12016-05-04  Mark Lam  <mark.lam@apple.com>
     2
     3        ES6 Function.name inferred from property names of literal objects can break some websites.
     4        https://bugs.webkit.org/show_bug.cgi?id=157246
     5
     6        Reviewed by Geoffrey Garen.
     7
     8        * WebKitTestRunner/TestController.cpp:
     9        (WTR::TestController::resetPreferencesToConsistentValues):
     10        (WTR::updateTestOptionsFromTestHeader):
     11        * WebKitTestRunner/TestOptions.h:
     12
     13        * WebKitTestRunner/ios/PlatformWebViewIOS.mm:
     14        (WTR::PlatformWebView::viewSupportsOptions):
     15        * WebKitTestRunner/mac/PlatformWebViewMac.mm:
     16        (WTR::PlatformWebView::viewSupportsOptions):
     17        - Add needsSiteSpecificQuirks to WKTR options that can be set.
     18
    1192016-05-04  Joanmarie Diggs  <jdiggs@igalia.com>
    220

  • trunk/Tools/WebKitTestRunner/TestController.cpp

    r200116 r200423  
    667667    WKPreferencesSetInteractiveFormValidationEnabled(preferences, true);
    668668    WKPreferencesSetMockScrollbarsEnabled(preferences, options.useMockScrollbars);
     669    WKPreferencesSetNeedsSiteSpecificQuirks(preferences, options.needsSiteSpecificQuirks);
    669670
    670671    static WKStringRef defaultTextEncoding = WKStringCreateWithUTF8CString("ISO-8859-1");
    … …  
    956957        if (key == "useMockScrollbars")
    957958            testOptions.useMockScrollbars = parseBooleanTestHeaderValue(value);
     959        if (key == "needsSiteSpecificQuirks")
     960            testOptions.needsSiteSpecificQuirks = parseBooleanTestHeaderValue(value);
    958961        pairStart = pairEnd + 1;
    959962    }

  • trunk/Tools/WebKitTestRunner/TestOptions.h

    r200116 r200423  
    4242    bool useDataDetection { false };
    4343    bool useMockScrollbars { true };
     44    bool needsSiteSpecificQuirks { false };
    4445
    4546    Vector<String> overrideLanguages;

  • trunk/Tools/WebKitTestRunner/ios/PlatformWebViewIOS.mm

    r190065 r200423  
    213213bool PlatformWebView::viewSupportsOptions(const TestOptions& options) const
    214214{
    215     if (m_options.overrideLanguages != options.overrideLanguages)
     215    if (m_options.overrideLanguages != options.overrideLanguages || m_options.needsSiteSpecificQuirks != options.needsSiteSpecificQuirks)
    216216        return false;
    217217

  • trunk/Tools/WebKitTestRunner/mac/PlatformWebViewMac.mm

    r200116 r200423  
    234234bool PlatformWebView::viewSupportsOptions(const TestOptions& options) const
    235235{
    236     if (m_options.useThreadedScrolling != options.useThreadedScrolling || m_options.overrideLanguages != options.overrideLanguages || m_options.useMockScrollbars != options.useMockScrollbars)
     236    if (m_options.useThreadedScrolling != options.useThreadedScrolling || m_options.overrideLanguages != options.overrideLanguages || m_options.useMockScrollbars != options.useMockScrollbars || m_options.needsSiteSpecificQuirks != options.needsSiteSpecificQuirks)
    237237        return false;
    238238
Note: See TracChangeset for help on using the changeset viewer.