https://w3c.github.io/webappsec/specs/content-security-policy/#fire-a-violation-event Would be useful for https://bugzilla.mozilla.org/show_bug.cgi?id=1036975#c4

(In reply to David Bruant from comment #0) > https://w3c.github.io/webappsec/specs/content-security-policy/#fire-a- > violation-event This link seems deprecated. Is this its new location? https://w3c.github.io/webappsec-csp/#violation-events

(In reply to Chung-Sheng Fu [:cfu] from comment #1) > This link seems deprecated. Is this its new location? > https://w3c.github.io/webappsec-csp/#violation-events Chris, could you confirm this?

In the end changing this bug should also enable us to switch on a lot of web-platform tests that are disabled at the moment because we are not supporting those policy violation events.

Comment on attachment 8920052 [details] Bug 1037335 - Fix test failures. https://reviewboard.mozilla.org/r/191044/#review196258 C/C++ static analysis found 1 defect in this patch. You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp` If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: dom/script/ScriptLoader.cpp:1224 (Diff revision 1) > nsAutoString nonce; > scriptContent->GetAttr(kNameSpaceID_None, nsGkAtoms::nonce, nonce); > bool parserCreated = aElement->GetParserCreated() != mozilla::dom::NOT_FROM_PARSER; > > bool allowInlineScript = false; > - rv = csp->GetAllowsInline(nsIContentPolicy::TYPE_SCRIPT, > + rv = csp->GetAllowsInline(scriptContent, Warning: Value stored to 'rv' is never read [clang-tidy: clang-analyzer-deadcode.DeadStores]

These are the patches that implement security policy violation event. Please take a look. However, I got many error from tests in testing/web-platform/tests/content-security-policy/securitypolicyviolation/. Here are the common failure cases: - event.blockedURI - Should be "inline" or "eval" under certain circumstances, but now I can only get "self". - Stripped in cross origin case. I can't find this rule in the spec. http://searchfox.org/mozilla-central/rev/d0448c970093f94bd986a21d3a9e8c366b316eb4/dom/security/nsCSPContext.cpp#802 - event.sample - Sometimes different from our expectation. - Not hidden when there is no "report-sample" value for "script-src" and "style-src" directives. - event.violatedDirective/effectiveDirective - Contains values, looking like originalPolicy. Since I'm just reusing existing code to collect event data, I think the CSP framework has some problems. http://searchfox.org/mozilla-central/rev/d0448c970093f94bd986a21d3a9e8c366b316eb4/dom/security/nsCSPContext.cpp#866-930 Should I also fix them in this bug? Below is the detail of test failures: - blockeduri-eval.html - exception.name: expected "EvalError" but got "Error" - event.blockedURI: expected "eval" but got "self" - blockeduri-inline.html - event.blockedURI: expected "inline" but got "self" - idl.html - this.array.members[this.base] is undefined - this.array.members[inherit_interface] is undefined - this.array.members has "SecurityPolicyViolationEventDisposition", "SecurityPolicyViolationEventInit", "SecurityPolicyViolationEvent", but this.base and inherit_interface are "Event". Is it a parser bug? - inside-dedicated-worker.html - inside-service-worker.https.html - inside-shared-worker.html - No event fired. But I can't also see violation report in JS console. I'm not sure what happens in worker. - script-sample-no-opt-in.html - event.blockedURI: self - event.sample: not blank - script-sample.html - event.blockedURI: self - event.sample - Inline event handlers: expected "assert_unreached('inline event handler')" but got "onclick attribute on A element" - JavaScript URLs in iframes: expected "javascript:'inline url'" but got "" - eval(), setInterval(), setTimeout(): expected "assert_unreached('eval')" but got "call to eval() or related function blocked by CSP" - Seems cached - securitypolicyviolation-block-cross-origin-image-from-script.sub.html - securitypolicyviolation-block-cross-origin-image.sub.html - securitypolicyviolation-block-image-from-script.sub.html - event.blockedURI: stripped - event.violatedDirective: contains values - securitypolicyviolation-block-image.sub.html - event.violatedDirective: contains values - style-sample-no-opt-in.html - event.blockedURI: self - event.sample: not blank - style-sample.html - event.blockedURI: self - targeting.html - event.blockedURI: self - event.lineNumber - attachShadow is not a function: according to MDN, Firefox doesn’t support this function https://developer.mozilla.org/en-US/docs/Web/API/Element/attachShadow#Browser_compatibility - Elements created in this document, but pushed into a same-origin frame: it seems to me that the event should be fired to the policy's document, instead of the target's root document - upgrade-insecure-requests-reporting.https.html - event.effectiveDirective: contains values

Fixed static analysis warning and added a mochitest.

Updated web-platform tests. Most of them expected TIMEOUT because the event was not fired. For the IDL test, because SecurityPolicyViolationEvent inherits Event, an empty interface Event is required in the #untested element.

Comment on attachment 8920996 [details] Bug 1037335 - Add a mochitest for security policy violation event. https://reviewboard.mozilla.org/r/191966/#review198162 that looks good, r=me, thanks

Comment on attachment 8921310 [details] Bug 1037335 - Fix test failures. https://reviewboard.mozilla.org/r/192312/#review198164 All those updates look fine, except I am not entirely sure what the idl update is about. If you can explain, then r+ ::: testing/web-platform/tests/content-security-policy/securitypolicyviolation/idl.html:26 (Diff revision 1) > long lineNumber; > long columnNumber; > }; > + > + interface Event { > + }; can you add a comment why this is needed?

Comment on attachment 8920052 [details] Bug 1037335 - Fix test failures. https://reviewboard.mozilla.org/r/191044/#review198186 ::: dom/security/nsCSPContext.cpp:245 (Diff revision 3) > > // Do not send a report or notify observers if this is a preload - the > // decision may be wrong due to the inability to get the nonce, and will > // incorrectly fail the unit tests. > if (!aIsPreload && aSendViolationReports) { > - this->AsyncReportViolation((aSendContentLocationInViolationReports ? > + this->AsyncReportViolation(nullptr, /* TODO event target */ I think you don't need that entire patch. If I am not mistaken the event target should always be the including document, right? In that case you could simply query nsCOMPtr<nsIDocument> doc = do_QueryReferent(mLoadingContext); and pass the doc to AsyncReportViolation.

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #19)olationReports ? > > + this->AsyncReportViolation(nullptr, /* TODO event target */ Now that I think about that line a little more. As the patch stands right now you would miss out on all events that are triggered by violations due to loading external resources. Ultimately changing this to using the doc should enable even more wpt tests.

Comment on attachment 8920053 [details] Bug 1037335 - Implement security policy violation event. https://reviewboard.mozilla.org/r/191046/#review198214 I am clearing ni? for now until we have resolved the issue in the other patch. As discussed, probably we should gather a list of wpt tests that are failing because of incorrect stripping of the uri. I think we should assemble that list and the re-evaluate what parts of the URI need to be stripped. Also, please explain a little what you are doing in your code: * as a comment within bugzilla * and also add comments to the code itself. Thanks!

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #18) > Comment on attachment 8921310 [details] > Bug 1037335 - Update web-platform tests. > > https://reviewboard.mozilla.org/r/192312/#review198164 > > All those updates look fine, except I am not entirely sure what the idl > update is about. If you can explain, then r+ > > ::: > testing/web-platform/tests/content-security-policy/securitypolicyviolation/ > idl.html:26 > (Diff revision 1) > > long lineNumber; > > long columnNumber; > > }; > > + > > + interface Event { > > + }; > > can you add a comment why this is needed? During the test, these 2 line always throw exceptions that this.array.members[this.base] and this.array.members[inherit_interface] are undefined: http://searchfox.org/mozilla-central/rev/dd47bee6468de7e1221b4d006342ad6b9813d0e5/dom/imptests/idlharness.js#707,833 According to my observation, this.base and inherit_interface are "Event", while this.array.members looks like { SecurityPolicyViolationEventDisposition: {...}, SecurityPolicyViolationEventInit: {...}, SecurityPolicyViolationEvent: {...} } So I guess the input data should include interface Event so that the parser can create a related record. Other IDL tests also do this, e.g. http://searchfox.org/mozilla-central/rev/dd47bee6468de7e1221b4d006342ad6b9813d0e5/dom/events/test/pointerevents/idlharness.html#37,57

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #19) > Comment on attachment 8920052 [details] > Bug 1037335 - Pass event target to nsCSPContext::AsyncReportViolation. > > https://reviewboard.mozilla.org/r/191044/#review198186 > > ::: dom/security/nsCSPContext.cpp:245 > (Diff revision 3) > > > > // Do not send a report or notify observers if this is a preload - the > > // decision may be wrong due to the inability to get the nonce, and will > > // incorrectly fail the unit tests. > > if (!aIsPreload && aSendViolationReports) { > > - this->AsyncReportViolation((aSendContentLocationInViolationReports ? > > + this->AsyncReportViolation(nullptr, /* TODO event target */ > > I think you don't need that entire patch. If I am not mistaken the event > target should always be the including document, right? > > In that case you could simply query > nsCOMPtr<nsIDocument> doc = do_QueryReferent(mLoadingContext); > > and pass the doc to AsyncReportViolation. Thank you for pointing this out. However, I get confused after further read the spec. CSP Level 2 writes "Fire a violation event at the protected resource’s Document" https://www.w3.org/TR/CSP2/#violation-reports Does it mean the owner document of the violation's element? CSP Level 3 writes "fires a SecurityPolicyViolationEvent at violation’s global object" https://www.w3.org/TR/CSP3/#report-violation It seems to be the loading context of the policy to me, i.e., mLoadingContext in nsCSPContext. What's more interesting is another version of CSP Level 3, maybe the latest version? https://w3c.github.io/webappsec-csp/#report-violation There are some rules about the event target. I'm following this because Google Chrome behaves this way and our web-platform tests expect this result, e.g. http://searchfox.org/mozilla-central/rev/dd47bee6468de7e1221b4d006342ad6b9813d0e5/testing/web-platform/tests/content-security-policy/securitypolicyviolation/targeting.html I think there is no "correct" one, but which is the best for us to implement?

According to previous discussion, now I simply use the policy's loading context (i.e., nsCOMPtr<nsIDocument> doc = do_QueryReferent(mLoadingContext);) as event target. I also fixed all the test failures, please take a look again. There are 2 "Fix test failures" patches because some of the tests require review from DOM peer. Here is the try link: https://treeherder.mozilla.org/#/jobs?repo=try&revision=130b624f4d9ebdcaeacd00bdc5f40840864f0507 Besides, this web-platform test suffers from timing issue http://searchfox.org/mozilla-central/source/testing/web-platform/tests/content-security-policy/style-src/style-src-imported-style-blocked.html The violation event is sometimes fired before document.addEventListener, so I would like to update the test by > document.addEventListener("securitypolicyviolation", t_spv.step_func_done(function(e) { > assert_equals("style-src", e.violatedDirective); > })); > + > + var l = document.createElement("link"); > + l.rel = "stylesheet"; > + l.href = "/content-security-policy/style-src/resources/style-src-import.sub.css"; > + document.head.appendChild(l); > </script> > - <link href="/content-security-policy/style-src/resources/style-src-import.sub.css" rel=stylesheet type=text/css> > </head> > <body> > <div id='log'></div> Can I make such change in wpt?

(In reply to Chung-Sheng Fu [:cfu] from comment #23) > Thank you for pointing this out. However, I get confused after further read > the spec. > > CSP Level 2 writes "Fire a violation event at the protected resource’s > Document" > https://www.w3.org/TR/CSP2/#violation-reports > Does it mean the owner document of the violation's element? > > CSP Level 3 writes "fires a SecurityPolicyViolationEvent at violation’s > global object" > https://www.w3.org/TR/CSP3/#report-violation > It seems to be the loading context of the policy to me, i.e., > mLoadingContext in nsCSPContext. > > What's more interesting is another version of CSP Level 3, maybe the latest > version? > https://w3c.github.io/webappsec-csp/#report-violation > There are some rules about the event target. I'm following this because > Google Chrome behaves this way and our web-platform tests expect this > result, e.g. > http://searchfox.org/mozilla-central/rev/ > dd47bee6468de7e1221b4d006342ad6b9813d0e5/testing/web-platform/tests/content- > security-policy/securitypolicyviolation/targeting.html > > I think there is no "correct" one, but which is the best for us to implement? Well, that is somehow confusing to me as well. Anne, is that only wording or is "the protected resource's Document' something different then 'global object' in regards of CSP? I would imagine firing the event using our internal used 'mLoadingContext' which we can QI into a document is the right context, but I am also not entirely sure. Anne, we are missing something? Or does the spec need a little tuning to clarify.

1) https://w3c.github.io/webappsec-csp/ is the correct URL and seems to define this with the appropriate amount of detail. W3C URLs with /TR/ in them are generally best avoided. 2) A Document object is different from the global object (a global object is Window, DedicatedWorkerGlobalScope, etc.). It seems like we want a Document object here as the target when the global object is a Window object per the algorithm in the specification. 3) https://github.com/w3c/webappsec-csp/issues/242 might change the level of detail we expose in these events, but given that Chrome doesn't seem to deviate from the specification as it stands today, we probably shouldn't either. It might be worth making sure this is tested though in web-platform-tests.

Comment on attachment 8920052 [details] Bug 1037335 - Fix test failures. https://reviewboard.mozilla.org/r/191044/#review201426

Comment on attachment 8920053 [details] Bug 1037335 - Implement security policy violation event. https://reviewboard.mozilla.org/r/191046/#review203868 Other than my nits, this already looks pretty good. r- only because of the type conversion of the status code. I would like to see it one more time before we can flag a dom peer for review (most likely smaug). Have you run that through TRY? Are there more web-platform tests than what I've already reviewed, or just those? My gut feeling tells me we should pass more tests. ::: dom/security/nsCSPContext.cpp:844 (Diff revision 4) > - nsAString& aViolatedDirective, > + nsAString& aViolatedDirective, > - uint32_t aViolatedPolicyIndex, > + uint32_t aViolatedPolicyIndex, > - nsAString& aSourceFile, > + nsAString& aSourceFile, > - nsAString& aScriptSample, > + nsAString& aScriptSample, > - uint32_t aLineNum) > + uint32_t aLineNum, > + mozilla::dom::SecurityPolicyViolationEventInit& aInit) nit: call this aViolationEventInit or aViolationInit to be more explicit about it. ::: dom/security/nsCSPContext.cpp:892 (Diff revision 4) > rv = this->GetPolicyString(aViolatedPolicyIndex, originalPolicy); > NS_ENSURE_SUCCESS(rv, rv); > - report.mCsp_report.mOriginal_policy = originalPolicy; > + aInit.mOriginalPolicy = originalPolicy; > > - // referrer > - if (!mReferrer.IsEmpty()) { > + // source-file > + [&aSourceFile] () { why do you prefer [&aSourceFile]() ? Please rewrite. ::: dom/security/nsCSPContext.cpp:935 (Diff revision 4) > + nsresult rv = channel->GetResponseStatus(&statusCode); > + if (NS_FAILED(rv)) { > + return 0; > + } > + > + return static_cast<uint16_t>(statusCode); What do we gain by having this assignment being a C++ lambda? I would prefer if you could rewrite that piece of code unless you have a strong reason this needs to be a lambda. More importantly, you are casting a uint_32 to uint_16 with no checks, we should fix that. ::: dom/security/nsCSPContext.cpp:942 (Diff revision 4) > + > + // line-number > + aInit.mLineNumber = aLineNum; > + > + // column-number > + aInit.mColumnNumber = 0; // TODO extend the todo with a comment what needs to happen please and move it above the assignment.

Comment on attachment 8928904 [details] Bug 1037335 - Fix timing issue of web-platform test: content-security-policy/style-src/style-src-imported-style-blocked.html. https://reviewboard.mozilla.org/r/200232/#review205702

Comment on attachment 8920053 [details] Bug 1037335 - Implement security policy violation event. https://reviewboard.mozilla.org/r/191046/#review205712 Great, so this looks good. Please address my nits, flag smaug for review and also file follow up bugs as discussed (blocking this bug). ::: dom/security/nsCSPContext.cpp:918 (Diff revision 5) > + aViolationEventInit.mStatusCode = 0; > + nsCOMPtr<nsIDocument> doc = do_QueryReferent(mLoadingContext); > + if (doc) { > + nsCOMPtr<nsIHttpChannel> channel = do_QueryInterface(doc->GetChannel()); > + if (channel) { > + uint32_t statusCode = 0; nit: just restructure that whole thing a little bit. Something like: uint32_t statusCode = 0; { whatever funky busuiness to query real statuscode } aViolationEventInit.mStatusCode = statusCode; ::: dom/security/nsCSPContext.cpp:921 (Diff revision 5) > + nsCOMPtr<nsIHttpChannel> channel = do_QueryInterface(doc->GetChannel()); > + if (channel) { > + uint32_t statusCode = 0; > + nsresult rv = channel->GetResponseStatus(&statusCode); > + if (NS_SUCCEEDED(rv) && > + // A simple validation of HTTP status code. Please use something like (statusCode <= UINT16_MAX) to be more explicit about what we are doing here. ::: dom/security/nsCSPContext.cpp:932 (Diff revision 5) > + > + // line-number > + aViolationEventInit.mLineNumber = aLineNum; > + > + // column-number > + // TODO We are not calcultaing the column number now, and wpt only assert it to be 0 currently. nit: slightly rephrase that comment to // TODO: Set correct column number and please file a follow up to write a web platform test testing the correct column number. ::: dom/security/nsCSPContext.cpp:935 (Diff revision 5) > + > + // column-number > + // TODO We are not calcultaing the column number now, and wpt only assert it to be 0 currently. > + aViolationEventInit.mColumnNumber = 0; > + > + aViolationEventInit.mBubbles = true; I think we also need to set |composed = true;| here - see https://w3c.github.io/webappsec-csp/#violation-events

Hi smaug, the CSP violation event implementation has some changes in webidl, and it breaks some mochitest for DOM interfaces so I also modified the tests. Could you please take a look at the patches? Thanks!

Comment on attachment 8920053 [details] Bug 1037335 - Implement security policy violation event. https://reviewboard.mozilla.org/r/191046/#review205982 ::: dom/security/nsCSPContext.cpp:1126 (Diff revision 6) > } > return NS_OK; > } > > +nsresult > +nsCSPContext::FireViolationEvent( Please use 2 spaces for indentation ::: dom/security/nsCSPContext.cpp:1134 (Diff revision 6) > + nsCOMPtr<nsIDocument> doc = do_QueryReferent(mLoadingContext); > + if (!doc) { > + return NS_OK; > + } > + > + RefPtr<mozilla::dom::Event> event = You should mark this event trusted ::: dom/security/nsCSPContext.cpp:1141 (Diff revision 6) > + doc, > + NS_LITERAL_STRING("securitypolicyviolation"), > + aViolationEventInit); > + > + bool rv; > + return doc->DispatchEvent(event, &rv); So we fire events always to document, never to elements. Why? Based on some spec bugs, sounds like Chrome has similar issues. This is a bit odd. Browsers not following the spec? ::: dom/webidl/SecurityPolicyViolationEvent.webidl:27 (Diff revision 6) > + readonly attribute unsigned short statusCode; > + readonly attribute long lineNumber; > + readonly attribute long columnNumber; > +}; > + > +dictionary SecurityPolicyViolationEventInit : EventInit Ok, so the spec doesn't have this kind of dictionary. It doesn't have default values. That looks like a spec issue https://github.com/w3c/webappsec-csp/issues/265 I assume it will use default values and not required values.

Comment on attachment 8920052 [details] Bug 1037335 - Fix test failures. https://reviewboard.mozilla.org/r/191044/#review205994

Looks like there are some more spec issues, so perhaps better to get them fixed first and then update the patch and ask re-review.

Happy to see y'all putting together an implementation, and equally happy to work through spec issues y'all have uncovered. CCing Andy, who's doing most of Chrome's CSP work these days. Would y'all mind summarizing the above discussion? What issues are you facing, and how would you like to see the spec change to help resolve them?

I think Olli was referring to 265 through 268 on https://github.com/w3c/webappsec-csp/issues. 270 also seems relevant.

Given the situation, what we should do is, add a pref which is enabled within Nightly and Early Beta, so we can improve test coverage for web platform tests. Then we can work out remaining spec issues and then we can enable violation events once everything is resolved. CS, can you please get that pref ready and flag me for review?

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #52) > CS, can you please get that pref ready and flag me for review? Additional note: the pref needs to check both, the webidl interface and the place where we dispatch events.

I added a pref named "security.security_policy_violation_event.enabled", which is true by default within NIGHTLY_BUILD and EARLY_BETA_OR_EARLIER. Please take a look. Besides, with this patch, it seems that we should revert the patches that fix wpt failures, while mochitest is somehow not affected. Here is the try link https://treeherder.mozilla.org/#/jobs?repo=try&revision=d4e5cdff43fb84fe47a4e2e98be003797ea63836

Comment on attachment 8930808 [details] Bug 1037335 - Add a pref to enable only within Nightly and Early Beta. https://reviewboard.mozilla.org/r/201900/#review207294 ::: dom/security/nsCSPContext.cpp:1131 (Diff revision 1) > nsCSPContext::FireViolationEvent( > const mozilla::dom::SecurityPolicyViolationEventInit& aViolationEventInit) > { > + if (!Preferences::GetBool("security.security_policy_violation_event.enabled", false)) { > + return NS_OK; > + } please load that into a static variable and then just check the static instead of having to query the expensive pref all the time. You can look e.g. for |bool CSPService::sCSPEnabled = true;| then you see how we do it elsewhere. ::: modules/libpref/init/all.js:5771 (Diff revision 1) > #endif > pref("layers.omtp.release-capture-on-main-thread", false); > + > +#if defined(NIGHTLY_BUILD) || defined(EARLY_BETA_OR_EARLIER) > +pref("security.security_policy_violation_event.enabled", true); > +#endif Please rename that pref to pref("security.csp.enable_violation_events and move it right under the other CSP prefs, just look for: pref("security.csp. and you just need to use #ifdef EARLY_BETA_OR_EARLIER

(In reply to Chung-Sheng Fu [:cfu] from comment #60) > Besides, with this patch, it seems that we should revert the patches that > fix wpt failures, while mochitest is somehow not affected. Here is the try > link > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=d4e5cdff43fb84fe47a4e2e98be003797ea63836 For the mochitest we added we have to explicitly flip the pref to true, just grep for " SpecialPowers.pushPrefEnv({" and you will see how we do that elsewhere. Similar for the failing wpt tests, in the *.ini you can explicitly flip the pref to true. E.g. see here: https://dxr.mozilla.org/mozilla-central/source/testing/web-platform/meta/html/browsers/origin/origin-of-data-document.html.ini#3

Sorry I didn't read the try result carefully. The failures were caused by some recently added tests, which didn't expect the events to be fired, from bug 1419296. So I fixed them in the existing patch "Fix test failures." (attachment 8921310 [details]), please take a look again if necessary. But anyway, I still enabled security.csp.enable_violation_events in those tests that listen to securitypolicyviolation event, in order to ensure they are running under a correct environment. Please take a look again at the patch "Add a pref to enable only within Nightly and Early Beta." (attachment 8930808 [details]), thanks. Here is the latest try link for your reference https://treeherder.mozilla.org/#/jobs?repo=try&revision=43040564d8745177ffdef5fabf3104d95dac35d8

Comment on attachment 8930808 [details] Bug 1037335 - Add a pref to enable only within Nightly and Early Beta. https://reviewboard.mozilla.org/r/201900/#review208440 Please fix my nit and then we can get that landed - thanks! ::: modules/libpref/init/all.js:2528 (Diff revision 2) > pref("security.csp.enable", true); > pref("security.csp.experimentalEnabled", false); > pref("security.csp.enableStrictDynamic", true); > +#ifdef EARLY_BETA_OR_EARLIER > +pref("security.csp.enable_violation_events", true); > +#endif please add an else where you explicitly set the pref to false please.

Some of these patches are still set to r? and cannot be landed for that reason.

I encountered some technical problems of Review Board, so I'm going to obsolete existing patches and try to land them again through attachment. Sorry for inconvenience.

Please update the patches, there is a conflict when applying the patch. applying Bug-1037335---Fix-test-failures-rckerschbsmaug.patch applying Bug-1037335---Fix-test-failures-rckerschb.patch patching file testing/web-platform/tests/content-security-policy/reporting/securitypolicyviolation-idl.html Hunk #1 FAILED at 88 1 out of 1 hunks FAILED -- saving rejects to file testing/web-platform/tests/content-security-policy/reporting/securitypolicyviolation-idl.html.rej patch failed, unable to continue (try -v) patch failed, rejects left in working directory errors during apply, please fix and qrefresh Bug-1037335---Fix-test-failures-rckerschb.patch

Fixed conflict and new failures due to bug 1421370.

remote: WebIDL file dom/webidl/SecurityPolicyViolationEvent.webidl altered in changeset 17b3ddbc8027 without DOM peer review remote: remote: remote: remote: ************************** ERROR **************************** remote: remote: Changes to WebIDL files in this repo require review from a DOM peer in the form of r=... remote: This is to ensure that we behave responsibly with exposing new Web APIs. We appreciate your understanding.. remote: remote: ************************************************************* Couldn't push it for missing DOM peer review.

Won't test_interfaces.js fail later when this stuff is in late_beta/release?

(In reply to Olli Pettay [:smaug] from comment #100) > Won't test_interfaces.js fail later when this stuff is in late_beta/release? I thought we explicitly pushed the pref for all mochitests? At least I suggested that in comment 62.

I don't see pref being enabled for that, and pref shouldn't be explicitly enabled for that test, since it is about the interfaces we expose in the global scope. Do we just need release: false there or something.

Pushed by ebalazs@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/48ba62a0feb0 Implement security policy violation event. r=ckerschb,smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/207b671db6ec Add a mochitest for security policy violation event. r=ckerschb https://hg.mozilla.org/integration/mozilla-inbound/rev/0f09a387cbc4 Fix test failures. r=ckerschb,smaug https://hg.mozilla.org/integration/mozilla-inbound/rev/7678ac4b15ad Fix test failures. r=ckerschb https://hg.mozilla.org/integration/mozilla-inbound/rev/fd99633486a5 Fix timing issue of web-platform test: content-security-policy/style-src/style-src-imported-style-blocked.html. r=ckerschb https://hg.mozilla.org/integration/mozilla-inbound/rev/3f2598dad67c Add a pref to enable only within Nightly and Early Beta. r=ckerschb,smaug

Right now, with these patches as-landed, I would expect test_interfaces to fail once we get to late beta (after the EARLY_BETA_OR_EARLIER ifdef gets manually flipped midway through the cycle). Unfortunately, we don't have that ifdef exposed anywhere in an easily-available way to test_interfaces. For the short-term, I'd recommend making this Nightly-only instead and adding the appropriate annotations to test_interfaces. Long-term, I would suggest getting EARLY_BETA_OR_EARLIER added to AppConstants.jsm and then moving bug 1313741 forward to make use of that in the test.

https://hg.mozilla.org/mozilla-central/rev/48ba62a0feb0 https://hg.mozilla.org/mozilla-central/rev/207b671db6ec https://hg.mozilla.org/mozilla-central/rev/0f09a387cbc4 https://hg.mozilla.org/mozilla-central/rev/7678ac4b15ad https://hg.mozilla.org/mozilla-central/rev/fd99633486a5 https://hg.mozilla.org/mozilla-central/rev/3f2598dad67c

I didn't set the pref in test_interfaces because it seemed not appropriate to me. Sorry for not pointing this out and confusing you. I will handle it as soon as possible if there is any failure caused by change of release phase.

Hi there, I have had a go at documenting this; I've created a page for the interface itself, and all the properties plus the constructor. https://developer.mozilla.org/en-US/docs/Web/API/SecurityPolicyViolationEvent I have also added an entry to our experimental features page: https://developer.mozilla.org/en-US/Firefox/Experimental_features#Security I had a couple of questions: 1. Does what I have done so far look correct? 2. What should I include for an example to show how to use this? I don't really understand how you would use this at all, and can't find any examples. Thanks!

(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #107) Thanks for putting this together! > 1. Does what I have done so far look correct? Please note that Security Policy Violation events are enabled in Nightly and Early Beta but not in Release. The Bug to change that is Bug 1432523 (but we have to clear the dependencies for this bug first). > 2. What should I include for an example to show how to use this? I don't > really understand how you would use this at all, and can't find any examples. Here is a good example. https://bugzilla.mozilla.org/attachment.cgi?id=8933131&action=diff The idea is that one could implement an event listener for CSP violations and based on that filter out some errors (e.g. caused by extensions) and only do custom reporting to someones web server instead of using CSPs built in report-uri mechanism.