Bug 188358 - WKURLSchemeHandler crashes when sent errors with sync XHR
Summary: WKURLSchemeHandler crashes when sent errors with sync XHR
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: Safari 11
Hardware: iPhone / iPad iOS 11
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-08-06 14:14 PDT by Rob Napier
Modified: 2018-08-09 16:07 PDT (History)
4 users (show)

See Also:


Attachments
Demonstration Swift playground (15.80 KB, application/zip)
2018-08-06 14:14 PDT, Rob Napier
no flags Details
Patch (6.22 KB, patch)
2018-08-09 12:29 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rob Napier 2018-08-06 14:14:08 PDT
Created attachment 346655 [details]
Demonstration Swift playground

Calling urlSchemeTask.didFailWithError() prior to sending data crashes with the following stack:

error: Execution was interrupted, reason: EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0).
The process has been left at the point where it was interrupted, use "thread return -x" to return to the state before expression evaluation.

* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)
  * frame #0: 0x00000001223e5431 libswiftCore.dylib`function signature specialization <Arg[2] = Dead, Arg[3] = Dead> of Swift._fatalErrorMessage(_: Swift.StaticString, _: Swift.StaticString, file: Swift.StaticString, line: Swift.UInt, flags: Swift.UInt32) -> Swift.Never + 113
    frame #1: 0x000000012221ad63 libswiftCore.dylib`Swift._fatalErrorMessage(_: Swift.StaticString, _: Swift.StaticString, file: Swift.StaticString, line: Swift.UInt, flags: Swift.UInt32) -> Swift.Never + 19
    frame #2: 0x000000012679acfe $__lldb_expr18`main at Untitled Page.xcplaygroundpage:49
    frame #3: 0x000000010c89a600 MyPlayground`linkResources + 304
    frame #4: 0x000000010e03b76c CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__ + 12
    frame #5: 0x000000010e03af20 CoreFoundation`__CFRunLoopDoBlocks + 336
    frame #6: 0x000000010e035784 CoreFoundation`__CFRunLoopRun + 1284
    frame #7: 0x000000010e034f41 CoreFoundation`CFRunLoopRunSpecific + 625
    frame #8: 0x000000011560e1b5 GraphicsServices`GSEventRunModal + 62
    frame #9: 0x0000000111d23df4 UIKitCore`UIApplicationMain + 140
    frame #10: 0x000000010c89a6cd MyPlayground`main + 205
    frame #11: 0x000000010fa4b9ed libdyld.dylib`start + 1


Crash appears to be due to WebURLSchemeTask::didComplete, specifically this line (https://github.com/WebKit/webkit/blob/master/Source/WebKit/UIProcess/WebURLSchemeTask.cpp#L136):

m_syncCompletionHandler(m_syncResponse, error, IPC::DataReference { (const uint8_t*)m_syncData->data(), m_syncData->size() });

If didReceiveData has not been called yet, m_syncData will be nullptr, and crash.

didComplete explicitly permits no response to have been sent, if there is an error (note that the docs for Cocoa's didReceiveResponse are misleading about this; even if a response is sent, it doesn't address the issue, however).

    if (!m_responseSent && error.isNull())
        return ExceptionType::NoResponseSent;


This is also rdar://42974387
Comment 1 Chris Dumez 2018-08-09 11:56:24 PDT
<rdar://problem/42974387>
Comment 2 Chris Dumez 2018-08-09 11:56:41 PDT
I can take a look.
Comment 3 Chris Dumez 2018-08-09 12:04:27 PDT
Looks like Alex was already working on it.
Comment 4 Alex Christensen 2018-08-09 12:29:01 PDT
Created attachment 346855 [details]
Patch
Comment 5 Alex Christensen 2018-08-09 12:29:20 PDT
Thanks for the great report!
Comment 6 Chris Dumez 2018-08-09 14:16:33 PDT
Comment on attachment 346855 [details]
Patch

r=me
Comment 7 WebKit Commit Bot 2018-08-09 14:43:54 PDT
Comment on attachment 346855 [details]
Patch

Clearing flags on attachment: 346855

Committed r234735: <https://trac.webkit.org/changeset/234735>
Comment 8 WebKit Commit Bot 2018-08-09 14:43:56 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Alex Christensen 2018-08-09 16:07:39 PDT
http://trac.webkit.org/r234741