Closed Bug 1013507 Opened 10 years ago Closed 10 years ago

Write testcase to verify different console messages for CSP and CSP-RO

Categories

(Core :: DOM: Security, defect)

Product:
Core
Component:
DOM: Security
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla32

People

(Reporter: ckerschb, Assigned: freddy)

References

Details

Attachments

(1 file, 1 obsolete file)

bug-1010953.patch
6.10 KB, patch
freddy
: review+
Details | Diff | Splinter Review
Bug 1010953 introduces different console message for CSP violations and CSP(report only) violations. This bug is going to provide a testcase for that change.
> I think we should have a file loading with 2 policies (one CSP and and CSP-report-only), you can simply load 2 scripts, one that is allowed by CSP and one only by CSP-RO; afterwards just check that we correctly log to the console. I'm not sure I understand which cases we want to catch with the test. Why do we need 2 policies?
Flags: needinfo?(mozilla)
Hey Freddy, let me clarify: I would like to see the two different console messages in the test: a) one that logs a console message due to a CSP violation, and b) one that logs a console message due to a CSP-RO violation. For example, you could use the following test-setup: Content-Security-Policy: default-src 'self'; img-src: 'self'; script-src example.com; Content-Security-Policy-Report-Only: default-src 'self'; img-src: example.com; script-src 'self'; And then load an image from example.com which would violate the CSP policy and also load a script from example.com which would violate the CSP-RO policy. We would/should see the two different console messages. Does that make sense?
Flags: needinfo?(mozilla)
Attached patch bug-1013507.patch (obsolete) — — Splinter Review
How about this?
Assignee: nobody → fbraun
Status: NEW → ASSIGNED
Attachment #8426973 - Flags: review?(mozilla)
Trying this caused me a lot of pain until I realized that this test should *not* be run with ./mach mochitest-browser but with mochitest-devtools. E.g. > ./mach mochitest-devtools browser/devtools/webconsole/test/browser_webconsole_bug_1010953_cspro.js
Comment on attachment 8426973 [details] [diff] [review] bug-1013507.patch Review of attachment 8426973 [details] [diff] [review]: ----------------------------------------------------------------- Thanks Freddy for writing the test, looks good to me. Please address my nitpicks, run it through try and and r=me. ::: browser/devtools/webconsole/test/browser_webconsole_bug_1010953_cspro.js @@ +7,5 @@ > +// Tests that the Web Console CSP report only messages are displayed > + > +const TEST_VIOLATION = "http://example.com/browser/browser/devtools/webconsole/test/test_bug_1010953_cspro.html"; > +const CSP_VIOLATION_MSG = 'Content Security Policy: The page\'s settings blocked the loading of a resource at http://some.example.com/test.png ("img-src http://example.com:80").'; > +const CSP_REPORT_MSG = 'The page\'s settings observed the loading of a resource at http://some.example.com/test_bug_1010953_cspro.js ("script-src http://example.com:80"). A CSP report is being sent.'; I am confused, why doesn't CSP_REPORT_MSG not also start with "Content Security Policy: "? I assume(hope) that the actual console message starts with that string, but you just didn't put it in the const var, right? Please add it, so we can test as precisely as possible. ::: browser/devtools/webconsole/test/test_bug_1010953_cspro.html @@ +1,5 @@ > +<!DOCTYPE HTML> > +<html> > +<head> > + <meta charset="UTF-8"> > + <title>Test for Bug 1010953 - logging when CSP is in report only mode.</title> Can we make this more descriptive: Test for Bug 1010953 - Verify that CSP and CSPRO log different console messages. @@ +23,5 @@ > +Overview as suggested by Christoph Kerschbaumer [:ckerschb] in bug 1010953 comment 12: > + > +... We think it would be great to have a testcase for that change though. I think we should have a file loading with 2 policies (one CSP and and CSP-report-only), you can simply load 2 scripts, one that is allowed by CSP and one only by CSP-RO; afterwards just check that we correctly log to the console. > + > +--> move the overview of the test to the JS file, since the JS file starts the test; no need to mention my name, just describe what the test is doing, e.g. Description of the test: We are loading: a) a script, which is allowed by the CSP header but not by the CSPRO header - we confirm that the right message is displayed in the console. b) an image, which is allows by the CSPRO header but not by the CSP header - we confirm that the correct message is displayed in the console. I guess you can even replace the comment on top of browser_webconsole_bug_1010953_cspro.js with that comment ^^.
Attachment #8426973 - Flags: review?(mozilla) → review+
Blocks: 1011058
Addressing the mentioned nits and carrying over review+ from ckerschb.
Attachment #8426973 - Attachment is obsolete: true
Attachment #8427605 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e91fc820b8c3
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: