Closed
Bug 885433
Opened 11 years ago
Closed 11 years ago
CSP 1.0 should not block inline scripts or eval unless script-src or default-src are included
Categories
(Core :: Security, defect)
Tracking
()
VERIFIED
FIXED
mozilla25
People
(Reporter: dveditz, Assigned: grobinson)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 5 obsolete files)
9.38 KB,
patch
|
grobinson
:
review+
bajaj
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Between the Mozilla proposal and the CSP 1.0 spec the rules for when inline-scripts and eval are blocked changed. In the final spec site authors opt-in to the stricter regime NOT merely by using CSP but by opting into "script" restrictions specifically. This is done explicitly by including script-src or implicitly by using default-src
The same applies to inline style restrictions, which should not be enforced unless there's a style-src or default-src directive.
A policy of
CSP: sandbox; frame-options mysite.com yoursite.com;
should not block any inline scripts or styles, and should allow eval
Updated•11 years ago
|
Summary: CSP should not block inline scripts or evail unless script-src or default-src are included → CSP should not block inline scripts or eval unless script-src or default-src are included
Comment 1•11 years ago
|
||
Bug 886132 might be an instance of this for inline styles. It looks like the policy has a script-src only and not default-src or style-src, but inline styles are being blocked per the messages in the web console. A fix for this would be to initialize allowsInlineScripts/Styles/Eval to true and flip it the appropriate inline script/style flag to false if a script-src/style-src directive is seen, flipping both to false if default-src is explicitly seen (not inferred).
Assignee | ||
Comment 2•11 years ago
|
||
Mochitest with two different CSP's: one that does not mention default-src, script-src, or style-src (and so does not trigger unsafe-inline or unsafe-eval protections) and one that is just default-src 'self' (which should trigger unsafe-inline protection for scripts and stylesheets and unsafe-eval protection for scripts).
Right now three tests fail, representing the original issue created by the bug, and three tests pass, representing the blocking that is now applied by default whenever CSP is used but should only be applied if default-src, script-src, or style-src are used.
I note that while writing this test, I had difficulty with 'unsafe-eval'. In my experience, the eval worked if I did not provide a CSP but would not work with a CSP, even if I set "script-src 'unsafe-inline' 'unsafe-eval';". This may be a separate bug, and if so it also implies problems with the evalscript mochitest.
Assignee: nobody → grobinson
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•11 years ago
|
||
Fixed Bug 887974, which was the cause of the 'unsafe-eval' weirdness mentioned above.
Depends on: 887974
Assignee | ||
Comment 4•11 years ago
|
||
First pass at implementing the correct behavior of "opting-in" to default blocking of inline and eval. Not the most elegant but it gets the job done (otherwise would need to refactor that whole function). CSP test suite passes.
Attachment #768091 -
Attachment is obsolete: true
Attachment #768738 -
Flags: feedback?(sstamm)
Attachment #768738 -
Flags: feedback?(imelven)
Assignee | ||
Comment 5•11 years ago
|
||
Nicer code. Passes the CSP test suite. I also checked the problematic URL from Bug 886132 and was not able to reproduce the problems with Disqus.
Attachment #768738 -
Attachment is obsolete: true
Attachment #768738 -
Flags: feedback?(sstamm)
Attachment #768738 -
Flags: feedback?(imelven)
Attachment #768986 -
Flags: feedback?(sstamm)
Attachment #768986 -
Flags: feedback?(imelven)
Comment 7•11 years ago
|
||
Comment on attachment 768986 [details] [diff] [review]
Patch 2
Review of attachment 768986 [details] [diff] [review]:
-----------------------------------------------------------------
A few small things, but pretty much there. We talked yesterday about needing to loop twice,
I'm not too worried about it for now because as we all know, we want to rewrite this in C++ some day...
Good tests !
::: content/base/src/CSPUtils.jsm
@@ +528,5 @@
> + dirs[dirname] = dirvalue;
> + }
> +
> + /* Spec compliant policies have different default behavior for inline
> + * scripts, styles, and eval. Bug 885433 */
the usual style seems to be to have the closing */ on its own line
Good comment, especially referencing the bug #
@@ +535,5 @@
> + aCSPR._allowInlineStyles = true;
> +
> + /* In CSP 1.0, you need to opt-in to blocking inline scripts and eval by
> + * specifying either default-src or script-src, and to blocking inline
> + * styles by specifying either default-src or style-src. */
same as above, closing */ on its own line
::: content/base/test/Makefile.in
@@ +634,5 @@
> + test_CSP_bug885433.html \
> + file_CSP_bug885433_allows.html \
> + file_CSP_bug885433_allows.html^headers^ \
> + file_CSP_bug885433_blocks.html \
> + file_CSP_bug885433_blocks.html^headers^ \
these don't look indented the same as the rest of the file
::: content/base/test/file_CSP_bug885433_allows.html
@@ +19,5 @@
> +
> + // Use eval to set a style attribute
> + try {
> + eval('document.getElementById("unsafe-eval-script-allowed").style.color = "green";');
> + } catch (e) {}
do you need the try/catch ? this is a failing test if the eval is blocked anyways..
::: content/base/test/file_CSP_bug885433_blocks.html
@@ +3,5 @@
> +The Content-Security-Policy header for this file does not include any of the
> +default-src, script-src, or style-src directives. It should not use the old
> +default behavior of blocking unsafe-inline and unsafe-eval on scripts, and
> +unsafe-inline on styles, because no directives related to scripts or styles
> +are specified. -->
this comment is not true ;) It needs to be reversed, the header
does include default-src, everything should be blocked etc.
::: content/base/test/test_CSP_bug885433.html
@@ +1,4 @@
> +<!DOCTYPE HTML>
> +<html>
> +<head>
> + <title>Test for Content Security Policy inline stylesheets stuff</title>
I'd like it if the title was more explicit about what's being tested :)
@@ +50,5 @@
> + SimpleTest.finish();
> +}
> +
> +SpecialPowers.pushPrefEnv(
> + {'set':[["security.csp.speccompliant", true]]},
do we need to set this any more ? The pref is flipped in Fx23 and later..
Attachment #768986 -
Flags: feedback?(imelven) → feedback+
Comment 8•11 years ago
|
||
Comment on attachment 768986 [details] [diff] [review]
Patch 2
Review of attachment 768986 [details] [diff] [review]:
-----------------------------------------------------------------
looks pretty good to me.
::: content/base/src/CSPUtils.jsm
@@ +528,5 @@
> + dirs[dirname] = dirvalue;
> + }
> +
> + /* Spec compliant policies have different default behavior for inline
> + * scripts, styles, and eval. Bug 885433 */
super-Nit: comments inside functions should really be line comments, not block comments.
::: content/base/test/file_CSP_bug885433_allows.html
@@ +19,5 @@
> +
> + // Use eval to set a style attribute
> + try {
> + eval('document.getElementById("unsafe-eval-script-allowed").style.color = "green";');
> + } catch (e) {}
I think eval throws a security exception if CSP blocks it. The try/catch can help us detect and recover from it if there are other tests in this document. (This is probably more precise if it doesn't just choke and stop generating results.) Can you make the failure case more explicit by handling the exception or providing a comment that explains what a thrown exception means?
::: content/base/test/file_CSP_bug885433_blocks.html
@@ +3,5 @@
> +The Content-Security-Policy header for this file does not include any of the
> +default-src, script-src, or style-src directives. It should not use the old
> +default behavior of blocking unsafe-inline and unsafe-eval on scripts, and
> +unsafe-inline on styles, because no directives related to scripts or styles
> +are specified. -->
I recommend pasting a duplicate of the ^headers^ in a comment here and briefly explain what is being tested and what should succeed. This way it's easy to see the test case and the expected results all in one spot.
Attachment #768986 -
Flags: feedback?(sstamm) → feedback+
Assignee | ||
Comment 9•11 years ago
|
||
(In reply to Sid Stamm [:geekboy or :sstamm] from comment #8)
> Comment on attachment 768986 [details] [diff] [review]
> Patch 2
>
> Review of attachment 768986 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> looks pretty good to me.
>
> ::: content/base/src/CSPUtils.jsm
> @@ +528,5 @@
> > + dirs[dirname] = dirvalue;
> > + }
> > +
> > + /* Spec compliant policies have different default behavior for inline
> > + * scripts, styles, and eval. Bug 885433 */
>
> super-Nit: comments inside functions should really be line comments, not
> block comments.
>
Just curious - why?
> ::: content/base/test/file_CSP_bug885433_allows.html
> @@ +19,5 @@
> > +
> > + // Use eval to set a style attribute
> > + try {
> > + eval('document.getElementById("unsafe-eval-script-allowed").style.color = "green";');
> > + } catch (e) {}
>
> I think eval throws a security exception if CSP blocks it. The try/catch
> can help us detect and recover from it if there are other tests in this
> document. (This is probably more precise if it doesn't just choke and stop
> generating results.) Can you make the failure case more explicit by
> handling the exception or providing a comment that explains what a thrown
> exception means?
>
That was the motivation for the try/catch. I'll comment it and have it trigger a test failure.
> ::: content/base/test/file_CSP_bug885433_blocks.html
> @@ +3,5 @@
> > +The Content-Security-Policy header for this file does not include any of the
> > +default-src, script-src, or style-src directives. It should not use the old
> > +default behavior of blocking unsafe-inline and unsafe-eval on scripts, and
> > +unsafe-inline on styles, because no directives related to scripts or styles
> > +are specified. -->
>
> I recommend pasting a duplicate of the ^headers^ in a comment here and
> briefly explain what is being tested and what should succeed. This way it's
> easy to see the test case and the expected results all in one spot.
Good idea!
Comment 10•11 years ago
|
||
(In reply to Garrett Robinson [:grobinson] from comment #9)
> Just curious - why?
For consistent style with the rest of the file. Doesn't matter a whole lot, but I find it easier to manage line comments in general, or at the very least, a consistent style.
e.g.:
http://mxr.mozilla.org/mozilla-central/source/content/base/src/CSPUtils.jsm#307
http://mxr.mozilla.org/mozilla-central/source/content/base/src/CSPUtils.jsm#572
http://mxr.mozilla.org/mozilla-central/source/content/base/src/CSPUtils.jsm#792
Assignee | ||
Comment 11•11 years ago
|
||
(In reply to Ian Melven :imelven from comment #7)
> Comment on attachment 768986 [details] [diff] [review]
> Patch 2
>
> Review of attachment 768986 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> A few small things, but pretty much there. We talked yesterday about needing
> to loop twice,
> I'm not too worried about it for now because as we all know, we want to
> rewrite this in C++ some day...
>
I think this rewrite actually improves that situation. Now there is one loop that parses the directives into an object, which we can then simply query for the special-case directives rather than doing a loop-and-compare. More efficient and more readable.
> Good tests !
>
> ::: content/base/src/CSPUtils.jsm
> @@ +528,5 @@
> > + dirs[dirname] = dirvalue;
> > + }
> > +
> > + /* Spec compliant policies have different default behavior for inline
> > + * scripts, styles, and eval. Bug 885433 */
>
> the usual style seems to be to have the closing */ on its own line
>
> Good comment, especially referencing the bug #
>
> @@ +535,5 @@
> > + aCSPR._allowInlineStyles = true;
> > +
> > + /* In CSP 1.0, you need to opt-in to blocking inline scripts and eval by
> > + * specifying either default-src or script-src, and to blocking inline
> > + * styles by specifying either default-src or style-src. */
>
> same as above, closing */ on its own line
>
> ::: content/base/test/Makefile.in
> @@ +634,5 @@
> > + test_CSP_bug885433.html \
> > + file_CSP_bug885433_allows.html \
> > + file_CSP_bug885433_allows.html^headers^ \
> > + file_CSP_bug885433_blocks.html \
> > + file_CSP_bug885433_blocks.html^headers^ \
>
> these don't look indented the same as the rest of the file
>
Ah, my editor inserts spaces by default and the convention in Makefiles appears to be tabs. I know that is a requirement for GNU Make, doesn't appear to be a requirement for whatever we're using. I'll make it conform to the rest of the file.
> ::: content/base/test/file_CSP_bug885433_allows.html
> @@ +19,5 @@
> > +
> > + // Use eval to set a style attribute
> > + try {
> > + eval('document.getElementById("unsafe-eval-script-allowed").style.color = "green";');
> > + } catch (e) {}
>
> do you need the try/catch ? this is a failing test if the eval is blocked
> anyways..
>
Yes, otherwise eval throws a security exception which would derail the rest of the tests.
> ::: content/base/test/file_CSP_bug885433_blocks.html
> @@ +3,5 @@
> > +The Content-Security-Policy header for this file does not include any of the
> > +default-src, script-src, or style-src directives. It should not use the old
> > +default behavior of blocking unsafe-inline and unsafe-eval on scripts, and
> > +unsafe-inline on styles, because no directives related to scripts or styles
> > +are specified. -->
>
> this comment is not true ;) It needs to be reversed, the header
> does include default-src, everything should be blocked etc.
>
Thanks for the catch - copy/paste forgetfulness.
> ::: content/base/test/test_CSP_bug885433.html
> @@ +1,4 @@
> > +<!DOCTYPE HTML>
> > +<html>
> > +<head>
> > + <title>Test for Content Security Policy inline stylesheets stuff</title>
>
> I'd like it if the title was more explicit about what's being tested :)
>
> @@ +50,5 @@
> > + SimpleTest.finish();
> > +}
> > +
> > +SpecialPowers.pushPrefEnv(
> > + {'set':[["security.csp.speccompliant", true]]},
>
> do we need to set this any more ? The pref is flipped in Fx23 and later..
Good point! I'll remove it.
Assignee | ||
Comment 12•11 years ago
|
||
Incorporates imelven and sstamm's feedback. Specific changes and rationale are described in the previous comments.
Attachment #768986 -
Attachment is obsolete: true
Attachment #769221 -
Flags: review?(sstamm)
Attachment #769221 -
Flags: review?(imelven)
Assignee | ||
Comment 13•11 years ago
|
||
Forgot to fix the title of the bug per imelven's review.
Attachment #769221 -
Attachment is obsolete: true
Attachment #769221 -
Flags: review?(sstamm)
Attachment #769221 -
Flags: review?(imelven)
Attachment #769229 -
Flags: review?(sstamm)
Attachment #769229 -
Flags: review?(imelven)
Comment 14•11 years ago
|
||
Comment on attachment 769229 [details] [diff] [review]
Patch 4
Review of attachment 769229 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good to me :D
Attachment #769229 -
Flags: review?(imelven) → review+
Comment 17•11 years ago
|
||
Given the breakage in current Aurora (24.0a2), this should also land there.
Comment 18•11 years ago
|
||
From what I hear, 23 should be unaffected by this (correct me if I'm wrong), but both 24 and 25 are, and as the bug blocks the very popular Disqus system from working correctly, I think we should track this for both affected trains.
status-firefox23:
--- → unaffected
status-firefox24:
--- → affected
status-firefox25:
--- → affected
tracking-firefox24:
--- → ?
tracking-firefox25:
--- → ?
Comment 19•11 years ago
|
||
B2G VM (arm) failed on the try run with the tests in the patch :
19:40:51 INFO - 40 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_CSP_bug885433.html | Inline script should be blocked
19:40:51 INFO - 41 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_CSP_bug885433.html | Eval should be blocked
19:40:51 INFO - 42 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_CSP_bug885433.html | Inline style should be blocked
20:20:44 INFO - I/GeckoDump( 814): 40 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_CSP_bug885433.html | Inline script should be blocked
20:20:44 INFO - I/GeckoDump( 814): 41 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_CSP_bug885433.html | Eval should be blocked
20:20:44 INFO - I/GeckoDump( 814): 42 ERROR TEST-UNEXPECTED-FAIL | /tests/content/base/test/test_CSP_bug885433.html | Inline style should be blocked
Assignee | ||
Comment 20•11 years ago
|
||
(In reply to Ian Melven :imelven from comment #19)
> B2G VM (arm) failed on the try run with the tests in the patch :
>
> 19:40:51 INFO - 40 ERROR TEST-UNEXPECTED-FAIL |
> /tests/content/base/test/test_CSP_bug885433.html | Inline script should be
> blocked
> 19:40:51 INFO - 41 ERROR TEST-UNEXPECTED-FAIL |
> /tests/content/base/test/test_CSP_bug885433.html | Eval should be blocked
> 19:40:51 INFO - 42 ERROR TEST-UNEXPECTED-FAIL |
> /tests/content/base/test/test_CSP_bug885433.html | Inline style should be
> blocked
> 20:20:44 INFO - I/GeckoDump( 814): 40 ERROR TEST-UNEXPECTED-FAIL |
> /tests/content/base/test/test_CSP_bug885433.html | Inline script should be
> blocked
> 20:20:44 INFO - I/GeckoDump( 814): 41 ERROR TEST-UNEXPECTED-FAIL |
> /tests/content/base/test/test_CSP_bug885433.html | Eval should be blocked
> 20:20:44 INFO - I/GeckoDump( 814): 42 ERROR TEST-UNEXPECTED-FAIL |
> /tests/content/base/test/test_CSP_bug885433.html | Inline style should be
> blocked
This is because this fix is only for CSP 1.0, which is not on by default in B2G (Bug 858787). The test originally used SpecialPowers.pushPrefEnv to flip security.csp.speccompliant for this test, but as you pointed out in Comment 7 this should no longer be necessary because the pref is on by default in >= Fx23.
I changed the title of the bug to clarify that this solely affects CSP 1.0.
Our options here are
1. Restore the use of pushPrefEnv to flip the spec and land this ASAP, or
2. Wait until Bug 858787 lands
Since this patches fixes two other bugs (at the moment), I prefer the first option. We should land it as soon as possible to fix desktop Firefox. I also think it is logical because the bug is only for CSP 1.0, to explicitly set that pref even if it is now on by default.
Summary: CSP should not block inline scripts or eval unless script-src or default-src are included → CSP 1.0 should not block inline scripts or eval unless script-src or default-src are included
Comment 21•11 years ago
|
||
(In reply to Garrett Robinson [:grobinson] from comment #20)
> (In reply to Ian Melven :imelven from comment #19)
> > B2G VM (arm) failed on the try run with the tests in the patch :
> >
> > 19:40:51 INFO - 40 ERROR TEST-UNEXPECTED-FAIL |
> > /tests/content/base/test/test_CSP_bug885433.html | Inline script should be
> > blocked
> > 19:40:51 INFO - 41 ERROR TEST-UNEXPECTED-FAIL |
> > /tests/content/base/test/test_CSP_bug885433.html | Eval should be blocked
> > 19:40:51 INFO - 42 ERROR TEST-UNEXPECTED-FAIL |
> > /tests/content/base/test/test_CSP_bug885433.html | Inline style should be
> > blocked
> > 20:20:44 INFO - I/GeckoDump( 814): 40 ERROR TEST-UNEXPECTED-FAIL |
> > /tests/content/base/test/test_CSP_bug885433.html | Inline script should be
> > blocked
> > 20:20:44 INFO - I/GeckoDump( 814): 41 ERROR TEST-UNEXPECTED-FAIL |
> > /tests/content/base/test/test_CSP_bug885433.html | Eval should be blocked
> > 20:20:44 INFO - I/GeckoDump( 814): 42 ERROR TEST-UNEXPECTED-FAIL |
> > /tests/content/base/test/test_CSP_bug885433.html | Inline style should be
> > blocked
>
> This is because this fix is only for CSP 1.0, which is not on by default in
> B2G (Bug 858787). The test originally used SpecialPowers.pushPrefEnv to flip
> security.csp.speccompliant for this test, but as you pointed out in Comment
> 7 this should no longer be necessary because the pref is on by default in >=
> Fx23.
Ah yes, but it's only on by default for desktop at the moment. Thanks for reminding me.
> I changed the title of the bug to clarify that this solely affects CSP 1.0.
>
> Our options here are
>
> 1. Restore the use of pushPrefEnv to flip the spec and land this ASAP, or
> 2. Wait until Bug 858787 lands
>
> Since this patches fixes two other bugs (at the moment), I prefer the first
> option. We should land it as soon as possible to fix desktop Firefox. I also
> think it is logical because the bug is only for CSP 1.0, to explicitly set
> that pref even if it is now on by default.
I prefer #1 as well, and totally agree that we should get this fixed for desktop as soon as possible. We can clean up the specCompliant pushPrefEnvs after we get CSP 1.0 on for Firefox OS, and I think your general strategy of fixing everything on desktop that we know about first and then coming back to B2G again is a very good one.
Updated•11 years ago
|
Assignee | ||
Comment 22•11 years ago
|
||
Reinstate running the test with the speccompliant pref. I confirmed that the Mochitest passes on B2G, which was the only failure in the last try run. Carrying over r+ from imelven.
Attachment #769229 -
Attachment is obsolete: true
Attachment #769229 -
Flags: review?(sstamm)
Attachment #769948 -
Flags: review+
Comment 23•11 years ago
|
||
Comment 24•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Comment 25•11 years ago
|
||
Comment on attachment 769948 [details] [diff] [review]
Patch 5
[Approval Request Comment]
Bug caused by (feature/regressing bug #): CSP 1.0 / bug 842657
User impact if declined: sites that expect CSP to behave per spec may break e.g. disqus
Testing completed (on m-c, etc.): just landed on m-c, but comes with a mochitest, and we have many other CSP tests that should catch regression.
Risk to taking this patch (and alternatives if risky): possible CSP regressions, but see above re user impact and testing
String or IDL/UUID changes made by this patch: none
This doesn't need to go to beta because the bug fix that exposed this inconsistency with the spec first landed in Fx24.
Attachment #769948 -
Flags: approval-mozilla-aurora?
Comment 26•11 years ago
|
||
Do we know if this has already landed in Nightly 25? The online commenting provider disqus says this bug is preventing nightly users from using their commenting system which is use by millions of sites.
Just wondering?
Comment 27•11 years ago
|
||
@Benjamin, it's already landed in Nightly. (In reply to Benjamin Kerensa from comment #26)
> Do we know if this has already landed in Nightly 25? The online commenting
> provider disqus says this bug is preventing nightly users from using their
> commenting system which is use by millions of sites.
>
> Just wondering?
Comment 28•11 years ago
|
||
(In reply to krzysztof.glebowicz from comment #27)
> @Benjamin, it's already landed in Nightly. (In reply to Benjamin Kerensa
> from comment #26)
> > Do we know if this has already landed in Nightly 25? The online commenting
> > provider disqus says this bug is preventing nightly users from using their
> > commenting system which is use by millions of sites.
> >
> > Just wondering?
Right, this should be in the 7/3 Nightly.
Comment 29•11 years ago
|
||
I just verified that disqus works for me on Windows with the 7/3 Fx25 Nightly using the test case from bug 886132 (http://edition.cnn.com/2013/06/22/politics/nsa-leaks/index.html)
Comment 30•11 years ago
|
||
Disqus is broken in the latest Aurora due to this bug. Is this something that will be uplifted into 24?
Comment 31•11 years ago
|
||
Waiting on approval for uplift (comment 25).
Comment 32•11 years ago
|
||
(In reply to Sid Stamm [:geekboy or :sstamm] from comment #31)
> Waiting on approval for uplift (comment 25).
Thanks! </tldr>
Comment 33•11 years ago
|
||
Comment on attachment 769948 [details] [diff] [review]
Patch 5
We definitely wan this fix and better sooner than later, approving for aurora.Based on your comment regarding possible fallouts, please add qawanted to get targeted qa testing that may be need here if you have any testcases in mind.
Attachment #769948 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 34•11 years ago
|
||
(In reply to bhavana bajaj [:bajaj] from comment #33)
> Comment on attachment 769948 [details] [diff] [review]
> Patch 5
>
> We definitely wan this fix and better sooner than later, approving for
> aurora.Based on your comment regarding possible fallouts, please add
> qawanted to get targeted qa testing that may be need here if you have any
> testcases in mind.
Thanks, I'll land it on aurora. Regarding test cases, the best thing to do would be to run through csptesting.herokuapp.com or the W3C test suite and hopefully see that our score improves :)
Comment 35•11 years ago
|
||
Updated•11 years ago
|
Comment 36•11 years ago
|
||
We actually need this on FF23 as well for bug 888172 to be able to land and have a spec compliant CSP 1.0 implementation so marking this affected on FF23 and we'll take this in our second-to-last Beta. I spoke with Garrett in IRC and the risk here is low due to this only touching CSP code and having tests for that area as well.
tracking-firefox23:
--- → +
Updated•11 years ago
|
Attachment #769948 -
Flags: approval-mozilla-beta+
Comment 37•11 years ago
|
||
Comment 38•11 years ago
|
||
Confirmed pre-1.0 behavior.
Verified 1.0 correct behavior in FF23, 24, 25.
Status: RESOLVED → VERIFIED
Comment 39•11 years ago
|
||
I'm still seeing this is 28.
Comment 40•11 years ago
|
||
(In reply to Benjamin Kerensa from comment #39)
> I'm still seeing this is 28.
Might have been a regression sometime between 25 and 28, there were definitely a few bugs in CSP very recently - can you file a new bug for this with steps to repro (assuming they are different from the ones above ?) Thanks !
Updated•11 years ago
|
Flags: needinfo?(bkerensa)
Comment 41•11 years ago
|
||
I'm checking on this with Disqus which still does not work on Nightly they previously said this bug was the reason.
Flags: needinfo?(bkerensa)
Comment 42•11 years ago
|
||
Did you hear anything from Disqus? Does the issue persist?
Flags: needinfo?(bkerensa)
Comment 43•11 years ago
|
||
(In reply to Florian Bender from comment #42)
> Did you hear anything from Disqus? Does the issue persist?
Nothing back from Disqus but the issues does not persist in Nightly (currently 32) which is what I run.
Flags: needinfo?(bkerensa)
You need to log in
before you can comment on or make changes to this bug.
Description
•