Changeset 95504 in webkit


Ignore:
Timestamp:
Sep 19, 2011 6:41:42 PM (13 years ago)
Author:
barraclough@apple.com
Message:

String#split is buggy
​https://bugs.webkit.org/show_bug.cgi?id=68348

Reviewed by Sam Weinig.

Source/JavaScriptCore:

  • runtime/StringPrototype.cpp:

(JSC::jsStringWithReuse):

  • added helper function to reuse original JSString value.

(JSC::stringProtoFuncSplit):

  • Rewritten from the spec.
  • tests/mozilla/ecma/String/15.5.4.8-2.js:

(getTestCases):

  • This test is not ES5 compliant.

LayoutTests:

  • fast/js/script-tests/string-split-conformance.js: Added.
  • fast/js/string-split-conformance-expected.txt: Added.
  • fast/js/string-split-conformance.html: Added.
    • Added new Layout test based on:

​http://stevenlevithan.com/demo/split.cfm

  • sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.14_String.prototype.split/S15.5.4.14_A1_T6-expected.txt:
  • sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.14_String.prototype.split/S15.5.4.14_A1_T7-expected.txt:
  • sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.14_String.prototype.split/S15.5.4.14_A1_T8-expected.txt:
  • sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.14_String.prototype.split/S15.5.4.14_A1_T9-expected.txt:
  • sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.14_String.prototype.split/S15.5.4.14_A2_T7-expected.txt:
