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

Alex Christensen
Reported 2019-03-01 19:55:49 PST
Fix crashes when NetworkResourceLoader/NetworkLoadChecker are destroyed during HTTPS upgrade check
Attachments
Patch (1.64 KB, patch)
2019-03-01 19:57 PST, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Patch (2.37 KB, patch)
2019-03-04 11:53 PST, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Patch (2.72 KB, patch)
2019-03-04 12:19 PST, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Alex Christensen
Comment 1 2019-03-01 19:57:50 PST
Created attachment 363402 [details] Patch
Chris Dumez
Comment 2 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...
Alex Christensen
Comment 3 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.
Alex Christensen
Comment 4 2019-03-04 11:53:26 PST
Created attachment 363534 [details] Patch
Chris Dumez
Comment 5 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.
Alex Christensen
Comment 6 2019-03-04 12:19:56 PST
Created attachment 363539 [details] Patch
WebKit Commit Bot
Comment 7 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>
WebKit Commit Bot
Comment 8 2019-03-04 12:54:52 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 9 2019-03-04 12:55:38 PST
<rdar://problem/48573506>
Note You need to log in before you can comment on or make changes to this bug.