Bug 105374

Created attachment 180070 [details] Patch

Created attachment 180995 [details] Patch

Hey Adam! It'd be nice if you could take a look at this, but it's certainly not a priority. Just throw it on your (massive?) post-holiday pile. :)

Comment on attachment 180995 [details] Patch Attachment 180995 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15630290 New failing tests: http/tests/security/contentSecurityPolicy/report-uri-from-javascript.html

Comment on attachment 180995 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=180995&action=review > Source/WebCore/page/ContentSecurityPolicy.cpp:1610 > + RefPtr<ScriptCallStack> stack = createScriptCallStack(1, false); What does the 1 mean in in the first position here? > Source/WebCore/page/ContentSecurityPolicy.cpp:1615 > + cspReport->setString("source-file", callFrame.sourceURL()); > + cspReport->setNumber("line-number", callFrame.lineNumber()); I guess this can be used to extract information from cross-origin scripts. Should we check whether the script is same-origin (or used CORS) the same way we do for window.onerror?

(In reply to comment #5) > (From update of attachment 180995 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=180995&action=review > > > Source/WebCore/page/ContentSecurityPolicy.cpp:1610 > > + RefPtr<ScriptCallStack> stack = createScriptCallStack(1, false); > > What does the 1 mean in in the first position here? The number of call stack frames to generate. Fewer is faster, and in this case, we only need the top frame so that we can point to the last line that executed. This is also, sadly, the cause of the Mac failure. Its top frame is 'native code', which isn't what we want. I'll probably grab two frames and check the second if a) it exists, and b) it has a line number. > > > Source/WebCore/page/ContentSecurityPolicy.cpp:1615 > > + cspReport->setString("source-file", callFrame.sourceURL()); > > + cspReport->setNumber("line-number", callFrame.lineNumber()); > > I guess this can be used to extract information from cross-origin scripts. Should we check whether the script is same-origin (or used CORS) the same way we do for window.onerror? Probably, yes.

Created attachment 181277 [details] Patch

Comment on attachment 181277 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=181277&action=review > Source/WebCore/page/ContentSecurityPolicy.cpp:1616 > + int frameNumber = 0; > + if (!stack->at(0).lineNumber() && stack->size() > 1 && stack->at(1).lineNumber()) > + frameNumber = 1; > + > + const ScriptCallFrame& callFrame = stack->at(frameNumber); I might have made this a helper function with a descriptive frame.

(In reply to comment #8) > (From update of attachment 181277 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=181277&action=review > > > Source/WebCore/page/ContentSecurityPolicy.cpp:1616 > > + int frameNumber = 0; > > + if (!stack->at(0).lineNumber() && stack->size() > 1 && stack->at(1).lineNumber()) > > + frameNumber = 1; > > + > > + const ScriptCallFrame& callFrame = stack->at(frameNumber); > > I might have made this a helper function with a descriptive frame. That makes sense. I'll spin a new patch with this bit extracted, and throw it into the CQ. Thanks!

Created attachment 181334 [details] Patch for landing

Comment on attachment 181334 [details] Patch for landing Clearing flags on attachment: 181334 Committed r138834: <http://trac.webkit.org/changeset/138834>

All reviewed patches have been landed. Closing bug.

The test added by this patch has been flaky on all bots: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=http%2Ftests%2Fsecurity%2FcontentSecurityPolicy%2Freport-uri-from-javascript.html

(In reply to comment #13) > The test added by this patch has been flaky on all bots: > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=http%2Ftests%2Fsecurity%2FcontentSecurityPolicy%2Freport-uri-from-javascript.html Thanks for filing https://bugs.webkit.org/show_bug.cgi?id=106165. I'll take care of the flake there.