Bug 223788 - ASSERTION FAILED: !m_needExceptionCheck in RTCPeerConnection::CertificateParameters WebCore::convertDictionary<WebCore::RTCPeerConnection::CertificateParameters>
Summary: ASSERTION FAILED: !m_needExceptionCheck in RTCPeerConnection::CertificatePara...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Frédéric Wang (:fredw)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-26 02:31 PDT by Ryosuke Niwa
Modified: 2021-04-13 03:26 PDT (History)
16 users (show)

See Also:


Attachments
Test (136 bytes, text/html)
2021-03-26 02:32 PDT, Ryosuke Niwa
no flags Details
Patch (2.42 KB, patch)
2021-04-12 06:54 PDT, Frédéric Wang (:fredw)
rniwa: review+
Details | Formatted Diff | Diff
Patch for landing (4.78 KB, patch)
2021-04-12 23:58 PDT, Frédéric Wang (:fredw)
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (5.27 KB, patch)
2021-04-13 02:41 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2021-03-26 02:31:43 PDT
e.g.

ERROR: Unchecked JS exception:
    This scope can throw a JS exception: toNumberSlowCase @ ./runtime/JSCJSValue.cpp:60
        (ExceptionScope::m_recursionDepth was 8)
    But the exception was unchecked as of this scope: convert @ /Volumes/Data/safari-4/OpenSource/Source/WebCore/bindings/js/JSDOMConvertNumbers.h:347
        (ExceptionScope::m_recursionDepth was 7)

