CSP report blocked-uri value should be an empty string when resource has no URL

RESOLVED FIXED in Firefox 61

Status

()

defect
P2
normal
RESOLVED FIXED
4 years ago
Last year

People

(Reporter: scotthelme, Assigned: jkt)

Tracking

(Blocks 1 bug)

43 Branch
mozilla61
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox61 fixed)

Details

(Whiteboard: [domsecurity-active])

Attachments

(1 attachment, 12 obsolete attachments)

Reporter

Description

4 years ago
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/47.0.2526.106 Safari/537.36

Steps to reproduce:

Issue a CSP that bans inline script.

Content-Security-Policy: default-src 'self'


Include an inline script on a page to cause a CSP violation.

<script>alert("Hello World!");</script>


The CSP report that is sent contains a blocked-uri value.

"blocked-uri":"self"

According to the specification the blocked-uri should be an "empty string if the resource has no URL (inline script and inline style, for example)". 

http://www.w3.org/TR/CSP2/#violation-report-blocked-uri


Actual results:

The CSP report that is sent contains a blocked-uri value.

"blocked-uri":"self"


Expected results:

The CSP report that is sent contains an empty blocked-uri.

"blocked-uri":""

Updated

4 years ago
Component: Untriaged → DOM: Security
Product: Firefox → Core
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [domsecurity-backlog]
Priority: -- → P2
Priority: P2 → P3
Whiteboard: [domsecurity-backlog] → [domsecurity-backlog2]

Comment 1

3 years ago
Hi! I would like to attempt to solve this bug.

I need to confirm few things. I was initially planning to use regular expressions to find urls.

What values can reportBlockedURI take? Is it a url, a url embedded in a string or an empty 
string?

Comment 2

3 years ago
Posted patch Please check this patch (obsolete) — Splinter Review
Please check the patch and tell me if its good enough
Attachment #8800745 - Flags: review?(ckerschb)
Comment on attachment 8800745 [details] [diff] [review]
Please check this patch

Review of attachment 8800745 [details] [diff] [review]:
-----------------------------------------------------------------

Ideally I suppose we could change | nsISupports* aBlockedContentSource | to |nsIURI* aBlockedURI|. I am not entirely certain what aBlockedContentSource might contain other than an nsIURI, potentially 'self' and some other strings. If it's only 'self' then my approach would work, but this needs verification of course.

If am correct however, we could eliminate that whole else [1] branch; only strip the URI for reporting if there is a URI and leave the empty string in case aBlockedURI is null.

[1] https://dxr.mozilla.org/mozilla-central/source/dom/security/nsCSPContext.cpp?q=nsCOMPtr%3CnsISupportsCString%3E+cstr+%3D+do_QueryInterface%28aBlockedContentSource%29&redirect_type=single#812
Attachment #8800745 - Flags: review?(ckerschb) → review-

Comment 4

3 years ago
Right, I have removed the else condition. However, I don't understand why checking if reportBlockedURI is a url is incorrect. It takes care of all cases except if its a url. I've attached the modified patch. Please review

Comment 5

3 years ago
Posted patch Modified Patch (obsolete) — Splinter Review
Attachment #8801514 - Flags: review?(ckerschb)

Comment 6

3 years ago
I'm sorry. I just realised that I don't have to make a function to check for url. 
This patch is the one you need.
Please review

Comment 7

3 years ago
Posted patch bug1236222.patch (obsolete) — Splinter Review
Please Review
Attachment #8800745 - Attachment is obsolete: true
Attachment #8801514 - Attachment is obsolete: true
Attachment #8801514 - Flags: review?(ckerschb)
Attachment #8801568 - Flags: review?(ckerschb)
Comment on attachment 8801568 [details] [diff] [review]
bug1236222.patch

Review of attachment 8801568 [details] [diff] [review]:
-----------------------------------------------------------------

Hey Tanuja, as suggested in comment 3 we should try to do the following:

