Summary: | Web Inspector: Null ResourceResponse Preflight requests cause crash | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||
Component: | Web Inspector | Assignee: | Joseph Pecoraro <joepeck> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bburg, cdumez, commit-queue, dbates, japhet, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer, youennf | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Joseph Pecoraro
2016-11-28 15:47:29 PST
Created attachment 295545 [details]
[PATCH] Proposed Fix
Youeen if you have any ideas how we can get a Null Response here, please provide pointers and I can try and add a LayoutTest to cover it. The LayoutTests (http/tests/xmlhttprequest) seem to cover quite a few cases, and even after reading the code I'm unsure how we could get a Null response here without the Request being a failure or cancelled. For these crashes to happen Web Inspector would have to have been open, otherwise we wouldn't get into WebCore::InspectorNetworkAgent at all. Have you tested with a CORS preflight request that is redirected? (In reply to comment #5) > Have you tested with a CORS preflight request that is redirected? When handling such a case we get here <http://trac.webkit.org/browser/trunk/Source/WebCore/loader/SubresourceLoader.cpp?rev=208919#L190>. (In reply to comment #5) > Have you tested with a CORS preflight request that is redirected? I ran a LayoutTest that should cover this with Web Inspector open and didn't encounter any issues: http://127.0.0.1:8000/xmlhttprequest/redirections-and-user-headers.html However looking at that code: > ResourceResponse opaqueRedirectedResponse; > opaqueRedirectedResponse.setURL(redirectResponse.url()); > opaqueRedirectedResponse.setType(ResourceResponse::Type::Opaqueredirect); > m_resource->responseReceived(opaqueRedirectedResponse); ResourceResponseBase::setURL would make this response non-null. I'll try putting an early ASSERT in and seeing if a Debug build can catch this with any LayoutTests. Preflight requests redirect fetch mode is set to Manual so that no redirection is followed. For simplicity and efficiency currently, we return a null response at the "application level" (i.e. the preflight checker). But it seems useful to pass the information to the inspector (and service worker in the future). So maybe we should make a copy of the redirect response in manual-redirect mode, still setting the response type to opaqueRedirect. Comment on attachment 295545 [details]
[PATCH] Proposed Fix
r=me
Comment on attachment 295545 [details] [PATCH] Proposed Fix Clearing flags on attachment: 295545 Committed r209134: <http://trac.webkit.org/changeset/209134> All reviewed patches have been landed. Closing bug. |