Bug 190046

Summary: Web Inspector: crash in InspectorNetworkAgent::didReceiveResponse when loading denied x-frame resources
Product: WebKit Reporter: Devin Rousso <hi>
Component: Web InspectorAssignee: Devin Rousso <hi>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dbates, ews-watchlist, inspector-bugzilla-changes, joepeck, mkwst, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews106 for mac-sierra-wk2
none
Archive of layout-test-results from ews121 for ios-simulator-wk2
none
Patch
none
Patch none

Devin Rousso
Reported 2018-09-27 11:08:32 PDT
Create a test file "crash.html" and add the following to it: <iframe src="http://devinrousso.com"></iframe> Open the file in release/debug, open WebInspector, and refresh the page. This is caused by the `ResourceResponse` being "null", which means we generate a `nullptr` value for our `Inspector::Protocol::Network::Response`, which isn't JSON serializable, so crash.
Attachments
Patch (9.65 KB, patch)
2018-09-27 11:33 PDT, Devin Rousso
no flags
Archive of layout-test-results from ews106 for mac-sierra-wk2 (3.27 MB, application/zip)
2018-09-27 12:32 PDT, EWS Watchlist
no flags
Archive of layout-test-results from ews121 for ios-simulator-wk2 (2.51 MB, application/zip)
2018-09-27 16:58 PDT, EWS Watchlist
no flags
Patch (9.22 KB, patch)
2018-09-27 23:37 PDT, Devin Rousso
no flags
Patch (9.88 KB, patch)
2018-09-28 14:58 PDT, Devin Rousso
no flags
Devin Rousso
Comment 1 2018-09-27 11:33:20 PDT
EWS Watchlist
Comment 2 2018-09-27 12:32:14 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 3 2018-09-27 12:32:15 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 4 2018-09-27 16:58:18 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 5 2018-09-27 16:58:20 PDT Comment hidden (obsolete)
Devin Rousso
Comment 6 2018-09-27 23:37:50 PDT
Joseph Pecoraro
Comment 7 2018-09-28 10:24:09 PDT
Comment on attachment 350982 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=350982&action=review Oops apparently I never submitted these comments. > Source/WebKit/WebProcess/Network/WebResourceLoader.cpp:190 > - m_coreLoader->documentLoader()->stopLoadingAfterXFrameOptionsOrContentSecurityPolicyDenied(m_coreLoader->identifier(), ResourceResponse { }); > + m_coreLoader->documentLoader()->stopLoadingAfterXFrameOptionsOrContentSecurityPolicyDenied(m_coreLoader->identifier(), response); My guess is that this now non-null response is going to cause differences in WebCore handling of the response separate from Web Inspector code. That might be a good thing or a bad thing. It looks like some tests saw differences. > LayoutTests/http/tests/inspector/network/x-frame-options.html:18 > + description: "Ensure that x-frame/CSP denials are recieved and don't crash.", Nit: "X-Frame-Options" instead of "x-frame" is clearer and easier to search for. Also drop the "don't crash" part, which is pretty much always our expectation. This fix eliminates the crash but then also provides inspector with the correct data it wasn't getting before and the description of the test can be the latter which. > LayoutTests/http/tests/inspector/network/x-frame-options.html:35 > +<p>Tests for various x-frame-options headers.</p> Nit: X-Frame-Options Did you want to add another test for a different option, like "sameorigin"?
Joseph Pecoraro
Comment 8 2018-09-28 10:25:13 PDT
Comment on attachment 351058 [details] Patch r=me, but the same comments apply regarding the test.
Devin Rousso
Comment 9 2018-09-28 14:58:09 PDT
WebKit Commit Bot
Comment 10 2018-09-28 17:22:15 PDT
Comment on attachment 351117 [details] Patch Clearing flags on attachment: 351117 Committed r236627: <https://trac.webkit.org/changeset/236627>
WebKit Commit Bot
Comment 11 2018-09-28 17:22:17 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 12 2018-09-28 17:23:38 PDT
Note You need to log in before you can comment on or make changes to this bug.