1) Change the signature of the function from
nsCSPContext::SendReports(nsISupports* aBlockedContentSource,
to
nsCSPContext::SendReports(nsIURI* aBlockedURI,

2) in the cases where we used to pass a string as aBlockedContentSource we should pass nullptr (if possible)

3) We should replace that block

  // blocked-uri
  if (aBlockedContentSource) {
    nsAutoCString reportBlockedURI;
    nsCOMPtr<nsIURI> uri = do_QueryInterface(aBlockedContentSource);
    // could be a string or URI
    if (uri) {
      StripURIForReporting(uri, mSelfURI, reportBlockedURI);
    } else {
      nsCOMPtr<nsISupportsCString> cstr = do_QueryInterface(aBlockedContentSource);
      if (cstr) {
        cstr->GetData(reportBlockedURI);
      }
    }
    if (reportBlockedURI.IsEmpty()) {
      // this can happen for frame-ancestors violation where the violating
      // ancestor is cross-origin.
      NS_WARNING("No blocked URI (null aBlockedContentSource) for CSP violation report.");
    }
    report.mCsp_report.mBlocked_uri = NS_ConvertUTF8toUTF16(reportBlockedURI);
  }

with something like

if (aBlockedURI) {
  nsCOMPtr<nsIURI> uri = aBlockedURI;
  // could be a string or URI
  StripURIForReporting(uri, mSelfURI, reportBlockedURI);
  report.mCsp_report.mBlocked_uri = NS_ConvertUTF8toUTF16(reportBlockedURI);
}
Attachment #8801568 - Flags: review?(ckerschb)
Assignee: nobody → f2013658

Comment 9

