Consider the Access-Control-Allow-Origin header for Background Fetch

Consider the Access-Control-Allow-Origin header for Background Fetch

This adds very basic support for CORS to Background Fetch. A full
security review will be requested and done prior to shipping this.

BUG=692579
Review-Url: https://codereview.chromium.org/2818703003
Cr-Commit-Position: refs/heads/master@{#464740}
Committed: https://chromium.googlesource.com/chromium/src/+/bbecee1672a258c459e05b7a9b88f4ef907497eb

[Peter Beverloo, 2017-04-13 17:05:10]

peter@chromium.org changed reviewers:
+ avi@chromium.org, miguelg@chromium.org

[Peter Beverloo, 2017-04-13 17:05:11]

+miguelg, avi for BUILD.gn

[Peter Beverloo, 2017-04-13 17:05:26]

The CQ bit was checked by peter@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-13 17:06:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Miguel Garcia, 2017-04-13 17:14:22]

lgtm

https://codereview.chromium.org/2818703003/diff/1/content/browser/background_...
File content/browser/background_fetch/background_fetch_cross_origin_filter.cc
(right):

https://codereview.chromium.org/2818703003/diff/1/content/browser/background_...
content/browser/background_fetch/background_fetch_cross_origin_filter.cc:34:
std::set<url::Origin> candidate_origins;
would unordered_set work here? (you'd have to change the ParseHeadersListValue
as well).

https://codereview.chromium.org/2818703003/diff/1/content/browser/background_...
content/browser/background_fetch/background_fetch_cross_origin_filter.cc:39:
url::Origin origin = url::Origin(GURL(origin_string));
Can you create the URL out of the splitted string separately and DCHECK it's
valid or something like it?

https://codereview.chromium.org/2818703003/diff/1/content/browser/background_...
content/browser/background_fetch/background_fetch_cross_origin_filter.cc:81: //
TODO(peter): Consider the Access-Control-Allow-Credentials header.
this kind of call for having a bug to reference.

[Peter Beverloo, 2017-04-13 17:26:02]

https://codereview.chromium.org/2818703003/diff/1/content/browser/background_...
File content/browser/background_fetch/background_fetch_cross_origin_filter.cc
(right):

https://codereview.chromium.org/2818703003/diff/1/content/browser/background_...
content/browser/background_fetch/background_fetch_cross_origin_filter.cc:34:
std::set<url::Origin> candidate_origins;
On 2017/04/13 17:14:21, Miguel Garcia wrote:
> would unordered_set work here? (you'd have to change the ParseHeadersListValue
> as well).

No, because there's no hasher for url::Origin().

https://codereview.chromium.org/2818703003/diff/1/content/browser/background_...
content/browser/background_fetch/background_fetch_cross_origin_filter.cc:39:
url::Origin origin = url::Origin(GURL(origin_string));
On 2017/04/13 17:14:22, Miguel Garcia wrote:
> Can you create the URL out of the splitted string separately and DCHECK it's
> valid or something like it?

The GURL url::Origin constructor is defined to return a unique origin if the URL
is not valid. It does this as the very first thing in the constructor.

https://codereview.chromium.org/2818703003/diff/1/content/browser/background_...
content/browser/background_fetch/background_fetch_cross_origin_filter.cc:81: //
TODO(peter): Consider the Access-Control-Allow-Credentials header.
On 2017/04/13 17:14:21, Miguel Garcia wrote:
> this kind of call for having a bug to reference.

Agreed. Filed now. I'll also mention it in my e-mail to the security folks.

[Peter Beverloo, 2017-04-13 17:26:11]

The CQ bit was checked by peter@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-13 17:26:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Avi (use Gerrit), 2017-04-13 17:32:00]

lgtm

[commit-bot: I haz the power, 2017-04-13 18:33:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-13 18:33:51]

Dry run: This issue passed the CQ dry run.

[Peter Beverloo, 2017-04-14 17:01:26]

The CQ bit was checked by peter@chromium.org

[Peter Beverloo, 2017-04-14 17:01:27]

The patchset sent to the CQ was uploaded after l-g-t-m from miguelg@chromium.org
Link to the patchset: https://codereview.chromium.org/2818703003/#ps20001
(title: "Consider the Access-Control-Allow-Origin header for Background Fetch")

[commit-bot: I haz the power, 2017-04-14 17:02:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-14 17:10:00]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1492189286843960,
"parent_rev": "5f3a801f041786870d779585c2267e41a027cd25", "commit_rev":
"bbecee1672a258c459e05b7a9b88f4ef907497eb"}

[commit-bot: I haz the power, 2017-04-14 17:10:53]

Description was changed from

==========
Consider the Access-Control-Allow-Origin header for Background Fetch

This adds very basic support for CORS to Background Fetch. A full
security review will be requested and done prior to shipping this.

BUG=692579
==========

to

==========
Consider the Access-Control-Allow-Origin header for Background Fetch

This adds very basic support for CORS to Background Fetch. A full
security review will be requested and done prior to shipping this.

BUG=692579

Review-Url: https://codereview.chromium.org/2818703003
Cr-Commit-Position: refs/heads/master@{#464740}
Committed:
https://chromium.googlesource.com/chromium/src/+/bbecee1672a258c459e05b7a9b88...
==========

[commit-bot: I haz the power, 2017-04-14 17:10:54]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/bbecee1672a258c459e05b7a9b88...