Unchecked exception detected at:
    1   0x47a9eaa1e JSC::VM::verifyExceptionCheckNeedIsSatisfied(unsigned int, JSC::ExceptionEventLocation&)
    2   0x47a9c4e38 JSC::ThrowScope::throwException(JSC::JSGlobalObject*, JSC::JSValue)
    3   0x47a713cad JSC::throwException(JSC::JSGlobalObject*, JSC::ThrowScope&, JSC::JSValue)
    4   0x47a676674 JSC::throwTypeError(JSC::JSGlobalObject*, JSC::ThrowScope&, WTF::String const&)
    5   0x47a676800 JSC::throwTypeError(JSC::JSGlobalObject*, JSC::ThrowScope&, WTF::ASCIILiteral)
    6   0x45afaf05c WebCore::throwNonFiniteTypeError(JSC::JSGlobalObject&, JSC::ThrowScope&)
    7   0x458ccea18 WebCore::Converter<WebCore::IDLDouble>::convert(JSC::JSGlobalObject&, JSC::JSValue)
    8   0x458c44085 WebCore::Converter<WebCore::IDLDouble>::ReturnType WebCore::convert<WebCore::IDLDouble>(JSC::JSGlobalObject&, JSC::JSValue)
    9   0x459877dfa WebCore::RTCPeerConnection::CertificateParameters WebCore::convertDictionary<WebCore::RTCPeerConnection::CertificateParameters>(JSC::JSGlobalObject&, JSC::JSValue)
    10  0x45a8e798e WebCore::certificateTypeFromAlgorithmIdentifier(JSC::JSGlobalObject&, WTF::Variant<JSC::Strong<JSC::JSObject, (JSC::ShouldStrongDestructorGrabLock)0>, WTF::String>&&)
    11  0x45a8e7746 WebCore::RTCPeerConnection::generateCertificate(JSC::JSGlobalObject&, WTF::Variant<JSC::Strong<JSC::JSObject, (JSC::ShouldStrongDestructorGrabLock)0>, WTF::String>&&, WebCore::DOMPromiseDeferred<WebCore::IDLInterface<WebCore::RTCCertificate> >&&)
    12  0x4598a08e2 WebCore::jsRTCPeerConnectionConstructorFunction_generateCertificateBody(JSC::JSGlobalObject*, JSC::CallFrame*, WTF::Ref<WebCore::DeferredPromise, WTF::RawPtrTraits<WebCore::DeferredPromise> >&&)
    13  0x4598a0cd4 long long WebCore::IDLOperationReturningPromise<WebCore::JSRTCPeerConnection>::callStatic<&(WebCore::jsRTCPeerConnectionConstructorFunction_generateCertificateBody(JSC::JSGlobalObject*, JSC::CallFrame*, WTF::Ref<WebCore::DeferredPromise, WTF::RawPtrTraits<WebCore::DeferredPromise> >&&)), (WebCore::CastedThisErrorBehavior)2>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*)::'lambda'(JSC::JSGlobalObject&, JSC::CallFrame&, WTF::Ref<WebCore::DeferredPromise, WTF::RawPtrTraits<WebCore::DeferredPromise> >&&)::operator()(JSC::JSGlobalObject&, JSC::CallFrame&, WTF::Ref<WebCore::DeferredPromise, WTF::RawPtrTraits<WebCore::DeferredPromise> >&&) const
    14  0x4598a0afb JSC::JSValue WebCore::callPromiseFunction<long long WebCore::IDLOperationReturningPromise<WebCore::JSRTCPeerConnection>::callStatic<&(WebCore::jsRTCPeerConnectionConstructorFunction_generateCertificateBody(JSC::JSGlobalObject*, JSC::CallFrame*, WTF::Ref<WebCore::DeferredPromise, WTF::RawPtrTraits<WebCore::DeferredPromise> >&&)), (WebCore::CastedThisErrorBehavior)2>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*)::'lambda'(JSC::JSGlobalObject&, JSC::CallFrame&, WTF::Ref<WebCore::DeferredPromise, WTF::RawPtrTraits<WebCore::DeferredPromise> >&&)>(JSC::JSGlobalObject&, JSC::CallFrame&, &(WebCore::jsRTCPeerConnectionConstructorFunction_generateCertificateBody(JSC::JSGlobalObject*, JSC::CallFrame*, WTF::Ref<WebCore::DeferredPromise, WTF::RawPtrTraits<WebCore::DeferredPromise> >&&)))
    15  0x4598a06b1 long long WebCore::IDLOperationReturningPromise<WebCore::JSRTCPeerConnection>::callStatic<&(WebCore::jsRTCPeerConnectionConstructorFunction_generateCertificateBody(JSC::JSGlobalObject*, JSC::CallFrame*, WTF::Ref<WebCore::DeferredPromise, WTF::RawPtrTraits<WebCore::DeferredPromise> >&&)), (WebCore::CastedThisErrorBehavior)2>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*)
    16  0x4598a0684 WebCore::jsRTCPeerConnectionConstructorFunction_generateCertificate(JSC::JSGlobalObject*, JSC::CallFrame*)
    17  0x30122f8011d8
    18  0x4793d21ef llint_entry
    19  0x4793b0250 vmEntryToJavaScript
    20  0x47a26bb2b JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
    21  0x47a26b088 JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::JSGlobalObject*, JSC::JSObject*)
    22  0x47a648847 JSC::evaluate(JSC::JSGlobalObject*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
    23  0x47a64899a JSC::profiledEvaluate(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
    24  0x45b04d84c WebCore::JSExecState::profiledEvaluate(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
    25  0x45b04d42e WebCore::ScriptController::evaluateInWorld(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld&)
    26  0x45b04d259 WebCore::ScriptController::evaluateInWorldIgnoringException(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld&)
    27  0x45b04db55 WebCore::ScriptController::evaluateIgnoringException(WebCore::ScriptSourceCode const&)
    28  0x45b79e7e6 WebCore::ScriptElement::executeClassicScript(WebCore::ScriptSourceCode const&)
    29  0x45b79c7fb WebCore::ScriptElement::prepareScript(WTF::TextPosition const&, WebCore::ScriptElement::LegacyTypeSupport)
    30  0x45bd2edb6 WebCore::HTMLScriptRunner::runScript(WebCore::ScriptElement&, WTF::TextPosition const&)
    31  0x45bd2ebb7 WebCore::HTMLScriptRunner::execute(WTF::Ref<WebCore::ScriptElement, WTF::RawPtrTraits<WebCore::ScriptElement> >&&, WTF::TextPosition const&)
    32  0x45bd0d401 WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder()
    33  0x45bd0d885 WebCore::HTMLDocumentParser::pumpTokenizerLoop(WebCore::HTMLDocumentParser::SynchronousMode, bool, WebCore::PumpSession&)
    34  0x45bd0cbff WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode)
    35  0x45bd0c396 WebCore::HTMLDocumentParser::pumpTokenizerIfPossible(WebCore::HTMLDocumentParser::SynchronousMode)
    36  0x45bd0e634 WebCore::HTMLDocumentParser::append(WTF::RefPtr<WTF::StringImpl, WTF::RawPtrTraits<WTF::StringImpl>, WTF::DefaultRefDerefTraits<WTF::StringImpl> >&&)
    37  0x45b5a5d56 WebCore::DecodedDataDocumentParser::flush(WebCore::DocumentWriter&)
    38  0x45c19b5fe WebCore::DocumentWriter::end()
    39  0x45c14da24 WebCore::DocumentLoader::finishedLoading()
    40  0x45c14d3c1 WebCore::DocumentLoader::notifyFinished(WebCore::CachedResource&, WebCore::NetworkLoadMetrics const&)
    41  0x45c2d008a WebCore::CachedResource::checkNotify(WebCore::NetworkLoadMetrics const&)
    42  0x45c2cbb7c WebCore::CachedResource::finishLoading(WebCore::SharedBuffer*, WebCore::NetworkLoadMetrics const&)
    43  0x45c2cd0fc WebCore::CachedRawResource::finishLoading(WebCore::SharedBuffer*, WebCore::NetworkLoadMetrics const&)
    44  0x45c2534a4 WebCore::SubresourceLoader::didFinishLoading(WebCore::NetworkLoadMetrics const&)
    45  0x449ceed5a WebKit::WebResourceLoader::didFinishResourceLoad(WebCore::NetworkLoadMetrics const&)
    46  0x44a2d3cc0 void IPC::callMemberFunctionImpl<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&), std::__1::tuple<WebCore::NetworkLoadMetrics>, 0ul>(WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&), std::__1::tuple<WebCore::NetworkLoadMetrics>&&, std::__1::integer_sequence<unsigned long, 0ul>)
    47  0x44a2d3c10 void IPC::callMemberFunction<WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&), std::__1::tuple<WebCore::NetworkLoadMetrics>, std::__1::integer_sequence<unsigned long, 0ul> >(std::__1::tuple<WebCore::NetworkLoadMetrics>&&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&))
    48  0x44a2d19be void IPC::handleMessage<Messages::WebResourceLoader::DidFinishResourceLoad, WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&)>(IPC::Decoder&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(WebCore::NetworkLoadMetrics const&))
    49  0x44a2d132e WebKit::WebResourceLoader::didReceiveWebResourceLoaderMessage(IPC::Connection&, IPC::Decoder&)
    50  0x449caff50 WebKit::NetworkProcessConnection::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
    51  0x448085904 IPC::Connection::dispatchMessage(IPC::Decoder&)
    52  0x4480860cc IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)
    53  0x4480866f0 IPC::Connection::dispatchOneIncomingMessage()