3 years ago
Thanks Christoph, 
                 I did not Change the signature of the function from
                    nsCSPContext::SendReports(nsISupports* aBlockedContentSource,
                                  to
                    nsCSPContext::SendReports(nsIURI* aBlockedURI,

earlier, as the function is being called elsewhere (line 1061 [1]) and it will no longer be a part of the nsCSPContext class, and thus throws this error

/home/fay/mozilla-central/dom/security/nsCSPContext.cpp:783:1: error: prototype for ‘nsresult nsCSPContext::SendReports(nsIURI*, nsIURI*, nsAString_internal&, uint32_t, nsAString_internal&, nsAString_internal&, uint32_t)’ does not match any in class ‘nsCSPContext’
 0:05.14  nsCSPContext::SendReports(nsIURI* aBlockedURI,

The patch I submitted earlier takes a different approach to reporting only the URL. But I'll try to implement what you are suggesting. I'll have to modify multiple functions that call the SendReports function.
[1] https://dxr.mozilla.org/mozilla-central/source/dom/security/nsCSPContext.cpp?#1061

Comment 10

3 years ago
Posted patch bug1236222.patch (obsolete) — Splinter Review
This one is the one you were suggesting. Please review
Attachment #8801726 - Flags: review?(ckerschb)
Comment on attachment 8801726 [details] [diff] [review]
bug1236222.patch

Review of attachment 8801726 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/security/nsCSPContext.cpp
@@ +1048,5 @@
>  
>        // 2) send reports for the policy that was violated
> +      nsCOMPtr<nsIURI> uri = do_QueryInterface(mBlockedContentSource);
> +    // could be a string or URI
> +      if (uri) {

What I meant in my earlier comment is that we should go further back than one level and fix all the entry points that end up here. E.g. can we also the signature of CSPReportSenderRunnable() as well as AsyncReportViolation() to use an nsIURI instead of an nsISupports?
Attachment #8801726 - Flags: review?(ckerschb)
By the way, does your patch cause any tests within dom/security/test/csp to fail?

Comment 13

3 years ago
Shall I check for the url in AsyncReportViolation(), cause its being called in three different places.
Also, it didn't cause any failure
(In reply to Tanuja from comment #13)
> Shall I check for the url in AsyncReportViolation(), cause its being called
> in three different places.
That would be the plan.

> Also, it didn't cause any failure
That's no good. We must have a test for that somewhere. I would imagine that test_redirects.html would fail once we update the right bits within our code.
Status: NEW → ASSIGNED

Comment 15

3 years ago
Posted patch bug1236222.patch (obsolete) — Splinter Review
I've fixed it many functions deep.
Please review.
Attachment #8801851 - Flags: review?(ckerschb)

Comment 16

3 years ago
I tried the nightly on a site with blocked scripts. When 'self', its blank. Else it gives url.

fay@ENVY:~/mozilla-central$ ./mach run
 0:00.89 /home/fay/mozilla-central/obj-x86_64-pc-linux-gnu/dist/bin/firefox -no-remote -profile /home/fay/mozilla-central/obj-x86_64-pc-linux-gnu/tmp/scratch_user
1476730890981	addons.xpi-utils	WARN	Disabling foreign installed add-on ubufox@ubuntu.com in app-system-share
ATTENTION: default value of option force_s3tc_enable overridden by environment.
ATTENTION: default value of option force_s3tc_enable overridden by environment.
ATTENTION: default value of option force_s3tc_enable overridden by environment.
Blocked:
Blocked:http://htmledit.squarefree.com/

Comment 17

3 years ago
Posted patch bug1236222.patch (obsolete) — Splinter Review
This one is better. Please review.
Attachment #8801568 - Attachment is obsolete: true
Attachment #8801726 - Attachment is obsolete: true
Attachment #8801851 - Attachment is obsolete: true
Attachment #8801851 - Flags: review?(ckerschb)
Attachment #8801874 - Flags: review?(ckerschb)
Comment on attachment 8801874 [details] [diff] [review]
bug1236222.patch

Review of attachment 8801874 [details] [diff] [review]:
-----------------------------------------------------------------

I think we need to go all the way to find the root cause and replace all nsISupports* with nsIURI* - thanks!

::: dom/security/nsCSPContext.cpp
@@ +804,2 @@
>    // blocked-uri
> +  

nit: trailing whitespace.

@@ +809,2 @@
>      // could be a string or URI
> +    StripURIForReporting(uri, mSelfURI, reportBlockedURI); 

no need for the tmp variable uri, just use:
StripURIForReporting(aBlockedURI, ...);

Please also fix the trailing whitespace issue.

@@ +811,2 @@
>      report.mCsp_report.mBlocked_uri = NS_ConvertUTF8toUTF16(reportBlockedURI);
> +    

nit: trailing whitespace.

@@ +1083,5 @@
>        return NS_OK;
>      }
>  
>    private:
> +    nsCOMPtr<nsIURI>   mBlockedURI;

nit, please align with other member variables.

@@ +1132,5 @@
>                                     uint32_t aLineNum)
>  {
>    NS_ENSURE_ARG_MAX(aViolatedPolicyIndex, mPolicies.Length() - 1);
>  
> +  nsCOMPtr<nsIURI> uri = do_QueryInterface(aBlockedContentSource);

As suggested earlier, we also need to update the callsites of AsyncReportViolation to pass aURI instead of aBlockedContentSource

@@ +1134,5 @@
>    NS_ENSURE_ARG_MAX(aViolatedPolicyIndex, mPolicies.Length() - 1);
>  
> +  nsCOMPtr<nsIURI> uri = do_QueryInterface(aBlockedContentSource);
> +
> +  if (uri) {

why do we need to branch here and duplicate code?

::: dom/security/nsCSPContext.h
@@ +56,5 @@
>                        uint32_t aLineNumber,
>                        uint32_t aColumnNumber,
>                        uint32_t aSeverityFlag);
>  
> +    nsresult SendReports(nsIURI* mBlockedURI,

prefix 'm' denotes member variable, prefix of 'a' denotes 'argument', please update to aBlockedURI.
Attachment #8801874 - Flags: review?(ckerschb)

Comment 19

3 years ago
Posted patch bug1236222.patch (obsolete) — Splinter Review
I've fixed the AsyncReportViolation part. Please review
Attachment #8801874 - Attachment is obsolete: true
Attachment #8805803 - Flags: review?(ckerschb)
Comment on attachment 8805803 [details] [diff] [review]
bug1236222.patch

Review of attachment 8805803 [details] [diff] [review]:
-----------------------------------------------------------------

Hey Tanuja, I had a close look at your patch and also at our code. It took me a long time but I traced all the callsites for you to figure out whether we can update AsyncReportViolation as well as SendReports. Answer is, we can only update SendReports but not AsyncReportViolation. Reason is that AsyncReportViolation also logs information to the developer console [1] which would get lost if we only pass a nullptr instead of the actual string to be reported to the console.

In other words that means, that whenver we call SendReports() we have to check if we can query the interface of nsISupports into an nsIURI. If that's not possible the result should be a nullptr anyway and we should be good to go. So you would have to do something like:
  nsCOMPtr<nsIURI> uri = do_QueryInterface(uriOrString);
an then call
  SendReports(uri, ...)


[1] https://dxr.mozilla.org/mozilla-central/source/dom/security/nsCSPContext.cpp#1090

::: dom/security/nsCSPContext.cpp
@@ +807,2 @@
>      nsAutoCString reportBlockedURI;
> +    StripURIForReporting(aBlockedURI, mSelfURI, reportBlockedURI);

Keep those changes.

::: dom/security/nsCSPContext.h
@@ +56,5 @@
>                        uint32_t aLineNumber,
>                        uint32_t aColumnNumber,
>                        uint32_t aSeverityFlag);
>  
> +    nsresult SendReports(nsIURI* aBlockedURI,

Keep that.

@@ +64,5 @@
>                           nsAString& aSourceFile,
>                           nsAString& aScriptSample,
>                           uint32_t aLineNum);
>  
> +    nsresult AsyncReportViolation(nsIURI* aBlockedContentSource,

Please change that back.
Attachment #8805803 - Flags: review?(ckerschb)

Comment 21

3 years ago
Posted patch bug1236222.patch (obsolete) — Splinter Review
I've added the changes you've asked for. Please review.
Attachment #8805803 - Attachment is obsolete: true
Comment on attachment 8806300 [details] [diff] [review]
bug1236222.patch

Review of attachment 8806300 [details] [diff] [review]:
-----------------------------------------------------------------

This one is getting really close. Does it pass all csp tests in the tree? E.g. does it pass:
./mach mochitest dom/security/test/csp?

::: dom/security/nsCSPContext.cpp
@@ +1043,5 @@
>                                                       mViolatedDirective.get());
>        NS_ENSURE_SUCCESS(rv, rv);
>  
>        // 2) send reports for the policy that was violated
> +      nsCOMPtr<nsIURI> uri = do_QueryInterface(mBlockedContentSource);

please name the variable blockedURI

@@ +1050,5 @@
>                                 mSourceFile, mScriptSample, mLineNum);
>  
>        // 3) log to console (one per policy violation)
>        // mBlockedContentSource could be a URI or a string.
> +      nsCOMPtr<nsIURI> blockedURI = uri;

please remove that line and just reuse blockedURI from above.

::: dom/security/nsCSPContext.h
@@ +159,5 @@
>    private:
>      nsCOMPtr<nsINetworkInterceptController> mInterceptController;
>  };
>  
> +#endif /* nsCSPContext_h___ */

what did change here?

Comment 23

3 years ago
Posted patch bug1236222.patch (obsolete) — Splinter Review
Fixed the problems. The tests gave the following results:

0 INFO TEST-START | Shutdown
1 INFO Passed:  660
2 INFO Failed:  1
3 INFO Todo:    2
4 INFO Mode:    e10s
5 INFO SimpleTest FINISHED
The following tests failed:
866 INFO TEST-UNEXPECTED-FAIL | dom/security/test/csp/test_report.html | Incorrect blocked-uri - got "", expected "self"
    SimpleTest.is@SimpleTest/SimpleTest.js:270:5
    window.checkResults@dom/security/test/csp/test_report.html:51:3
    ml@dom/security/test/csp/test_report.html:80:7
SUITE-END | took 32s


Failed as expected. This was the bug
Attachment #8806300 - Attachment is obsolete: true
(In reply to Tanuja from comment #23)
> 866 INFO TEST-UNEXPECTED-FAIL | dom/security/test/csp/test_report.html |
> Incorrect blocked-uri - got "", expected "self"

Alright, so before we can merge that change into the codebase we have to update that test.

Comment 25

3 years ago
Posted patch bug1236222.patch (obsolete) — Splinter Review
Fixed the test case. Got this as output

WARNING | leakcheck | refcount logging is off, so leaks can't be detected!
runtests.py | Running tests: end.
0 INFO TEST-START | Shutdown
1 INFO Passed:  661
2 INFO Failed:  0
3 INFO Todo:    2
4 INFO Mode:    e10s
5 INFO SimpleTest FINISHED
SUITE-END | took 30s
Attachment #8807647 - Attachment is obsolete: true
Comment on attachment 8808167 [details] [diff] [review]
bug1236222.patch

Review of attachment 8808167 [details] [diff] [review]:
-----------------------------------------------------------------

Code looks good and test update also looks good. Please update the commit message to something like:
> Bug 126222 - CSP: Blocked URI should be empty for inline violations. r=ckerschb

Btw, did you also run web platform tests?
./mach test testing/web-platform/tests/content-security-policy/
Attachment #8808167 - Flags: feedback+

Comment 27

3 years ago
Posted patch bug1236222.patch (obsolete) — Splinter Review
Got this as the result.


Summary
=======

Ran 492 tests (171 parents, 321 subtests)
Expected results: 474
Unexpected results: 8 (FAIL: 4, PASS: 4)
Skipped: 10

Unexpected Results
==================

/content-security-policy/blink-contrib/object-src-applet-archive-codebase.sub.html
----------------------------------------------------------------------------------
FAIL expected NOTRUN Expecting logs: ["PASS"]
PASS expected FAIL Violation report status OK.
/content-security-policy/blink-contrib/object-src-applet-archive.sub.html
-------------------------------------------------------------------------
FAIL expected NOTRUN Expecting logs: ["PASS"]
PASS expected FAIL Violation report status OK.
/content-security-policy/blink-contrib/object-src-applet-code-codebase.sub.html
-------------------------------------------------------------------------------
FAIL expected NOTRUN Expecting logs: ["PASS"]
PASS expected FAIL Violation report status OK.
/content-security-policy/blink-contrib/object-src-applet-code.sub.html
----------------------------------------------------------------------
FAIL expected NOTRUN Expecting logs: ["PASS"]
PASS expected FAIL Violation report status OK.
 7:31.39 LOG: MainThread INFO Closing logging queue
 7:31.39 LOG: MainThread INFO queue closed

So I retried with the default code. Built it again and ran the test again

Got this

Summary
=======

Ran 498 tests (174 parents, 324 subtests)
Expected results: 481
Unexpected results: 8 (FAIL: 4, PASS: 4)
Skipped: 9

Unexpected Results
==================

/content-security-policy/blink-contrib/object-src-applet-archive-codebase.sub.html
----------------------------------------------------------------------------------
FAIL expected NOTRUN Expecting logs: ["PASS"]
PASS expected FAIL Violation report status OK.
/content-security-policy/blink-contrib/object-src-applet-archive.sub.html
-------------------------------------------------------------------------
FAIL expected NOTRUN Expecting logs: ["PASS"]
PASS expected FAIL Violation report status OK.
/content-security-policy/blink-contrib/object-src-applet-code-codebase.sub.html
-------------------------------------------------------------------------------
FAIL expected NOTRUN Expecting logs: ["PASS"]
PASS expected FAIL Violation report status OK.
/content-security-policy/blink-contrib/object-src-applet-code.sub.html
----------------------------------------------------------------------
FAIL expected NOTRUN Expecting logs: ["PASS"]
PASS expected FAIL Violation report status OK.
 7:17.46 LOG: MainThread INFO Closing logging queue
 7:17.46 LOG: MainThread INFO queue closed


So the problem is not the changes we made.
Thanks; please upload a new patch with an updated commit message, something like:
> Bug 1236222: CSP: Blocked URI should be empty for inline violations. r=ckerschb
and I am happy to r+ that changeset.

Comment 29

3 years ago
Hi, the latest patch I uploaded has that commit message.
Attachment #8808167 - Attachment is obsolete: true

Comment 30

3 years ago
Pushed by mozilla@christophkerschbaumer.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9c1069e2a42e
CSP: Blocked URI should be empty for inline violations. r=ckerschb

Comment 31

3 years ago
Backout by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eac5fd08280a
Backed out changeset 9c1069e2a42e for failing xpcshell test test_csp_reports.js. r=backout
(In reply to Pulsebot from comment #31)
> Backout by archaeopteryx@coole-files.de:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/eac5fd08280a
> Backed out changeset 9c1069e2a42e for failing xpcshell test
> test_csp_reports.js. r=backout

Tanuja, can you take a look?
Flags: needinfo?(f2013658)
Backed this out for failing xpcshell test test_csp_reports.js:

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=38882473&repo=mozilla-inbound

[task 2016-11-09T11:17:53.321401Z] 11:17:53     INFO -  "CONSOLE_MESSAGE: (error) [JavaScript Error: "Content Security Policy: The page’s settings blocked the loading of a resource at data:image/png;base64,iVBORw0KGgoAAAANSU... (“default-src 'none'”)."]"
[task 2016-11-09T11:17:53.323815Z] 11:17:53     INFO -  "CONSOLE_MESSAGE: (error) [JavaScript Error: "Content Security Policy: The page’s settings blocked the loading of a resource at intent://mymaps.com/maps?um=1&ie=UTF-8&fb=1&sll (“default-src 'none'”)."]"
[task 2016-11-09T11:17:53.326137Z] 11:17:53     INFO -  "CONSOLE_MESSAGE: (error) [JavaScript Error: "Content Security Policy: The page’s settings blocked the loading of a resource at http://localhost:47040/foo/self/foo.js#bar (“default-src 'none'”)."]"
[task 2016-11-09T11:17:53.329703Z] 11:17:53     INFO -  "CONSOLE_MESSAGE: (error) [JavaScript Error: "Content Security Policy: The page’s settings blocked the loading of a resource at ftp://blocked.test/profile.png (“default-src 'none'”)."]"
[task 2016-11-09T11:17:53.331619Z] 11:17:53  WARNING -  TEST-UNEXPECTED-FAIL | dom/security/test/unit/test_csp_reports.js | makeReportHandler/< - [makeReportHandler/< : 55] "self" == ""
[task 2016-11-09T11:17:53.333731Z] 11:17:53     INFO -  /home/worker/workspace/build/tests/xpcshell/tests/dom/security/test/unit/test_csp_reports.js:makeReportHandler/<:55
[task 2016-11-09T11:17:53.337338Z] 11:17:53     INFO -  resource://testing-common/httpd.js:ServerHandler.prototype.handleResponse:2374
[task 2016-11-09T11:17:53.339187Z] 11:17:53     INFO -  resource://testing-common/httpd.js:Connection.prototype.process:1225
[task 2016-11-09T11:17:53.340839Z] 11:17:53     INFO -  resource://testing-common/httpd.js:RequestReader.prototype._handleResponse:1679
[task 2016-11-09T11:17:53.343214Z] 11:17:53     INFO -  resource://testing-common/httpd.js:RequestReader.prototype._processBody:1527
[task 2016-11-09T11:17:53.344893Z] 11:17:53     INFO -  resource://testing-common/httpd.js:RequestReader.prototype.onInputStreamReady:1395
[task 2016-11-09T11:17:53.346950Z] 11:17:53     INFO -  /home/worker/workspace/build/tests/xpcshell/head.js:_do_main:210
[task 2016-11-09T11:17:53.348927Z] 11:17:53     INFO -  /home/worker/workspace/build/tests/xpcshell/head.js:_execute_test:545
[task 2016-11-09T11:17:53.350814Z] 11:17:53     INFO -  -e:null:1
[task 2016-11-09T11:17:53.353285Z] 11:17:53     INFO -  exiting test

Comment 34

3 years ago
So, shall I modify this test too? It seems to expect a 'self'.

Comment 35

3 years ago
Also, how do I  run the unit test?
(In reply to Tanuja from comment #34)
> So, shall I modify this test too? It seems to expect a 'self'.

Yes, please test.

(In reply to Tanuja from comment #35)
> Also, how do I  run the unit test?

./mach test dom/security/test/unit/test_csp_reports.js
I am finishing this for Tanuja Sawant.
This is the follow-up with the tiny test changes. I've pushed this to our try server at https://treeherder.mozilla.org/#/jobs?repo=try&revision=de099c71e4021654aeb94b65c4fd49ea317b0681

Please review, when you get to it, ckerschb. (You don't officially accept review requests)
Attachment #8808524 - Attachment is obsolete: true
Flags: needinfo?(f2013658)
Attachment #8879121 - Flags: review?(ckerschb)
Comment on attachment 8879121 [details] [diff] [review]
0001-Bug-126222-CSP-Blocked-URI-should-be-empty-for-inlin.patch

Review of attachment 8879121 [details] [diff] [review]:
-----------------------------------------------------------------

thanks for pushing this one over the finishing line. r=me
Attachment #8879121 - Flags: review?(ckerschb) → review+
Assignee: f2013658 → fbraun
Priority: P3 → P2
Whiteboard: [domsecurity-backlog2] → [domsecurity-active]
Assignee: fbraun → nobody
Status: ASSIGNED → NEW
Assignee

Comment 39

Last year
Is this worth picking up? It looks like the patch won't remotely apply anymore.
Flags: needinfo?(ckerschb)
I wonder why this never landed, but it got r+ed. :jkt, can you drag it over the finishing line? thanks.
Flags: needinfo?(ckerschb) → needinfo?(jkt)
Comment hidden (mozreview-request)
Assignee

Updated

Last year
Assignee: nobody → jkt
Flags: needinfo?(jkt)
Comment hidden (mozreview-request)

Comment 43

Last year
mozreview-review
Comment on attachment 8957384 [details]
Bug 1236222 - CSP: Blocked URI should be empty for inline violations.

https://reviewboard.mozilla.org/r/226312/#review232286

Yep, I think that looks good. Can you please mark the other patch as obsolete? Thanks!
Attachment #8957384 - Flags: review?(ckerschb) → review+
Assignee

Updated

Last year
Attachment #8879121 - Attachment is obsolete: true

Comment 44

Last year
Pushed by jkingston@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7d62c45d5973
CSP: Blocked URI should be empty for inline violations. r=ckerschb
Comment hidden (mozreview-request)

Comment 47

Last year
Pushed by jkingston@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0e90c9123426
CSP: Blocked URI should be empty for inline violations. r=ckerschb

Comment 48

Last year
bugherder
https://hg.mozilla.org/mozilla-central/rev/0e90c9123426
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Assignee

Updated

Last year
Flags: needinfo?(jkt)
You need to log in before you can comment on or make changes to this bug.