Changeset 208699 in webkit


Ignore:
Timestamp:
Nov 14, 2016, 11:42:41 AM (9 years ago)
Author:
mark.lam@apple.com
Message:

Some of JSStringView::SafeView methods are not idiomatically safe for JSString to StringView conversions.
​https://bugs.webkit.org/show_bug.cgi?id=164701
<rdar://problem/27462104>

Reviewed by Darin Adler.

JSTests:

  • stress/string-prototype-charCodeAt-on-too-long-rope.js: Added.

Source/JavaScriptCore:

The characters8(), characters16(), and operator[] in JSString::SafeView converts
the underlying JSString to a StringView via get(), and then uses the StringView
without first checking if an exception was thrown during the conversion. This is
unsafe because the conversion may have failed.

Instead, we should remove these 3 convenience methods, and make the caller
explicitly call get() and do the appropriate exception checks before using the
StringView.

  • runtime/JSGlobalObjectFunctions.cpp:

(JSC::toStringView):
(JSC::encode):
(JSC::decode):
(JSC::globalFuncParseInt):
(JSC::globalFuncEscape):
(JSC::globalFuncUnescape):
(JSC::toSafeView): Deleted.

  • runtime/JSONObject.cpp:

(JSC::JSONProtoFuncParse):

  • runtime/JSString.h:

(JSC::JSString::SafeView::length):
(JSC::JSString::SafeView::characters8): Deleted.
(JSC::JSString::SafeView::characters16): Deleted.
(JSC::JSString::SafeView::operator[]): Deleted.

  • runtime/StringPrototype.cpp:

(JSC::stringProtoFuncRepeatCharacter):
(JSC::stringProtoFuncCharAt):
(JSC::stringProtoFuncCharCodeAt):
(JSC::stringProtoFuncNormalize):

