Bug 195238

Summary: Fix crashes when NetworkResourceLoader/NetworkLoadChecker are destroyed during HTTPS upgrade check
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Alex Christensen 2019-03-01 19:55:49 PST
Fix crashes when NetworkResourceLoader/NetworkLoadChecker are destroyed during HTTPS upgrade check
Comment 1 Alex Christensen 2019-03-01 19:57:50 PST
Created attachment 363402 [details]
Patch
Comment 2 Chris Dumez 2019-03-01 20:49:40 PST
Comment on attachment 363402 [details]
Patch

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

> Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp:227
> +    applyHTTPSUpgradeIfNeeded(WTFMove(request), [this, weakThis = makeWeakPtr(*this), client, handler = WTFMove(handler), originalRequest = WTFMove(originalRequest)](auto request) mutable {

Seems suspicious that you'd fix this here but not in the processContentExtensionRulesForLoad() lambda below...
Comment 3 Alex Christensen 2019-03-04 11:53:13 PST
processContentExtensionRulesForLoad's implementation currently already has this protection, but there's no reason not to make this more robust.
Comment 4 Alex Christensen 2019-03-04 11:53:26 PST
Created attachment 363534 [details]
Patch
Comment 5 Chris Dumez 2019-03-04 11:54:35 PST
Comment on attachment 363534 [details]
Patch

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

> Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp:244
> +            if (!weakThis)

Personally, I would have put this below the if (!result.has_value()) check so that we may return a more accurate exception when we already have one.
Comment 6 Alex Christensen 2019-03-04 12:19:56 PST
Created attachment 363539 [details]
Patch
Comment 7 WebKit Commit Bot 2019-03-04 12:54:50 PST
Comment on attachment 363539 [details]
Patch

Clearing flags on attachment: 363539

Committed r242377: <https://trac.webkit.org/changeset/242377>
Comment 8 WebKit Commit Bot 2019-03-04 12:54:52 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2019-03-04 12:55:38 PST
<rdar://problem/48573506>