991474 - CSP in C++: Remove selfURI from AppendPolicy after removing the old parser

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[Christoph Kerschbaumer [:ckerschb}]

The new CSP parser implementation is going to use the selfURI provided in

> void setRequestContext(in nsIURI selfURI,
>                         in nsIURI referrer,
>                         in nsIPrincipal documentPrincipal,
>                         in nsIChannel aChannel);

and therefore we can remove it from

> void appendPolicy(in AString policyString, in nsIURI selfURI,
>                   in boolean reportOnly, in boolean specCompliant);

because it's not going to be used any longer.

Comment 1

[Christoph Kerschbaumer [:ckerschb}]

When removing selfURI from AppendPolicy, we can further delete it from AppendCSPFromHeader, see:

/content/base/src/nsDocument.cpp (View Hg log or Hg annotations)
line 2635 -- AppendCSPFromHeader(nsIContentSecurityPolicy* csp, const nsAString& aHeaderValue,
line 2841 -- rv = AppendCSPFromHeader(csp, cspHeaderValue, selfURI, false, true);
line 2844 -- rv = AppendCSPFromHeader(csp, cspOldHeaderValue, selfURI, false, false);
line 2850 -- rv = AppendCSPFromHeader(csp, cspROHeaderValue, selfURI, true, true);
line 2853 -- rv = AppendCSPFromHeader(csp, cspOldROHeaderValue, selfURI, true, false);

Comment 2

[Sid Stamm [:geekboy or :sstamm]]

Attachment: remove-AppendPolicy-selfURI

Proposed patch.  This will have merge conflicts if another patch removes "specCompliant" from nsIContentSecurityPolicy::AppendPolicy().  Trivially fixed, but heads up.

Comment 3

[Sid Stamm [:geekboy or :sstamm]]

Attachment: remove-AppendPolicy-selfURI

Also, pushed this along with bug 994872, bug 991468 and bug 994782 to try to see how they do:
https://tbpl.mozilla.org/?tree=Try&rev=23c1af15cb65

Comment 4

[Christoph Kerschbaumer [:ckerschb}]

Comment on attachment 8462912 [details] [diff] [review]
remove-AppendPolicy-selfURI

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

This is ready to land! Let's roll!

Comment 5

[Sid Stamm [:geekboy or :sstamm]]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #4)
> This is ready to land! Let's roll!

But not before bug 994782.  :)

Comment 6

[Sid Stamm [:geekboy or :sstamm]]

flagging dev-doc-needed since this is a change for the IDL.  We should probably update the dev-docs for the other bugs that change the API and land the same time as this (see comment 3).

Comment 7

[Sid Stamm [:geekboy or :sstamm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5fd34036ca36

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5fd34036ca36