Location:
trunk
Files:
1 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r208698 r208699  
     12016-11-14  Mark Lam  <mark.lam@apple.com>
     2
     3        Some of JSStringView::SafeView methods are not idiomatically safe for JSString to StringView conversions.
     4        https://bugs.webkit.org/show_bug.cgi?id=164701
     5        <rdar://problem/27462104>
     6
     7        Reviewed by Darin Adler.
     8
     9        * stress/string-prototype-charCodeAt-on-too-long-rope.js: Added.
     10
    1112016-11-14  Mark Lam  <mark.lam@apple.com>
    212

  • trunk/Source/JavaScriptCore/ChangeLog

    r208698 r208699  
     12016-11-14  Mark Lam  <mark.lam@apple.com>
     2
     3        Some of JSStringView::SafeView methods are not idiomatically safe for JSString to StringView conversions.
     4        https://bugs.webkit.org/show_bug.cgi?id=164701
     5        <rdar://problem/27462104>
     6
     7        Reviewed by Darin Adler.
     8
     9        The characters8(), characters16(), and operator[] in JSString::SafeView converts
     10        the underlying JSString to a StringView via get(), and then uses the StringView
     11        without first checking if an exception was thrown during the conversion.  This is
     12        unsafe because the conversion may have failed.
     13       
     14        Instead, we should remove these 3 convenience methods, and make the caller
     15        explicitly call get() and do the appropriate exception checks before using the
     16        StringView.
     17
     18        * runtime/JSGlobalObjectFunctions.cpp:
     19        (JSC::toStringView):
     20        (JSC::encode):
     21        (JSC::decode):
     22        (JSC::globalFuncParseInt):
     23        (JSC::globalFuncEscape):
     24        (JSC::globalFuncUnescape):
     25        (JSC::toSafeView): Deleted.
     26        * runtime/JSONObject.cpp:
     27        (JSC::JSONProtoFuncParse):
     28        * runtime/JSString.h:
     29        (JSC::JSString::SafeView::length):
     30        (JSC::JSString::SafeView::characters8): Deleted.
     31        (JSC::JSString::SafeView::characters16): Deleted.
     32        (JSC::JSString::SafeView::operator[]): Deleted.
     33        * runtime/StringPrototype.cpp:
     34        (JSC::stringProtoFuncRepeatCharacter):
     35        (JSC::stringProtoFuncCharAt):
     36        (JSC::stringProtoFuncCharCodeAt):
     37        (JSC::stringProtoFuncNormalize):
     38
    1392016-11-14  Mark Lam  <mark.lam@apple.com>
    240

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

    r208074 r208699  
    5858
    5959template<typename CallbackWhenNoException>
    60 static ALWAYS_INLINE typename std::result_of<CallbackWhenNoException(JSString::SafeView&)>::type toSafeView(ExecState* exec, JSValue value, CallbackWhenNoException callback)
    61 {
     60static ALWAYS_INLINE typename std::result_of<CallbackWhenNoException(StringView)>::type toStringView(ExecState* exec, JSValue value, CallbackWhenNoException callback)
     61{
     62    VM& vm = exec->vm();
     63    auto scope = DECLARE_THROW_SCOPE(vm);
    6264    JSString* string = value.toStringOrNull(exec);
    6365    if (UNLIKELY(!string))
    6466        return { };
    6567    JSString::SafeView view = string->view(exec);
    66     return callback(view);
     68    StringView stringView = view.get();
     69    RETURN_IF_EXCEPTION(scope, { });
     70    return callback(stringView);
    6771}
    6872
    … …  
    159163static JSValue encode(ExecState* exec, const Bitmap<256>& doNotEscape)
    160164{
    161     return toSafeView(exec, exec->argument(0), [&] (JSString::SafeView& view) {
     165    return toStringView(exec, exec->argument(0), [&] (StringView view) {
    162166        if (view.is8Bit())
    163167            return encode(exec, doNotEscape, view.characters8(), view.length());
    … …  
    237241static JSValue decode(ExecState* exec, const Bitmap<256>& doNotUnescape, bool strict)
    238242{
    239     return toSafeView(exec, exec->argument(0), [&] (JSString::SafeView& view) {
     243    return toStringView(exec, exec->argument(0), [&] (StringView view) {
    240244        if (view.is8Bit())
    241245            return decode(exec, view.characters8(), view.length(), doNotUnescape, strict);
    … …  
    708712
    709713    // If ToString throws, we shouldn't call ToInt32.
    710     return toSafeView(exec, value, [&] (JSString::SafeView& view) {
    711         return JSValue::encode(jsNumber(parseInt(view.get(), radixValue.toInt32(exec))));
     714    return toStringView(exec, value, [&] (StringView view) {
     715        return JSValue::encode(jsNumber(parseInt(view, radixValue.toInt32(exec))));
    712716    });
    713717}
    … …  
    766770    );
    767771
    768     return JSValue::encode(toSafeView(exec, exec->argument(0), [&] (JSString::SafeView& view) {
     772    return JSValue::encode(toStringView(exec, exec->argument(0), [&] (StringView view) {
    769773        JSStringBuilder builder;
    770774        if (view.is8Bit()) {
    … …  
    805809EncodedJSValue JSC_HOST_CALL globalFuncUnescape(ExecState* exec)
    806810{
    807     return JSValue::encode(toSafeView(exec, exec->argument(0), [&] (JSString::SafeView& view) {
     811    return JSValue::encode(toStringView(exec, exec->argument(0), [&] (StringView view) {
    808812        StringBuilder builder;
    809813        int k = 0;

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

    r208123 r208699  
    764764    JSString::SafeView source = exec->uncheckedArgument(0).toString(exec)->view(exec);
    765765    RETURN_IF_EXCEPTION(scope, encodedJSValue());
     766    StringView view = source.get();
     767    RETURN_IF_EXCEPTION(scope, encodedJSValue());
    766768
    767769    JSValue unfiltered;
    768770    LocalScope localScope(vm);
    769     if (source.is8Bit()) {
    770         LiteralParser<LChar> jsonParser(exec, source.characters8(), source.length(), StrictJSON);
     771    if (view.is8Bit()) {
     772        LiteralParser<LChar> jsonParser(exec, view.characters8(), view.length(), StrictJSON);
    771773        unfiltered = jsonParser.tryLiteralParse();
    772774        if (!unfiltered)
    773775            return throwVMError(exec, scope, createSyntaxError(exec, jsonParser.getErrorMessage()));
    774776    } else {
    775         LiteralParser<UChar> jsonParser(exec, source.characters16(), source.length(), StrictJSON);
     777        LiteralParser<UChar> jsonParser(exec, view.characters16(), view.length(), StrictJSON);
    776778        unfiltered = jsonParser.tryLiteralParse();
    777779        if (!unfiltered)

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

    r207849 r208699  
    483483    bool is8Bit() const { return m_string->is8Bit(); }
    484484    unsigned length() const { return m_string->length(); }
    485     const LChar* characters8() const { return get().characters8(); }
    486     const UChar* characters16() const { return get().characters16(); }
    487     UChar operator[](unsigned index) const { return get()[index]; }
    488485
    489486private:

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

    r208563 r208699  
    792792EncodedJSValue JSC_HOST_CALL stringProtoFuncRepeatCharacter(ExecState* exec)
    793793{
     794    VM& vm = exec->vm();
     795    auto scope = DECLARE_THROW_SCOPE(vm);
     796
    794797    // For a string which length is single, instead of creating ropes,
    795798    // allocating a sequential buffer and fill with the repeated string for efficiency.
    … …  
    803806    RELEASE_ASSERT(repeatCountValue.isNumber());
    804807    int32_t repeatCount;
    805     {
    806         VM& vm = exec->vm();
    807         auto scope = DECLARE_THROW_SCOPE(vm);
    808         double value = repeatCountValue.asNumber();
    809         if (value > JSString::MaxLength)
    810             return JSValue::encode(throwOutOfMemoryError(exec, scope));
    811         repeatCount = static_cast<int32_t>(value);
    812     }
     808    double value = repeatCountValue.asNumber();
     809    if (value > JSString::MaxLength)
     810        return JSValue::encode(throwOutOfMemoryError(exec, scope));
     811    repeatCount = static_cast<int32_t>(value);
    813812    ASSERT(repeatCount >= 0);
    814813    ASSERT(!repeatCountValue.isDouble() || repeatCountValue.asDouble() == repeatCount);
    815814
    816     UChar character = string->view(exec)[0];
     815    JSString::SafeView safeView = string->view(exec);
     816    StringView view = safeView.get();
     817    ASSERT(view.length() == 1 && !scope.exception());
     818    UChar character = view[0];
     819    scope.release();
    817820    if (!(character & ~0xff))
    818821        return JSValue::encode(repeatCharacter(*exec, static_cast<LChar>(character), repeatCount));
    … …  
    905908        return throwVMTypeError(exec, scope);
    906909    JSString::SafeView string = thisValue.toString(exec)->view(exec);
     910    RETURN_IF_EXCEPTION(scope, encodedJSValue());
     911    StringView view = string.get();
     912    RETURN_IF_EXCEPTION(scope, encodedJSValue());
    907913    JSValue a0 = exec->argument(0);
    908914    if (a0.isUInt32()) {
    909915        uint32_t i = a0.asUInt32();
    910         if (i < string.length())
    911             return JSValue::encode(jsSingleCharacterString(exec, string[i]));
     916        if (i < view.length())
     917            return JSValue::encode(jsSingleCharacterString(exec, view[i]));
    912918        return JSValue::encode(jsEmptyString(exec));
    913919    }
    914920    double dpos = a0.toInteger(exec);
    915     if (dpos >= 0 && dpos < string.length())
    916         return JSValue::encode(jsSingleCharacterString(exec, string[static_cast<unsigned>(dpos)]));
     921    if (dpos >= 0 && dpos < view.length())
     922        return JSValue::encode(jsSingleCharacterString(exec, view[static_cast<unsigned>(dpos)]));
    917923    return JSValue::encode(jsEmptyString(exec));
    918924}
    … …  
    926932    if (!checkObjectCoercible(thisValue))
    927933        return throwVMTypeError(exec, scope);
    928     JSString::SafeView string = thisValue.toString(exec)->view(exec);
     934    JSString* jsString = thisValue.toString(exec);
     935    RETURN_IF_EXCEPTION(scope, encodedJSValue());
     936    JSString::SafeView string = jsString->view(exec);
     937    StringView view = string.get();
     938    RETURN_IF_EXCEPTION(scope, encodedJSValue());
    929939    JSValue a0 = exec->argument(0);
    930940    if (a0.isUInt32()) {
    931941        uint32_t i = a0.asUInt32();
    932         if (i < string.length())
    933             return JSValue::encode(jsNumber(string[i]));
     942        if (i < view.length())
     943            return JSValue::encode(jsNumber(view[i]));
    934944        return JSValue::encode(jsNaN());
    935945    }
    936946    double dpos = a0.toInteger(exec);
    937     if (dpos >= 0 && dpos < string.length())
    938         return JSValue::encode(jsNumber(string[static_cast<int>(dpos)]));
     947    if (dpos >= 0 && dpos < view.length())
     948        return JSValue::encode(jsNumber(view[static_cast<int>(dpos)]));
    939949    return JSValue::encode(jsNaN());
    940950}
    … …  
    20092019    JSString::SafeView source = thisValue.toString(exec)->view(exec);
    20102020    RETURN_IF_EXCEPTION(scope, encodedJSValue());
     2021    StringView view = source.get();
     2022    RETURN_IF_EXCEPTION(scope, encodedJSValue());
    20112023
    20122024    UNormalizationMode form = UNORM_NFC;
    … …  
    20282040    }
    20292041
    2030     return JSValue::encode(normalize(exec, source.get().upconvertedCharacters(), source.length(), form));
     2042    return JSValue::encode(normalize(exec, view.upconvertedCharacters(), view.length(), form));
    20312043}
    20322044
Note: See TracChangeset for help on using the changeset viewer.