Location:
trunk
Files:
3 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r95494 r95504  
     12011-09-19  Gavin Barraclough  <barraclough@apple.com>
     2
     3        String#split is buggy
     4        https://bugs.webkit.org/show_bug.cgi?id=68348
     5
     6        Reviewed by Sam Weinig.
     7
     8        * fast/js/script-tests/string-split-conformance.js: Added.
     9        * fast/js/string-split-conformance-expected.txt: Added.
     10        * fast/js/string-split-conformance.html: Added.
     11            - Added new Layout test based on:
     12                http://stevenlevithan.com/demo/split.cfm
     13        * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.14_String.prototype.split/S15.5.4.14_A1_T6-expected.txt:
     14        * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.14_String.prototype.split/S15.5.4.14_A1_T7-expected.txt:
     15        * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.14_String.prototype.split/S15.5.4.14_A1_T8-expected.txt:
     16        * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.14_String.prototype.split/S15.5.4.14_A1_T9-expected.txt:
     17        * sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.14_String.prototype.split/S15.5.4.14_A2_T7-expected.txt:
     18            - Check in failing results for these 5 tests; they are all wrong
     19              (see https://bugs.ecmascript.org/show_bug.cgi?id=61).
     20
    1212011-09-19  Sheriff Bot  <webkit.review.bot@gmail.com>
    222

  • trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.14_String.prototype.split/S15.5.4.14_A1_T6-expected.txt

    r58534 r95504  
    11S15.5.4.14_A1_T6
    22
    3 PASS
     3FAIL SputnikError: #3: var x; __split = new String("1undefined").split(x); __split.length === 2. Actual: 1
    44
    55TEST COMPLETE

  • trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.14_String.prototype.split/S15.5.4.14_A1_T7-expected.txt

    r58534 r95504  
    11S15.5.4.14_A1_T7
    22
    3 PASS
     3FAIL SputnikError: #3: __split = String("undefinedd").split(undefined); __split.length === 2. Actual: 1
    44
    55TEST COMPLETE

  • trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.14_String.prototype.split/S15.5.4.14_A1_T8-expected.txt

    r58534 r95504  
    11S15.5.4.14_A1_T8
    22
    3 PASS
     3FAIL SputnikError: #3: __obj = {toString:function(){}}; __split = String(__obj).split(void 0); __split.length === 2. Actual: 1
    44
    55TEST COMPLETE

  • trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.14_String.prototype.split/S15.5.4.14_A1_T9-expected.txt

    r58534 r95504  
    11S15.5.4.14_A1_T9
    22
    3 PASS
     3FAIL SputnikError: #3: __obj = {valueOf:function(){}, toString:void 0}; __split = new String(__obj).split(function(){}()); __split.length === 2. Actual: 1
    44
    55TEST COMPLETE

  • trunk/LayoutTests/sputnik/Conformance/15_Native_Objects/15.5_String/15.5.4/15.5.4.14_String.prototype.split/S15.5.4.14_A2_T7-expected.txt

    r58534 r95504  
    11S15.5.4.14_A2_T7
    22
    3 PASS
     3FAIL SputnikError: #2: var __string = "thisundefinedisundefinedaundefinedstringundefinedobject"; var __expected = ["this", "is", "a", "string", "object"]; __split = __string.split(void 0); __split.length === __expected.length. Actual: 1
    44
    55TEST COMPLETE

  • trunk/Source/JavaScriptCore/ChangeLog

    r95503 r95504  
     12011-09-19  Gavin Barraclough  <barraclough@apple.com>
     2
     3        String#split is buggy
     4        https://bugs.webkit.org/show_bug.cgi?id=68348
     5
     6        Reviewed by Sam Weinig.
     7
     8        * runtime/StringPrototype.cpp:
     9        (JSC::jsStringWithReuse):
     10            - added helper function to reuse original JSString value.
     11        (JSC::stringProtoFuncSplit):
     12            - Rewritten from the spec.
     13        * tests/mozilla/ecma/String/15.5.4.8-2.js:
     14        (getTestCases):
     15            - This test is not ES5 compliant.
     16
    1172011-09-19  Geoffrey Garen  <ggaren@apple.com>
    218

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

    r95108 r95504  
    157157
    158158// ------------------------------ Functions --------------------------
     159
     160// Helper for producing a JSString for 'string', where 'string' was been produced by
     161// calling ToString on 'originalValue'. In cases where 'originalValue' already was a
     162// string primitive we can just use this, otherwise we need to allocate a new JSString.
     163static inline JSString* jsStringWithReuse(ExecState* exec, JSValue originalValue, const UString& string)
     164{
     165    if (originalValue.isString()) {
     166        ASSERT(asString(originalValue)->value(exec) == string);
     167        return asString(originalValue);
     168    }
     169    return jsString(exec, string);
     170}
    159171
    160172static NEVER_INLINE UString substituteBackreferencesSlow(const UString& replacement, const UString& source, const int* ovector, RegExp* reg, size_t i)
    … …  
    780792}
    781793
     794// ES 5.1 - 15.5.4.14 String.prototype.split (separator, limit)
    782795EncodedJSValue JSC_HOST_CALL stringProtoFuncSplit(ExecState* exec)
    783796{
    784     JSValue thisValue = exec->hostThisValue();
    785     if (thisValue.isUndefinedOrNull()) // CheckObjectCoercible
    786         return throwVMTypeError(exec);
    787     UString s = thisValue.toString(exec);
    788     JSGlobalData* globalData = &exec->globalData();
    789 
    790     JSValue a0 = exec->argument(0);
    791     JSValue a1 = exec->argument(1);
    792 
     797    // 1. Call CheckObjectCoercible passing the this value as its argument.
     798    JSValue thisValue = exec->hostThisValue();
     799    if (thisValue.isUndefinedOrNull())
     800        return throwVMTypeError(exec);
     801
     802    // 2. Let S be the result of calling ToString, giving it the this value as its argument.
     803    // 6. Let s be the number of characters in S.
     804    UString input = thisValue.toString(exec);
     805
     806    // 3. Let A be a new array created as if by the expression new Array()
     807    //    where Array is the standard built-in constructor with that name.
    793808    JSArray* result = constructEmptyArray(exec);
    794     unsigned i = 0;
    795     unsigned p0 = 0;
    796     unsigned limit = a1.isUndefined() ? 0xFFFFFFFFU : a1.toUInt32(exec);
    797     if (a0.inherits(&RegExpObject::s_info)) {
    798         RegExp* reg = asRegExpObject(a0)->regExp();
    799         if (s.isEmpty() && reg->match(*globalData, s, 0) >= 0) {
    800             // empty string matched by regexp -> empty array
     809
     810    // 4. Let lengthA be 0.
     811    unsigned resultLength = 0;
     812
     813    // 5. If limit is undefined, let lim = 2^32-1; else let lim = ToUint32(limit).
     814    JSValue limitValue = exec->argument(1);
     815    unsigned limit = limitValue.isUndefined() ? 0xFFFFFFFFu : limitValue.toUInt32(exec);
     816
     817    // 7. Let p = 0.
     818    size_t position = 0;
     819
     820    // 8. If separator is a RegExp object (its [[Class]] is "RegExp"), let R = separator;
     821    //    otherwise let R = ToString(separator).
     822    JSValue separatorValue = exec->argument(0);
     823    if (separatorValue.inherits(&RegExpObject::s_info)) {
     824        JSGlobalData* globalData = &exec->globalData();
     825        RegExp* reg = asRegExpObject(separatorValue)->regExp();
     826
     827        // 9. If lim == 0, return A.
     828        if (!limit)
     829            return JSValue::encode(result);
     830
     831        // 10. If separator is undefined, then
     832        if (separatorValue.isUndefined()) {
     833            // a.  Call the [[DefineOwnProperty]] internal method of A with arguments "0",
     834            //     Property Descriptor {[[Value]]: S, [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: true}, and false.
     835            result->put(exec, 0, jsStringWithReuse(exec, thisValue, input));
     836            // b.  Return A.
    801837            return JSValue::encode(result);
    802838        }
    803         unsigned pos = 0;
    804         while (i != limit && pos < s.length()) {
     839
     840        // 11. If s == 0, then
     841        if (input.isEmpty()) {
     842            // a. Call SplitMatch(S, 0, R) and let z be its MatchResult result.
     843            // b. If z is not failure, return A.
     844            // c. Call the [[DefineOwnProperty]] internal method of A with arguments "0",
     845            //    Property Descriptor {[[Value]]: S, [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: true}, and false.
     846            // d. Return A.
     847            if (reg->match(*globalData, input, 0) < 0)
     848                result->put(exec, 0, jsStringWithReuse(exec, thisValue, input));
     849            return JSValue::encode(result);
     850        }
     851
     852        // 12. Let q = p.
     853        size_t matchPosition = 0;
     854        // 13. Repeat, while q != s
     855        while (matchPosition < input.length()) {
     856            // a. Call SplitMatch(S, q, R) and let z be its MatchResult result.
    805857            Vector<int, 32> ovector;
    806             int mpos = reg->match(*globalData, s, pos, &ovector);
     858            int mpos = reg->match(*globalData, input, matchPosition, &ovector);
     859            // b. If z is failure, then let q = q + 1.
    807860            if (mpos < 0)
    808861                break;
    809             int mlen = ovector[1] - ovector[0];
    810             pos = mpos + (mlen == 0 ? 1 : mlen);
    811             if (static_cast<unsigned>(mpos) != p0 || mlen) {
    812                 result->put(exec, i++, jsSubstring(exec, s, p0, mpos - p0));
    813                 p0 = mpos + mlen;
     862            matchPosition = mpos;
     863
     864            // c. Else, z is not failure
     865            // i. z must be a State. Let e be z's endIndex and let cap be z's captures array.
     866            size_t matchEnd = ovector[1];
     867
     868            // ii. If e == p, then let q = q + 1.
     869            if (matchEnd == position) {
     870                ++matchPosition;
     871                continue;
    814872            }
    815             for (unsigned si = 1; si <= reg->numSubpatterns(); ++si) {
    816                 int spos = ovector[si * 2];
    817                 if (spos < 0)
    818                     result->put(exec, i++, jsUndefined());
    819                 else
    820                     result->put(exec, i++, jsSubstring(exec, s, spos, ovector[si * 2 + 1] - spos));
     873            // iii. Else, e != p
     874
     875            // 1. Let T be a String value equal to the substring of S consisting of the characters at positions p (inclusive)
     876            //    through q (exclusive).
     877            // 2. Call the [[DefineOwnProperty]] internal method of A with arguments ToString(lengthA),
     878            //    Property Descriptor {[[Value]]: T, [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: true}, and false.
     879            result->put(exec, resultLength, jsSubstring(exec, input, position, matchPosition - position));
     880            // 3. Increment lengthA by 1.
     881            // 4. If lengthA == lim, return A.
     882            if (++resultLength == limit)
     883                return JSValue::encode(result);
     884
     885            // 5. Let p = e.
     886            // 8. Let q = p.
     887            position = matchEnd;
     888            matchPosition = matchEnd;
     889
     890            // 6. Let i = 0.
     891            // 7. Repeat, while i is not equal to the number of elements in cap.
     892            //  a Let i = i + 1.
     893            for (unsigned i = 1; i <= reg->numSubpatterns(); ++i) {
     894                // b Call the [[DefineOwnProperty]] internal method of A with arguments
     895                //   ToString(lengthA), Property Descriptor {[[Value]]: cap[i], [[Writable]]:
     896                //   true, [[Enumerable]]: true, [[Configurable]]: true}, and false.
     897                int sub = ovector[i * 2];
     898                result->put(exec, resultLength, sub < 0 ? jsUndefined() : jsSubstring(exec, input, sub, ovector[i * 2 + 1] - sub));
     899                // c Increment lengthA by 1.
     900                // d If lengthA == lim, return A.
     901                if (++resultLength == limit)
     902                    return JSValue::encode(result);
    821903            }
    822904        }
    823905    } else {
    824         UString u2 = a0.toString(exec);
    825         if (u2.isEmpty()) {
    826             if (s.isEmpty()) {
    827                 // empty separator matches empty string -> empty array
     906        UString separator = separatorValue.toString(exec);
     907
     908        // 9. If lim == 0, return A.
     909        if (!limit)
     910            return JSValue::encode(result);
     911
     912        // 10. If separator is undefined, then
     913        JSValue separatorValue = exec->argument(0);
     914        if (separatorValue.isUndefined()) {
     915            // a.  Call the [[DefineOwnProperty]] internal method of A with arguments "0",
     916            //     Property Descriptor {[[Value]]: S, [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: true}, and false.
     917            result->put(exec, 0, jsStringWithReuse(exec, thisValue, input));
     918            // b.  Return A.
     919            return JSValue::encode(result);
     920        }
     921
     922        // 11. If s == 0, then
     923        if (input.isEmpty()) {
     924            // a. Call SplitMatch(S, 0, R) and let z be its MatchResult result.
     925            // b. If z is not failure, return A.
     926            // c. Call the [[DefineOwnProperty]] internal method of A with arguments "0",
     927            //    Property Descriptor {[[Value]]: S, [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: true}, and false.
     928            // d. Return A.
     929            if (!separator.isEmpty())
     930                result->put(exec, 0, jsStringWithReuse(exec, thisValue, input));
     931            return JSValue::encode(result);
     932        }
     933
     934        // Optimized case for splitting on the empty string.
     935        if (separator.isEmpty()) {
     936            limit = std::min(limit, input.length());
     937            // Zero limt/input length handled in steps 9/11 respectively, above.
     938            ASSERT(limit);
     939
     940            do
     941                result->put(exec, position, jsSingleCharacterSubstring(exec, input, position));
     942            while (++position < limit);
     943
     944            return JSValue::encode(result);
     945        }
     946
     947        // 12. Let q = p.
     948        size_t matchPosition;
     949        // 13. Repeat, while q != s
     950        //   a. Call SplitMatch(S, q, R) and let z be its MatchResult result.
     951        //   b. If z is failure, then let q = q+1.
     952        //   c. Else, z is not failure
     953        while ((matchPosition = input.find(separator, position)) != notFound) {
     954            // 1. Let T be a String value equal to the substring of S consisting of the characters at positions p (inclusive)
     955            //    through q (exclusive).
     956            // 2. Call the [[DefineOwnProperty]] internal method of A with arguments ToString(lengthA),
     957            //    Property Descriptor {[[Value]]: T, [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: true}, and false.
     958            result->put(exec, resultLength, jsSubstring(exec, input, position, matchPosition - position));
     959            // 3. Increment lengthA by 1.
     960            // 4. If lengthA == lim, return A.
     961            if (++resultLength == limit)
    828962                return JSValue::encode(result);
    829             }
    830             while (i != limit && p0 < s.length() - 1)
    831                 result->put(exec, i++, jsSingleCharacterSubstring(exec, s, p0++));
    832         } else {
    833             size_t pos;
    834             while (i != limit && (pos = s.find(u2, p0)) != notFound) {
    835                 result->put(exec, i++, jsSubstring(exec, s, p0, pos - p0));
    836                 p0 = pos + u2.length();
    837             }
     963
     964            // 5. Let p = e.
     965            // 8. Let q = p.
     966            position = matchPosition + separator.length();
    838967        }
    839968    }
    840969
    841     // add remaining string
    842     if (i != limit)
    843         result->put(exec, i++, jsSubstring(exec, s, p0, s.length() - p0));
    844 
     970    // 14. Let T be a String value equal to the substring of S consisting of the characters at positions p (inclusive)
     971    //     through s (exclusive).
     972    // 15. Call the [[DefineOwnProperty]] internal method of A with arguments ToString(lengthA), Property Descriptor
     973    //     {[[Value]]: T, [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: true}, and false.
     974    result->put(exec, resultLength++, jsSubstring(exec, input, position, input.length() - position));
     975
     976    // 16. Return A.
    845977    return JSValue::encode(result);
    846978}

  • trunk/Source/JavaScriptCore/tests/mozilla/ecma/String/15.5.4.8-2.js

    r11995 r95504  
    108108    }
    109109
    110     // case where the value of the separator is undefined.  in this case. the value of the separator
    111     // should be ToString( separator ), or "undefined".
     110    // Case where the value of the separator is undefined.
     111    // Per ES5 15.5.4.14 step 10 this returns the input (unless limit is non-zero).
    112112
    113113    var TEST_STRING = "thisundefinedisundefinedaundefinedstringundefinedobject";
    114     var EXPECT_STRING = new Array( "this", "is", "a", "string", "object" );
     114    var EXPECT_STRING = new Array( "thisundefinedisundefinedaundefinedstringundefinedobject" );
    115115
    116116    array[item++] = new TestCase(   SECTION,
Note: See TracChangeset for help on using the changeset viewer.