Regression(r173761): ASSERTION FAILED: !is8Bit() in StringImpl::chara…

…cters16()

https://bugs.webkit.org/show_bug.cgi?id=142350

Patch by Chris Dumez <cdumez@apple.com> 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:


Canonical link: https://commits.webkit.org/160395@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@181105 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog

@@ -1,3 +1,16 @@
+2015-03-05  Chris Dumez  <cdumez@apple.com>
+
+        Regression(r173761): ASSERTION FAILED: !is8Bit() in StringImpl::characters16()
+        https://bugs.webkit.org/show_bug.cgi?id=142350
+
+        Reviewed by Michael Saboff and Benjamin Poulain.
+
+        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:
+
 2015-03-05  Roger Fong  <roger_fong@apple.com>
 
         Update inline media control icons for OSX.

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

@@ -54,6 +54,10 @@ shouldBe("'1e+100 bar'.startsWith(1e+100)", "true");
 shouldBe("'1e100 bar'.startsWith(1e100)", "false");
 shouldBe("'フーバー'.startsWith('フー')", "true");
 shouldBe("'フーバー'.startsWith('バー')", "false");
+shouldBe("'フーバー'.startsWith('abc')", "false");
+shouldBe("'フーバー'.startsWith('abc', 1)", "false");
+shouldBe("'foo bar'.startsWith('フー')", "false");
+shouldBe("'foo bar'.startsWith('フー', 1)", "false");
 
 // Test endsWith
 shouldBe("'foo bar'.endsWith('bar')", "true");
@@ -82,6 +86,10 @@ shouldBe("'foo 1e+100'.endsWith(1e+100)", "true");
 shouldBe("'foo 1e100'.endsWith(1e100)", "false");
 shouldBe("'フーバー'.endsWith('バー')", "true");
 shouldBe("'フーバー'.endsWith('フー')", "false");
+shouldBe("'フーバー'.endsWith('abc')", "false");
+shouldBe("'フーバー'.endsWith('abc')", "false");
+shouldBe("'foo bar'.endsWith('フー')", "false");
+shouldBe("'foo bar'.endsWith('フー', 3)", "false");
 
 // Call functions with an environment record as 'this'.
 shouldThrow("(function() { var f = String.prototype.startsWith; (function() { f('a'); })(); })()");

LayoutTests/js/string-includes-expected.txt

@@ -54,6 +54,10 @@ PASS '1e+100 bar'.startsWith(1e+100) is true
 PASS '1e100 bar'.startsWith(1e100) is false
 PASS 'フーバー'.startsWith('フー') is true
 PASS 'フーバー'.startsWith('バー') is false
+PASS 'フーバー'.startsWith('abc') is false
+PASS 'フーバー'.startsWith('abc', 1) is false
+PASS 'foo bar'.startsWith('フー') is false
+PASS 'foo bar'.startsWith('フー', 1) is false
 PASS 'foo bar'.endsWith('bar') is true
 PASS 'foo bar'.endsWith('ba', 6) is true
 PASS 'foo bar'.endsWith(' ba', 6) is true
@@ -80,6 +84,10 @@ PASS 'foo 1e+100'.endsWith(1e+100) is true
 PASS 'foo 1e100'.endsWith(1e100) is false
 PASS 'フーバー'.endsWith('バー') is true
 PASS 'フーバー'.endsWith('フー') is false
+PASS 'フーバー'.endsWith('abc') is false
+PASS 'フーバー'.endsWith('abc') is false
+PASS 'foo bar'.endsWith('フー') is false
+PASS 'foo bar'.endsWith('フー', 3) is false
 PASS (function() { var f = String.prototype.startsWith; (function() { f('a'); })(); })() threw exception TypeError: Type error.
 PASS (function() { var f = String.prototype.endsWith; (function() { f('a'); })(); })() threw exception TypeError: Type error.
 PASS (function() { var f = String.prototype.includes; (function() { f('a'); })(); })() threw exception TypeError: Type error.

