152825 – Web Inspector: CRASH Attempting to pause on CSP violation not inside of script

RESOLVED FIXED 152825

Web Inspector: CRASH Attempting to pause on CSP violation not inside of script

https://bugs.webkit.org/show_bug.cgi?id=152825

Summary Web Inspector: CRASH Attempting to pause on CSP violation not inside of script

Joseph Pecoraro

Reported 2016-01-06 19:57:44 PST

* SUMMARY CRASH Attempting to pause on CSP violation not inside of script * TEST <meta http-equiv="Content-Security-Policy" content="script-src 'none'"> <script>alert("test");</script> * STEPS TO REPRODUCE 1. Inspect test page 2. Enable Debugger "Break on All Exceptions" breakpoint 3. Reload => CRASH/ASSERT * ASSERT void Debugger::breakProgram() { … m_pauseOnNextStatement = true; setSteppingMode(SteppingModeEnabled); m_currentCallFrame = m_vm.topCallFrame; ASSERT(m_currentCallFrame); // <--- pauseIfNeeded(m_currentCallFrame); } Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 WTFCrash + 39 (Assertions.cpp:321) 1 JSC::Debugger::breakProgram() + 139 (Debugger.cpp:552) 2 Inspector::InspectorDebuggerAgent::breakProgram 3 Inspector::InspectorDebuggerAgent::scriptExecutionBlockedByCSP 4 WebCore::InspectorInstrumentation::scriptExecutionBlockedByCSPImpl 5 WebCore::InspectorInstrumentation::scriptExecutionBlockedByCSP 6 WebCore::ContentSecurityPolicy::reportBlockedScriptExecutionToInspector 7 WebCore::CSPDirectiveList::checkInlineAndReportViolation 8 WebCore::CSPDirectiveList::allowInlineScript 9 bool WebCore::isAllowedByAllWithContext 10 WebCore::ContentSecurityPolicy::allowInlineScript 11 WebCore::ScriptElement::executeScript 12 WebCore::ScriptElement::prepareScript 13 WebCore::HTMLScriptRunner::runScript 14 WebCore::HTMLScriptRunner::execute 15 WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder

Attachments
[PATCH] Proposed Fix (11.29 KB, patch) 2016-01-06 20:05 PST, Joseph Pecoraro	timothy: review+ dbates: commit-queue-	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Joseph Pecoraro

Comment 1 2016-01-06 20:04:21 PST

<rdar://problem/24021276>

Joseph Pecoraro

Comment 2 2016-01-06 20:05:12 PST

Created attachment 268438 [details] [PATCH] Proposed Fix

Daniel Bates

Comment 3 2016-01-06 21:11:43 PST

Comment on attachment 268438 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=268438&action=review > LayoutTests/ChangeLog:4 > + https://bugs.webkit.org/show_bug.cgi?id=152825 Please add the radar URL, <rdar://problem/24021276>, under this line. > LayoutTests/inspector/debugger/csp-exceptions.html:5 > +<script type="text/javascript" src="../../http/tests/inspector/resources/inspector-test.js"></script> This is OK as-is. The presence of the type attribute in this context is unnecessary as it defaults to "text/javascript" given that this is a HTML5 document by <https://html.spec.whatwg.org/multipage/scripting.html#attr-script-type> (6 January 2016). > LayoutTests/inspector/debugger/csp-exceptions.html:10 > +function triggerCSPExceptionInsideScript() { Nit: The placement of the opening curly brace for this function differs from the placement of the opening curly brace for function test (line 21). I suggest that we follow the WebKit Code Style Guideline and place the opening curly brace on its own line. Regardless, we should pick one style convention for writing a named function and stick with it throughout this document. > LayoutTests/inspector/debugger/csp-exceptions.html:14 > +function triggerCSPExceptionOutsideScript() { Ditto. > LayoutTests/inspector/debugger/resources/csp-exception-iframe.html:1 > +<meta http-equiv="Content-Security-Policy" content="script-src 'none'"> Please add <!DOCTYPE html> to the top of this file to specify that its conforms to the HTML5 standard. The markup of this file is only well formed with respect to the HTML5 standard. Specifically, this document takes advantage of the optional tags feature of the HTML5 standard to omit the <html>, </html>, <head>, </head>, <body>, and </body> tags. > Source/JavaScriptCore/ChangeLog:4 > + https://bugs.webkit.org/show_bug.cgi?id=152825 Please add the radar URL, <rdar://problem/24021276>, under this line. > Source/WebInspectorUI/ChangeLog:4 > + https://bugs.webkit.org/show_bug.cgi?id=152825 Please add the radar URL, <rdar://problem/24021276>, under this line.

Joseph Pecoraro

Comment 4 2016-01-07 10:49:48 PST

(In reply to comment #3) > Comment on attachment 268438 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=268438&action=review > > > LayoutTests/ChangeLog:4 > > + https://bugs.webkit.org/show_bug.cgi?id=152825 > > Please add the radar URL, <rdar://problem/24021276>, under this line. > > > LayoutTests/inspector/debugger/csp-exceptions.html:5 > > +<script type="text/javascript" src="../../http/tests/inspector/resources/inspector-test.js"></script> > > This is OK as-is. The presence of the type attribute in this context is > unnecessary as it defaults to "text/javascript" given that this is a HTML5 > document by > <https://html.spec.whatwg.org/multipage/scripting.html#attr-script-type> (6 > January 2016). > > > LayoutTests/inspector/debugger/csp-exceptions.html:10 > > +function triggerCSPExceptionInsideScript() { > > Nit: The placement of the opening curly brace for this function differs from > the placement of the opening curly brace for function test (line 21). I > suggest that we follow the WebKit Code Style Guideline and place the opening > curly brace on its own line. Regardless, we should pick one style convention > for writing a named function and stick with it throughout this document. I have been using this style for inspector tests for a while. Code that runs on the inspected page being very concise, code that runs in the inspector (the `test` function) matching the Web Inspector style. > > Source/JavaScriptCore/ChangeLog:4 > > + https://bugs.webkit.org/show_bug.cgi?id=152825 > > Please add the radar URL, <rdar://problem/24021276>, under this line. I've seen others do this. When do you consider it worthwhile to do this? Essentially every inspector bug has a radar. Given that this isn't automated by any tools it would mean manually editing each ChangeLog.

Brent Fulgham

Comment 5 2016-01-07 10:56:19 PST

Comment on attachment 268438 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=268438&action=review >>> LayoutTests/ChangeLog:4 >>> + https://bugs.webkit.org/show_bug.cgi?id=152825 >> >> Please add the radar URL, <rdar://problem/24021276>, under this line. > > I have been using this style for inspector tests for a while. Code that runs on the inspected page being very concise, code that runs in the inspector (the `test` function) matching the Web Inspector style. I always add the Radar when I have one, because it makes life better when reviewing changes in Trac. I am in full agreement with Dan on this one! :-) >>> Source/JavaScriptCore/ChangeLog:4 >>> + https://bugs.webkit.org/show_bug.cgi?id=152825 >> >> Please add the radar URL, <rdar://problem/24021276>, under this line. > > I've seen others do this. When do you consider it worthwhile to do this? > > Essentially every inspector bug has a radar. Given that this isn't automated by any tools it would mean manually editing each ChangeLog. I always add the Radar when I have one, because it makes life better when reviewing changes in Trac. I am in full agreement with Dan on this one! :-)

Joseph Pecoraro

Comment 6 2016-01-07 11:23:54 PST

<http://trac.webkit.org/changeset/194704>

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Nightly Build

Hardware All

OS All

Product WebKit

Component Web Inspector

Assignee

Joseph Pecoraro

Reported

2016-01-06 19:57 PST

Modified

2016-01-07 11:23 PST History

CC List

15 users Show

URL

Keywords InRadar

Depends on

Blocks