Skip to content

Commit

Permalink
Close a WebSocketTask if it completes with an error without closing p…
Browse files Browse the repository at this point in the history
…roperly

https://bugs.webkit.org/show_bug.cgi?id=274003
rdar://110487778

Reviewed by Brady Eidson.

If the TCP connection has an error but we have not received a close frame, then
CFNetwork will call didCompleteWithError but won't call the completion block of
receiveMessageWithCompletionHandler.  This is strange, but to make our interface
with CFNetwork more robust, close the WebSocket in this case if it hasn't already
been closed.  WebSocketTask::didClose will ignore this call if it has already
been closed.

Unfortunately, the API doesn't exist to make a unit test for this.  In attempting
to make a unit test, I filed rdar://127883777 which, when resolved will allow us
to test cases similar to this.  That radar shows that future work is still to be
done in this area, but manual testing verifies this fixes the issue in the
original radar of rdar://110487778

There are 3 ways a WebSocketTask can close:
1. receiveMessageWithCompletionHandler completion block can be called with an error
2. URLSession:webSocketTask:didCloseWithCode:reason: can be called
3. URLSession:task:didCompleteWithError: can be called
It is an error for didCompleteWithError to be called before another call has closed
the WebSocket, so this change calling didClose whether error is null or not is correct.

* Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:
(-[WKNetworkSessionDelegate URLSession:task:didCompleteWithError:]):

Canonical link: https://commits.webkit.org/278623@main
  • Loading branch information
achristensen07 committed May 10, 2024
1 parent 1399a72 commit ebe1ce9
Showing 1 changed file with 6 additions and 1 deletion.
7 changes: 6 additions & 1 deletion Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
#import <WebCore/ResourceRequest.h>
#import <WebCore/ResourceResponse.h>
#import <WebCore/SharedBuffer.h>
#import <WebCore/ThreadableWebSocketChannel.h>
#import <WebCore/WebCoreURLResponse.h>
#import <pal/spi/cf/CFNetworkSPI.h>
#import <wtf/BlockPtr.h>
Expand Down Expand Up @@ -899,7 +900,11 @@ - (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task didComp

if (auto networkDataTask = [self existingTask:task])
networkDataTask->didCompleteWithError(error, networkDataTask->networkLoadMetrics());
else if (error) {
else if (auto* webSocketTask = [self existingWebSocketTask:task]) {
// Even if error is null here, it's an error to close a WebSocket without a close frame which should be received
// in URLSession:webSocketTask:didCloseWithCode:reason: but this can be hit in some TCP error cases like rdar://110487778
webSocketTask->didClose(WebCore::ThreadableWebSocketChannel::CloseEventCodeAbnormalClosure, emptyString());
} else if (error) {
if (!_sessionWrapper)
return;
auto downloadID = _sessionWrapper->downloadMap.take(task.taskIdentifier);
Expand Down

0 comments on commit ebe1ce9

Please sign in to comment.