Source/JavaScriptCore/ChangeLog

@@ -1,3 +1,17 @@
+2015-03-05  Chris Dumez  <cdumez@apple.com>
+
+        Regression(r173761): ASSERTION FAILED: !is8Bit() in StringImpl::characters16()
+        https://bugs.webkit.org/show_bug.cgi?id=142350
+
+        Reviewed by Michael Saboff and Benjamin Poulain.
+
+        Call WTFString::hasInfixStartingAt() / hasInfixEndingAt() now that these
+        methods have been renamed for clarity.
+
+        * runtime/StringPrototype.cpp:
+        (JSC::stringProtoFuncStartsWith):
+        (JSC::stringProtoFuncEndsWith):
+
 2015-03-05  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         Implement ES6 StringIterator

Source/JavaScriptCore/runtime/StringPrototype.cpp

@@ -1652,7 +1652,7 @@ EncodedJSValue JSC_HOST_CALL stringProtoFuncStartsWith(ExecState* exec)
     if (exec->hadException())
         return JSValue::encode(jsUndefined());
 
-    return JSValue::encode(jsBoolean(stringToSearchIn.startsWith(searchString, start, true)));
+    return JSValue::encode(jsBoolean(stringToSearchIn.hasInfixStartingAt(searchString, start)));
 }
 
 EncodedJSValue JSC_HOST_CALL stringProtoFuncEndsWith(ExecState* exec)
@@ -1680,7 +1680,7 @@ EncodedJSValue JSC_HOST_CALL stringProtoFuncEndsWith(ExecState* exec)
         return JSValue::encode(jsUndefined());
     unsigned end = std::min<unsigned>(std::max(pos, 0), length);
 
-    return JSValue::encode(jsBoolean(stringToSearchIn.endsWith(searchString, end, true)));
+    return JSValue::encode(jsBoolean(stringToSearchIn.hasInfixEndingAt(searchString, end)));
 }
 
 EncodedJSValue JSC_HOST_CALL stringProtoFuncIncludes(ExecState* exec)

Source/WTF/ChangeLog

@@ -1,3 +1,36 @@
+2015-03-05  Chris Dumez  <cdumez@apple.com>
+
+        Regression(r173761): ASSERTION FAILED: !is8Bit() in StringImpl::characters16()
+        https://bugs.webkit.org/show_bug.cgi?id=142350
+
+        Reviewed by Michael Saboff and Benjamin Poulain.
+
+        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.
+
 2015-03-05  Chris Dumez  <cdumez@apple.com>
 
         NetworkCache efficacy logging is using too much CPU

Source/WTF/wtf/text/StringImpl.cpp

@@ -1357,7 +1357,7 @@ ALWAYS_INLINE static bool equalInner(const StringImpl* stringImpl, unsigned star
     return equalIgnoringCase(stringImpl->characters16() + startOffset, reinterpret_cast<const LChar*>(matchString), matchLength);
 }
 
