Changeset 181105 in webkit


Ignore:
Timestamp:
Mar 5, 2015 3:34:56 PM (9 years ago)
Author:
benjamin@webkit.org
Message:

Regression(r173761): ASSERTION FAILED: !is8Bit() in StringImpl::characters16()
​https://bugs.webkit.org/show_bug.cgi?id=142350

Patch by Chris Dumez <Chris Dumez> on 2015-03-05
Reviewed by Michael Saboff and Benjamin Poulain.

Source/JavaScriptCore:

Call WTFString::hasInfixStartingAt() / hasInfixEndingAt() now that these
methods have been renamed for clarity.

  • runtime/StringPrototype.cpp:

(JSC::stringProtoFuncStartsWith):
(JSC::stringProtoFuncEndsWith):

Source/WTF:

Fix ASSERTION FAILED: !is8Bit() in StringImpl::characters16() from
WTF::equalInner() after r173761. The code was incorrectly assuming that
if stringImpl is 16-bit, then matchString is 16-bit too, which is not
correct.

Also rename WTFString::startsWith() / endsWith() taking an offset to
hasInfixStartingAt() / hasInfixEndingAt() for clarity. It seems odd
to call it startsWith even though it won't technically *start* with
the pattern if the input offset is greater than zero.

Also drop the caseSensitive argument as it is never used (always true
at call sites.

  • wtf/text/StringImpl.cpp:

(WTF::equalInner):
(WTF::StringImpl::hasInfixStartingAt):
(WTF::StringImpl::hasInfixEndingAt):
(WTF::StringImpl::startsWith): Deleted.
(WTF::StringImpl::endsWith): Deleted.

  • wtf/text/StringImpl.h:
  • wtf/text/WTFString.h:

(WTF::String::hasInfixStartingAt):
(WTF::String::hasInfixEndingAt):
(WTF::String::startsWith): Deleted.
(WTF::String::endsWith): Deleted.

Tools:

Add API test for WTFString::hasInfixStartingAt() to make sure it doesn't
crash if the string is 8-bit but the pattern is 16-bit (and vice-versa).

  • TestWebKitAPI/Tests/WTF/WTFString.cpp:

(TestWebKitAPI::TEST):

LayoutTests:

Update String.startsWith() / endsWith() test to cover cases where the
input string is 8-bit and the pattern is 16-bit, and vice-versa.

  • js/script-tests/string-includes.js:
  • js/string-includes-expected.txt:
Location:
trunk
Files:
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r181096 r181105  
     12015-03-05  Chris Dumez  <cdumez@apple.com>
     2
     3        Regression(r173761): ASSERTION FAILED: !is8Bit() in StringImpl::characters16()
     4        https://bugs.webkit.org/show_bug.cgi?id=142350
     5
     6        Reviewed by Michael Saboff and Benjamin Poulain.
     7
     8        Update String.startsWith() / endsWith() test to cover cases where the
     9        input string is 8-bit and the pattern is 16-bit, and vice-versa.
     10
     11        * js/script-tests/string-includes.js:
     12        * js/string-includes-expected.txt:
     13
    1142015-03-05  Roger Fong  <roger_fong@apple.com>
    215

  • trunk/LayoutTests/js/script-tests/string-includes.js

    r177856 r181105  
    5555shouldBe("'フーバー'.startsWith('フー')", "true");
    5656shouldBe("'フーバー'.startsWith('バー')", "false");
     57shouldBe("'フーバー'.startsWith('abc')", "false");
     58shouldBe("'フーバー'.startsWith('abc', 1)", "false");
     59shouldBe("'foo bar'.startsWith('フー')", "false");
     60shouldBe("'foo bar'.startsWith('フー', 1)", "false");
    5761
    5862// Test endsWith
    … …  
    8387shouldBe("'フーバー'.endsWith('バー')", "true");
    8488shouldBe("'フーバー'.endsWith('フー')", "false");
     89shouldBe("'フーバー'.endsWith('abc')", "false");
     90shouldBe("'フーバー'.endsWith('abc')", "false");
     91shouldBe("'foo bar'.endsWith('フー')", "false");
     92shouldBe("'foo bar'.endsWith('フー', 3)", "false");
    8593
    8694// Call functions with an environment record as 'this'.

  • trunk/LayoutTests/js/string-includes-expected.txt

    r177856 r181105  
    5555PASS 'フーバー'.startsWith('フー') is true
    5656PASS 'フーバー'.startsWith('バー') is false
     57PASS 'フーバー'.startsWith('abc') is false
     58PASS 'フーバー'.startsWith('abc', 1) is false
     59PASS 'foo bar'.startsWith('フー') is false
     60PASS 'foo bar'.startsWith('フー', 1) is false
    5761PASS 'foo bar'.endsWith('bar') is true
    5862PASS 'foo bar'.endsWith('ba', 6) is true
    … …  
    8185PASS 'フーバー'.endsWith('バー') is true
    8286PASS 'フーバー'.endsWith('フー') is false
     87PASS 'フーバー'.endsWith('abc') is false
     88PASS 'フーバー'.endsWith('abc') is false
     89PASS 'foo bar'.endsWith('フー') is false
     90PASS 'foo bar'.endsWith('フー', 3) is false
    8391PASS (function() { var f = String.prototype.startsWith; (function() { f('a'); })(); })() threw exception TypeError: Type error.
    8492PASS (function() { var f = String.prototype.endsWith; (function() { f('a'); })(); })() threw exception TypeError: Type error.

  • trunk/Source/JavaScriptCore/ChangeLog

    r181084 r181105  
     12015-03-05  Chris Dumez  <cdumez@apple.com>
     2
     3        Regression(r173761): ASSERTION FAILED: !is8Bit() in StringImpl::characters16()
     4        https://bugs.webkit.org/show_bug.cgi?id=142350
     5
     6        Reviewed by Michael Saboff and Benjamin Poulain.
     7
     8        Call WTFString::hasInfixStartingAt() / hasInfixEndingAt() now that these
     9        methods have been renamed for clarity.
     10
     11        * runtime/StringPrototype.cpp:
     12        (JSC::stringProtoFuncStartsWith):
     13        (JSC::stringProtoFuncEndsWith):
     14
    1152015-03-05  Yusuke Suzuki  <utatane.tea@gmail.com>
    216

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

    r181084 r181105  
    16531653        return JSValue::encode(jsUndefined());
    16541654
    1655     return JSValue::encode(jsBoolean(stringToSearchIn.startsWith(searchString, start, true)));
     1655    return JSValue::encode(jsBoolean(stringToSearchIn.hasInfixStartingAt(searchString, start)));
    16561656}
    16571657
    … …  
    16811681    unsigned end = std::min<unsigned>(std::max(pos, 0), length);
    16821682
    1683     return JSValue::encode(jsBoolean(stringToSearchIn.endsWith(searchString, end, true)));
     1683    return JSValue::encode(jsBoolean(stringToSearchIn.hasInfixEndingAt(searchString, end)));
    16841684}
    16851685

  • trunk/Source/WTF/ChangeLog

    r181086 r181105  
     12015-03-05  Chris Dumez  <cdumez@apple.com>
     2
     3        Regression(r173761): ASSERTION FAILED: !is8Bit() in StringImpl::characters16()
     4        https://bugs.webkit.org/show_bug.cgi?id=142350
     5
     6        Reviewed by Michael Saboff and Benjamin Poulain.
     7
     8        Fix ASSERTION FAILED: !is8Bit() in StringImpl::characters16() from
     9        WTF::equalInner() after r173761. The code was incorrectly assuming that
     10        if stringImpl is 16-bit, then matchString is 16-bit too, which is not
     11        correct.
     12
     13        Also rename WTFString::startsWith() / endsWith() taking an offset to
     14        hasInfixStartingAt() / hasInfixEndingAt() for clarity. It seems odd
     15        to call it startsWith even though it won't technically *start* with
     16        the pattern if the input offset is greater than zero.
     17
     18        Also drop the caseSensitive argument as it is never used (always true
     19        at call sites.
     20
     21        * wtf/text/StringImpl.cpp:
     22        (WTF::equalInner):
     23        (WTF::StringImpl::hasInfixStartingAt):
     24        (WTF::StringImpl::hasInfixEndingAt):
     25        (WTF::StringImpl::startsWith): Deleted.
     26        (WTF::StringImpl::endsWith): Deleted.
     27        * wtf/text/StringImpl.h:
     28        * wtf/text/WTFString.h:
     29        (WTF::String::hasInfixStartingAt):
     30        (WTF::String::hasInfixEndingAt):
     31        (WTF::String::startsWith): Deleted.
     32        (WTF::String::endsWith): Deleted.
     33
    1342015-03-05  Chris Dumez  <cdumez@apple.com>
    235

  • trunk/Source/WTF/wtf/text/StringImpl.cpp

    r179429 r181105  
    13581358}
    13591359
    1360 ALWAYS_INLINE static bool equalInner(StringImpl& stringImpl, unsigned startOffset, StringImpl& matchString, bool caseSensitive)
     1360ALWAYS_INLINE static bool equalInner(const StringImpl& stringImpl, unsigned startOffset, const StringImpl& matchString)
    13611361{
    13621362    if (startOffset > stringImpl.length())
    … …  
    13671367        return false;
    13681368
    1369     if (caseSensitive) {
    1370         if (stringImpl.is8Bit())
     1369    if (stringImpl.is8Bit()) {
     1370        if (matchString.is8Bit())
    13711371            return equal(stringImpl.characters8() + startOffset, matchString.characters8(), matchString.length());
    1372         return equal(stringImpl.characters16() + startOffset, matchString.characters16(), matchString.length());
    1373     }
    1374     if (stringImpl.is8Bit())
    1375         return equalIgnoringCase(stringImpl.characters8() + startOffset, matchString.characters8(), matchString.length());
    1376     return equalIgnoringCase(stringImpl.characters16() + startOffset, matchString.characters16(), matchString.length());
     1372        return equal(stringImpl.characters8() + startOffset, matchString.characters16(), matchString.length());
     1373    }
     1374    if (matchString.is8Bit())
     1375        return equal(stringImpl.characters16() + startOffset, matchString.characters8(), matchString.length());
     1376    return equal(stringImpl.characters16() + startOffset, matchString.characters16(), matchString.length());
    13771377}
    13781378
    … …  
    14081408}
    14091409
    1410 bool StringImpl::startsWith(StringImpl& matchString, unsigned startOffset, bool caseSensitive) const
    1411 {
    1412     return equalInner(const_cast<StringImpl&>(*this), startOffset, matchString, caseSensitive);
     1410bool StringImpl::hasInfixStartingAt(const StringImpl& matchString, unsigned startOffset) const
     1411{
     1412    return equalInner(*this, startOffset, matchString);
    14131413}
    14141414
    … …  
    14371437}
    14381438
    1439 bool StringImpl::endsWith(StringImpl& matchString, unsigned endOffset, bool caseSensitive) const
     1439bool StringImpl::hasInfixEndingAt(const StringImpl& matchString, unsigned endOffset) const
    14401440{
    14411441    if (endOffset < matchString.length())
    14421442        return false;
    1443     return equalInner(const_cast<StringImpl&>(*this), endOffset - matchString.length(), matchString, caseSensitive);
     1443    return equalInner(*this, endOffset - matchString.length(), matchString);
    14441444}
    14451445

  • trunk/Source/WTF/wtf/text/StringImpl.h

    r179644 r181105  
    675675    template<unsigned matchLength>
    676676    bool startsWith(const char (&prefix)[matchLength], bool caseSensitive = true) const { return startsWith(prefix, matchLength - 1, caseSensitive); }
    677     WTF_EXPORT_STRING_API bool startsWith(StringImpl&, unsigned startOffset, bool caseSensitive) const;
     677    WTF_EXPORT_STRING_API bool hasInfixStartingAt(const StringImpl&, unsigned startOffset) const;
    678678
    679679    WTF_EXPORT_STRING_API bool endsWith(StringImpl*, bool caseSensitive = true);
    … …  
    682682    template<unsigned matchLength>
    683683    bool endsWith(const char (&prefix)[matchLength], bool caseSensitive = true) const { return endsWith(prefix, matchLength - 1, caseSensitive); }
    684     WTF_EXPORT_STRING_API bool endsWith(StringImpl&, unsigned endOffset, bool caseSensitive) const;
     684    WTF_EXPORT_STRING_API bool hasInfixEndingAt(const StringImpl&, unsigned endOffset) const;
    685685
    686686    WTF_EXPORT_STRING_API Ref<StringImpl> replace(UChar, UChar);

  • trunk/Source/WTF/wtf/text/WTFString.h

    r181086 r181105  
    271271    bool startsWith(const char (&prefix)[matchLength], bool caseSensitive = true) const
    272272        { return m_impl ? m_impl->startsWith<matchLength>(prefix, caseSensitive) : !matchLength; }
    273     bool startsWith(String& prefix, unsigned startOffset, bool caseSensitive) const
    274         { return m_impl && prefix.impl() ? m_impl->startsWith(*prefix.impl(), startOffset, caseSensitive) : false; }
     273    bool hasInfixStartingAt(const String& prefix, unsigned startOffset) const
     274        { return m_impl && prefix.impl() ? m_impl->hasInfixStartingAt(*prefix.impl(), startOffset) : false; }
    275275
    276276    bool endsWith(const String& s, bool caseSensitive = true) const
    … …  
    282282    bool endsWith(const char (&prefix)[matchLength], bool caseSensitive = true) const
    283283        { return m_impl ? m_impl->endsWith<matchLength>(prefix, caseSensitive) : !matchLength; }
    284     bool endsWith(String& suffix, unsigned endOffset, bool caseSensitive) const
    285         { return m_impl && suffix.impl() ? m_impl->endsWith(*suffix.impl(), endOffset, caseSensitive) : false; }
     284    bool hasInfixEndingAt(const String& suffix, unsigned endOffset) const
     285        { return m_impl && suffix.impl() ? m_impl->hasInfixEndingAt(*suffix.impl(), endOffset) : false; }
    286286
    287287    WTF_EXPORT_STRING_API void append(const String&);

  • trunk/Tools/ChangeLog

    r181098 r181105  
     12015-03-05  Chris Dumez  <cdumez@apple.com>
     2
     3        Regression(r173761): ASSERTION FAILED: !is8Bit() in StringImpl::characters16()
     4        https://bugs.webkit.org/show_bug.cgi?id=142350
     5
     6        Reviewed by Michael Saboff and Benjamin Poulain.
     7
     8        Add API test for WTFString::hasInfixStartingAt() to make sure it doesn't
     9        crash if the string is 8-bit but the pattern is 16-bit (and vice-versa).
     10
     11        * TestWebKitAPI/Tests/WTF/WTFString.cpp:
     12        (TestWebKitAPI::TEST):
     13
    1142015-03-05  Brent Fulgham  <bfulgham@apple.com>
    215

  • trunk/Tools/TestWebKitAPI/Tests/WTF/WTFString.cpp

    r174246 r181105  
    261261}
    262262
     263TEST(WTF, StringhasInfixStartingAt)
     264{
     265    EXPECT_TRUE(String("Test").is8Bit());
     266    EXPECT_TRUE(String("Te").is8Bit());
     267    EXPECT_TRUE(String("st").is8Bit());
     268    EXPECT_TRUE(String("Test").hasInfixStartingAt(String("Te"), 0));
     269    EXPECT_FALSE(String("Test").hasInfixStartingAt(String("Te"), 2));
     270    EXPECT_TRUE(String("Test").hasInfixStartingAt(String("st"), 2));
     271    EXPECT_FALSE(String("Test").hasInfixStartingAt(String("ST"), 2));
     272
     273    EXPECT_FALSE(String::fromUTF8("中国").is8Bit());
     274    EXPECT_FALSE(String::fromUTF8("中").is8Bit());
     275    EXPECT_FALSE(String::fromUTF8("国").is8Bit());
     276    EXPECT_TRUE(String::fromUTF8("中国").hasInfixStartingAt(String::fromUTF8("中"), 0));
     277    EXPECT_FALSE(String::fromUTF8("中国").hasInfixStartingAt(String::fromUTF8("中"), 1));
     278    EXPECT_TRUE(String::fromUTF8("中国").hasInfixStartingAt(String::fromUTF8("国"), 1));
     279
     280    EXPECT_FALSE(String::fromUTF8("中国").hasInfixStartingAt(String("Te"), 0));
     281    EXPECT_FALSE(String("Test").hasInfixStartingAt(String::fromUTF8("中"), 2));
     282}
     283
    263284} // namespace TestWebKitAPI
Note: See TracChangeset for help on using the changeset viewer.