ASSERTION FAILED: !m_needExceptionCheck
./runtime/VM.cpp(1418) : void JSC::VM::verifyExceptionCheckNeedIsSatisfied(unsigned int, JSC::ExceptionEventLocation &)
1   0x478de4cc9 WTFCrash
2   0x47a57d2db WTFCrashWithInfo(int, char const*, char const*, int)
3   0x47a9eab4e JSC::VM::verifyExceptionCheckNeedIsSatisfied(unsigned int, JSC::ExceptionEventLocation&)
4   0x47a9c4e38 JSC::ThrowScope::throwException(JSC::JSGlobalObject*, JSC::JSValue)
5   0x47a713cad JSC::throwException(JSC::JSGlobalObject*, JSC::ThrowScope&, JSC::JSValue)
6   0x47a676674 JSC::throwTypeError(JSC::JSGlobalObject*, JSC::ThrowScope&, WTF::String const&)
7   0x47a676800 JSC::throwTypeError(JSC::JSGlobalObject*, JSC::ThrowScope&, WTF::ASCIILiteral)
8   0x45afaf05c WebCore::throwNonFiniteTypeError(JSC::JSGlobalObject&, JSC::ThrowScope&)
9   0x458ccea18 WebCore::Converter<WebCore::IDLDouble>::convert(JSC::JSGlobalObject&, JSC::JSValue)
10  0x458c44085 WebCore::Converter<WebCore::IDLDouble>::ReturnType WebCore::convert<WebCore::IDLDouble>(JSC::JSGlobalObject&, JSC::JSValue)
11  0x459877dfa WebCore::RTCPeerConnection::CertificateParameters WebCore::convertDictionary<WebCore::RTCPeerConnection::CertificateParameters>(JSC::JSGlobalObject&, JSC::JSValue)
12  0x45a8e798e WebCore::certificateTypeFromAlgorithmIdentifier(JSC::JSGlobalObject&, WTF::Variant<JSC::Strong<JSC::JSObject, (JSC::ShouldStrongDestructorGrabLock)0>, WTF::String>&&)
13  0x45a8e7746 WebCore::RTCPeerConnection::generateCertificate(JSC::JSGlobalObject&, WTF::Variant<JSC::Strong<JSC::JSObject, (JSC::ShouldStrongDestructorGrabLock)0>, WTF::String>&&, WebCore::DOMPromiseDeferred<WebCore::IDLInterface<WebCore::RTCCertificate> >&&)
14  0x4598a08e2 WebCore::jsRTCPeerConnectionConstructorFunction_generateCertificateBody(JSC::JSGlobalObject*, JSC::CallFrame*, WTF::Ref<WebCore::DeferredPromise, WTF::RawPtrTraits<WebCore::DeferredPromise> >&&)
15  0x4598a0cd4 long long WebCore::IDLOperationReturningPromise<WebCore::JSRTCPeerConnection>::callStatic<&(WebCore::jsRTCPeerConnectionConstructorFunction_generateCertificateBody(JSC::JSGlobalObject*, JSC::CallFrame*, WTF::Ref<WebCore::DeferredPromise, WTF::RawPtrTraits<WebCore::DeferredPromise> >&&)), (WebCore::CastedThisErrorBehavior)2>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*)::'lambda'(JSC::JSGlobalObject&, JSC::CallFrame&, WTF::Ref<WebCore::DeferredPromise, WTF::RawPtrTraits<WebCore::DeferredPromise> >&&)::operator()(JSC::JSGlobalObject&, JSC::CallFrame&, WTF::Ref<WebCore::DeferredPromise, WTF::RawPtrTraits<WebCore::DeferredPromise> >&&) const
16  0x4598a0afb JSC::JSValue WebCore::callPromiseFunction<long long WebCore::IDLOperationReturningPromise<WebCore::JSRTCPeerConnection>::callStatic<&(WebCore::jsRTCPeerConnectionConstructorFunction_generateCertificateBody(JSC::JSGlobalObject*, JSC::CallFrame*, WTF::Ref<WebCore::DeferredPromise, WTF::RawPtrTraits<WebCore::DeferredPromise> >&&)), (WebCore::CastedThisErrorBehavior)2>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*)::'lambda'(JSC::JSGlobalObject&, JSC::CallFrame&, WTF::Ref<WebCore::DeferredPromise, WTF::RawPtrTraits<WebCore::DeferredPromise> >&&)>(JSC::JSGlobalObject&, JSC::CallFrame&, &(WebCore::jsRTCPeerConnectionConstructorFunction_generateCertificateBody(JSC::JSGlobalObject*, JSC::CallFrame*, WTF::Ref<WebCore::DeferredPromise, WTF::RawPtrTraits<WebCore::DeferredPromise> >&&)))
17  0x4598a06b1 long long WebCore::IDLOperationReturningPromise<WebCore::JSRTCPeerConnection>::callStatic<&(WebCore::jsRTCPeerConnectionConstructorFunction_generateCertificateBody(JSC::JSGlobalObject*, JSC::CallFrame*, WTF::Ref<WebCore::DeferredPromise, WTF::RawPtrTraits<WebCore::DeferredPromise> >&&)), (WebCore::CastedThisErrorBehavior)2>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*)
18  0x4598a0684 WebCore::jsRTCPeerConnectionConstructorFunction_generateCertificate(JSC::JSGlobalObject*, JSC::CallFrame*)
19  0x30122f8011d8
20  0x4793d21ef llint_entry
21  0x4793b0250 vmEntryToJavaScript
22  0x47a26bb2b JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
23  0x47a26b088 JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::JSGlobalObject*, JSC::JSObject*)
24  0x47a648847 JSC::evaluate(JSC::JSGlobalObject*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
25  0x47a64899a JSC::profiledEvaluate(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
26  0x45b04d84c WebCore::JSExecState::profiledEvaluate(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
27  0x45b04d42e WebCore::ScriptController::evaluateInWorld(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld&)
28  0x45b04d259 WebCore::ScriptController::evaluateInWorldIgnoringException(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld&)
29  0x45b04db55 WebCore::ScriptController::evaluateIgnoringException(WebCore::ScriptSourceCode const&)
30  0x45b79e7e6 WebCore::ScriptElement::executeClassicScript(WebCore::ScriptSourceCode const&)
31  0x45b79c7fb WebCore::ScriptElement::prepareScript(WTF::TextPosition const&, WebCore::ScriptElement::LegacyTypeSupport)

<rdar://68912746>
Comment 1 Ryosuke Niwa 2021-03-26 02:32:21 PDT
Created attachment 424335 [details]
Test

<script>
  let handler = {
    get: Object
  };
  let p = new Proxy({}, handler);
  RTCPeerConnection.generateCertificate(p);
</script>
Comment 2 Ryosuke Niwa 2021-03-26 02:33:31 PDT
Reproduced this with debug build of WebKitTestRunner at r273504.

Note that you need to specify __XPC_JSC_validateExceptionChecks=1 on macOS or enable JSC's validateExceptionChecks option in other ports to reproduce this.
Comment 3 Darin Adler 2021-03-26 10:19:05 PDT
Is fixing this as simple as using RELEASE_AND_RETURN instead of return in the function in JSConvertNumber.h? Or do we need RETURN_IF_EXCEPTION?
Comment 4 Ryosuke Niwa 2021-03-27 03:53:25 PDT
(In reply to Darin Adler from comment #3)
> Is fixing this as simple as using RELEASE_AND_RETURN instead of return in
> the function in JSConvertNumber.h? Or do we need RETURN_IF_EXCEPTION?

I think what we need is RETURN_IF_EXCEPTION. The issue appears to be that we're throwing an expiation without first checking if there had been any exception.
Comment 5 Frédéric Wang (:fredw) 2021-04-12 06:54:41 PDT
Created attachment 425741 [details]
Patch

(In reply to Ryosuke Niwa from comment #4)
> (In reply to Darin Adler from comment #3)
> > Is fixing this as simple as using RELEASE_AND_RETURN instead of return in
> > the function in JSConvertNumber.h? Or do we need RETURN_IF_EXCEPTION?
> 
> I think what we need is RETURN_IF_EXCEPTION. The issue appears to be that
> we're throwing an expiation without first checking if there had been any
> exception.

Using RETURN_IF_EXCEPTION seems to work... but should we instead propagate the exception, similar to logic in JSDOMConvertBase.h? That would also be more consistent to how IDLUnrestrictedFloat/IDLUnrestrictedDouble's convert behave when toNumber throws an exception. Or maybe we should catch exception in the unrestricted versions too?
Comment 6 Darin Adler 2021-04-12 11:14:43 PDT
(In reply to Frédéric Wang (:fredw) from comment #5)
> Using RETURN_IF_EXCEPTION seems to work... but should we instead propagate
> the exception

HEre’s my understanding:

RETURN_IF_EXCEPTION does propagate an exception. It does not "catch" an exception.

The function named propagateException converts DOM exceptions into JavaScript exceptions, and is not needed or involved when we propagate JavaScript exceptions from one level to the next.
Comment 7 Frédéric Wang (:fredw) 2021-04-12 23:17:15 PDT
OK, then the patch looks correct to me as it then.
Comment 8 Ryosuke Niwa 2021-04-12 23:20:12 PDT
Comment on attachment 425741 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=425741&action=review

> Source/WebCore/ChangeLog:8
> +        No new tests.

Wait, why no new tests?
Comment 9 Ryosuke Niwa 2021-04-12 23:21:21 PDT
Note that you can put the following line in the first line of a test to enable this check in WebKitTestRunner:
<!DOCTYPE html><!-- webkit-test-runner [ jscOptions=--validateExceptionChecks=true ] -->
Comment 10 Frédéric Wang (:fredw) 2021-04-12 23:26:32 PDT
Comment on attachment 425741 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=425741&action=review

>> Source/WebCore/ChangeLog:8
>> +        No new tests.
> 
> Wait, why no new tests?

So for this an the other JSC bugs, I'm not sure when "not handling exception failure" is a security or not. And yes, adding the test is easy otherwise.
Comment 11 Ryosuke Niwa 2021-04-12 23:29:29 PDT
(In reply to Frédéric Wang (:fredw) from comment #10)
> Comment on attachment 425741 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=425741&action=review
> 
> >> Source/WebCore/ChangeLog:8
> >> +        No new tests.
> > 
> > Wait, why no new tests?
> 
> So for this an the other JSC bugs, I'm not sure when "not handling exception
> failure" is a security or not. And yes, adding the test is easy otherwise.

Yusuke says this isn't a security bug so we should add a test.
Comment 12 Yusuke Suzuki 2021-04-12 23:40:05 PDT
Comment on attachment 425741 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=425741&action=review

> Source/WebCore/bindings/js/JSDOMConvertNumbers.h:297
> +        RETURN_IF_EXCEPTION(scope, 0.0);
>          if (UNLIKELY(!std::isfinite(number)))
>              throwNonFiniteTypeError(lexicalGlobalObject, scope);
>          return static_cast<float>(number);

If we returned, then the upper scope will catch the exception anyway. If we call `throwNonFiniteTypeError` while we have an exception, the exception will be overridden. While this is not right semantics, this is not a security issue.

> Source/WebCore/bindings/js/JSDOMConvertNumbers.h:353
> +        RETURN_IF_EXCEPTION(scope, 0.0);
>          if (UNLIKELY(!std::isfinite(number)))
>              throwNonFiniteTypeError(lexicalGlobalObject, scope);
>          return number;

Ditto.
Comment 13 Frédéric Wang (:fredw) 2021-04-12 23:58:20 PDT
Created attachment 425840 [details]
Patch for landing
Comment 14 Ryosuke Niwa 2021-04-13 01:58:05 PDT
Oh you probably need to skip the test on Windows since there is no WebRTC support there.
Comment 15 Frédéric Wang (:fredw) 2021-04-13 02:41:59 PDT
Created attachment 425851 [details]
Patch for landing
Comment 16 EWS 2021-04-13 03:26:26 PDT
Committed r275877 (236442@main): <https://commits.webkit.org/236442@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 425851 [details].