-ALWAYS_INLINE static bool equalInner(StringImpl& stringImpl, unsigned startOffset, StringImpl& matchString, bool caseSensitive)
+ALWAYS_INLINE static bool equalInner(const StringImpl& stringImpl, unsigned startOffset, const StringImpl& matchString)
 {
     if (startOffset > stringImpl.length())
         return false;
@@ -1366,14 +1366,14 @@ ALWAYS_INLINE static bool equalInner(StringImpl& stringImpl, unsigned startOffse
     if (matchString.length() + startOffset > stringImpl.length())
         return false;
 
-    if (caseSensitive) {
-        if (stringImpl.is8Bit())
+    if (stringImpl.is8Bit()) {
+        if (matchString.is8Bit())
             return equal(stringImpl.characters8() + startOffset, matchString.characters8(), matchString.length());
-        return equal(stringImpl.characters16() + startOffset, matchString.characters16(), matchString.length());
+        return equal(stringImpl.characters8() + startOffset, matchString.characters16(), matchString.length());
     }
-    if (stringImpl.is8Bit())
-        return equalIgnoringCase(stringImpl.characters8() + startOffset, matchString.characters8(), matchString.length());
-    return equalIgnoringCase(stringImpl.characters16() + startOffset, matchString.characters16(), matchString.length());
+    if (matchString.is8Bit())
+        return equal(stringImpl.characters16() + startOffset, matchString.characters8(), matchString.length());
+    return equal(stringImpl.characters16() + startOffset, matchString.characters16(), matchString.length());
 }
 
 bool StringImpl::startsWith(const StringImpl* str) const
@@ -1407,9 +1407,9 @@ bool StringImpl::startsWith(const char* matchString, unsigned matchLength, bool
     return equalInner(this, 0, matchString, matchLength, caseSensitive);
 }
 
-bool StringImpl::startsWith(StringImpl& matchString, unsigned startOffset, bool caseSensitive) const
+bool StringImpl::hasInfixStartingAt(const StringImpl& matchString, unsigned startOffset) const
 {
-    return equalInner(const_cast<StringImpl&>(*this), startOffset, matchString, caseSensitive);
+    return equalInner(*this, startOffset, matchString);
 }
 
 bool StringImpl::endsWith(StringImpl* matchString, bool caseSensitive)
@@ -1436,11 +1436,11 @@ bool StringImpl::endsWith(const char* matchString, unsigned matchLength, bool ca
     return equalInner(this, startOffset, matchString, matchLength, caseSensitive);
 }
 
-bool StringImpl::endsWith(StringImpl& matchString, unsigned endOffset, bool caseSensitive) const
+bool StringImpl::hasInfixEndingAt(const StringImpl& matchString, unsigned endOffset) const
 {
     if (endOffset < matchString.length())
         return false;
-    return equalInner(const_cast<StringImpl&>(*this), endOffset - matchString.length(), matchString, caseSensitive);
+    return equalInner(*this, endOffset - matchString.length(), matchString);
 }
 
 Ref<StringImpl> StringImpl::replace(UChar oldC, UChar newC)

Source/WTF/wtf/text/StringImpl.h

@@ -674,14 +674,14 @@ class StringImpl {
     WTF_EXPORT_STRING_API bool startsWith(const char*, unsigned matchLength, bool caseSensitive) const;
     template<unsigned matchLength>
     bool startsWith(const char (&prefix)[matchLength], bool caseSensitive = true) const { return startsWith(prefix, matchLength - 1, caseSensitive); }
-    WTF_EXPORT_STRING_API bool startsWith(StringImpl&, unsigned startOffset, bool caseSensitive) const;
+    WTF_EXPORT_STRING_API bool hasInfixStartingAt(const StringImpl&, unsigned startOffset) const;
 
     WTF_EXPORT_STRING_API bool endsWith(StringImpl*, bool caseSensitive = true);
     WTF_EXPORT_STRING_API bool endsWith(UChar) const;
     WTF_EXPORT_STRING_API bool endsWith(const char*, unsigned matchLength, bool caseSensitive) const;
     template<unsigned matchLength>
     bool endsWith(const char (&prefix)[matchLength], bool caseSensitive = true) const { return endsWith(prefix, matchLength - 1, caseSensitive); }
-    WTF_EXPORT_STRING_API bool endsWith(StringImpl&, unsigned endOffset, bool caseSensitive) const;
+    WTF_EXPORT_STRING_API bool hasInfixEndingAt(const StringImpl&, unsigned endOffset) const;
 
     WTF_EXPORT_STRING_API Ref<StringImpl> replace(UChar, UChar);
     WTF_EXPORT_STRING_API Ref<StringImpl> replace(UChar, StringImpl*);

Source/WTF/wtf/text/WTFString.h

@@ -270,8 +270,8 @@ class String {
     template<unsigned matchLength>
     bool startsWith(const char (&prefix)[matchLength], bool caseSensitive = true) const
         { return m_impl ? m_impl->startsWith<matchLength>(prefix, caseSensitive) : !matchLength; }
-    bool startsWith(String& prefix, unsigned startOffset, bool caseSensitive) const
-        { return m_impl && prefix.impl() ? m_impl->startsWith(*prefix.impl(), startOffset, caseSensitive) : false; }
+    bool hasInfixStartingAt(const String& prefix, unsigned startOffset) const
+        { return m_impl && prefix.impl() ? m_impl->hasInfixStartingAt(*prefix.impl(), startOffset) : false; }
 
     bool endsWith(const String& s, bool caseSensitive = true) const
         { return m_impl ? m_impl->endsWith(s.impl(), caseSensitive) : s.isEmpty(); }
@@ -281,8 +281,8 @@ class String {
     template<unsigned matchLength>
     bool endsWith(const char (&prefix)[matchLength], bool caseSensitive = true) const
         { return m_impl ? m_impl->endsWith<matchLength>(prefix, caseSensitive) : !matchLength; }
-    bool endsWith(String& suffix, unsigned endOffset, bool caseSensitive) const
-        { return m_impl && suffix.impl() ? m_impl->endsWith(*suffix.impl(), endOffset, caseSensitive) : false; }
+    bool hasInfixEndingAt(const String& suffix, unsigned endOffset) const
+        { return m_impl && suffix.impl() ? m_impl->hasInfixEndingAt(*suffix.impl(), endOffset) : false; }
 
     WTF_EXPORT_STRING_API void append(const String&);
     WTF_EXPORT_STRING_API void append(LChar);

Tools/ChangeLog

@@ -1,3 +1,16 @@
+2015-03-05  Chris Dumez  <cdumez@apple.com>
+
+        Regression(r173761): ASSERTION FAILED: !is8Bit() in StringImpl::characters16()
+        https://bugs.webkit.org/show_bug.cgi?id=142350
+
+        Reviewed by Michael Saboff and Benjamin Poulain.
+
+        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):
+
 2015-03-05  Brent Fulgham  <bfulgham@apple.com>
 
         [Win] Ensure build target directory exists when launching MSBuild

Tools/TestWebKitAPI/Tests/WTF/WTFString.cpp

@@ -260,4 +260,25 @@ TEST(WTF, StringToDouble)
     EXPECT_FALSE(ok);
 }
 
+TEST(WTF, StringhasInfixStartingAt)
+{
+    EXPECT_TRUE(String("Test").is8Bit());
+    EXPECT_TRUE(String("Te").is8Bit());
+    EXPECT_TRUE(String("st").is8Bit());
+    EXPECT_TRUE(String("Test").hasInfixStartingAt(String("Te"), 0));
+    EXPECT_FALSE(String("Test").hasInfixStartingAt(String("Te"), 2));
+    EXPECT_TRUE(String("Test").hasInfixStartingAt(String("st"), 2));
+    EXPECT_FALSE(String("Test").hasInfixStartingAt(String("ST"), 2));
+
+    EXPECT_FALSE(String::fromUTF8("中国").is8Bit());
+    EXPECT_FALSE(String::fromUTF8("中").is8Bit());
+    EXPECT_FALSE(String::fromUTF8("国").is8Bit());
+    EXPECT_TRUE(String::fromUTF8("中国").hasInfixStartingAt(String::fromUTF8("中"), 0));
+    EXPECT_FALSE(String::fromUTF8("中国").hasInfixStartingAt(String::fromUTF8("中"), 1));
+    EXPECT_TRUE(String::fromUTF8("中国").hasInfixStartingAt(String::fromUTF8("国"), 1));
+
+    EXPECT_FALSE(String::fromUTF8("中国").hasInfixStartingAt(String("Te"), 0));
+    EXPECT_FALSE(String("Test").hasInfixStartingAt(String::fromUTF8("中"), 2));
+}
+
 } // namespace TestWebKitAPI