Bug 58646 - CSP policy violations should log to the console
Summary: CSP policy violations should log to the console
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 53572
  Show dependency treegraph
 
Reported: 2011-04-15 00:29 PDT by Adam Barth
Modified: 2011-04-20 19:34 PDT (History)
4 users (show)

See Also:


Attachments
Patch (28.01 KB, patch)
2011-04-19 23:37 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch (43.08 KB, patch)
2011-04-20 00:09 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch for landing (41.76 KB, patch)
2011-04-20 13:24 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from eseidel-cq-sf (197.02 KB, application/zip)
2011-04-20 15:12 PDT, WebKit Commit Bot
no flags Details
Patch for landing (40.25 KB, patch)
2011-04-20 18:52 PDT, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2011-04-15 00:29:08 PDT
CSP policy violations should log to the console
Comment 1 Adam Barth 2011-04-19 23:37:37 PDT
Created attachment 90312 [details]
Patch
Comment 2 Adam Barth 2011-04-19 23:44:20 PDT
Comment on attachment 90312 [details]
Patch

These tests are flaky as written (order dependent).  Will fix.
Comment 3 WebKit Review Bot 2011-04-19 23:55:38 PDT
Attachment 90312 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8473567
Comment 4 Adam Barth 2011-04-20 00:09:06 PDT
Created attachment 90318 [details]
Patch
Comment 5 WebKit Review Bot 2011-04-20 00:18:28 PDT
Attachment 90312 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8473573
Comment 6 Eric Seidel (no email) 2011-04-20 12:09:16 PDT
Comment on attachment 90318 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=90318&action=review

Looks fine.  The only nit which really matters is the repeating of all that logging code.

> Source/WebCore/page/ContentSecurityPolicy.cpp:492
> +    if (!allowed) {

I might have reversed these ifs to flatten these blocks:
if (allowed)
    return allowed; (or simply true;)

But this is also OK.

> Source/WebCore/page/ContentSecurityPolicy.cpp:495
> +        if (Frame* frame = m_document->frame())
> +            frame->domWindow()->console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, consoleMessage, 1, String());

Seems like these two lines want to be a helper.  We repeat them 7 times. :)

> Source/WebCore/page/ContentSecurityPolicy.cpp:702
> +        m_scriptSrc = adoptPtr(new CSPDirective(value, m_document->securityOrigin()));

I would have added an origin() or securityOrigin() private method.  BUt this is OK.
Comment 7 Adam Barth 2011-04-20 13:24:33 PDT
Created attachment 90397 [details]
Patch for landing
Comment 8 Adam Barth 2011-04-20 13:26:20 PDT
> I would have added an origin() or securityOrigin() private method.  BUt this is OK.

I did everything except this last one.
Comment 9 WebKit Commit Bot 2011-04-20 15:12:12 PDT
Comment on attachment 90397 [details]
Patch for landing

Rejecting attachment 90397 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sf', 'bu..." exit_code: 2

Last 500 characters of output:
mlmp .
http/tests/xmlhttprequest ................................................................................................................................................................................
http/tests/xmlhttprequest/web-apps ...............
http/tests/xmlhttprequest/workers ...........
http/tests/xmlviewer .
http/tests/xmlviewer/dumpAsText ............
747.48s total testing time

23330 test cases (99%) succeeded
1 test case (<1%) was new
15 test cases (<1%) had stderr output

Full output: http://queues.webkit.org/results/8468863
Comment 10 WebKit Commit Bot 2011-04-20 15:12:15 PDT
Created attachment 90425 [details]
Archive of layout-test-results from eseidel-cq-sf

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: eseidel-cq-sf  Port: Mac  Platform: Mac OS X 10.6.4
Comment 11 Adam Barth 2011-04-20 18:52:50 PDT
Created attachment 90473 [details]
Patch for landing
Comment 12 WebKit Commit Bot 2011-04-20 19:34:12 PDT
Comment on attachment 90473 [details]
Patch for landing

Clearing flags on attachment: 90473

Committed r84457: <http://trac.webkit.org/changeset/84457>
Comment 13 WebKit Commit Bot 2011-04-20 19:34:19 PDT
All reviewed patches have been landed.  Closing bug.