1312541 - Test first-party isolation of cookies

Status: RESOLVED FIXED

(Core :: DOM: Security, defect, P1)

Description

[Arthur Edelstein [:arthur]]

Bug 1264567 added a tested for first-party isolation of localStorage. I'd like to do a similar test for cookies.

Comment 1

[Arthur Edelstein [:arthur]]

Attachment: 0001-Bug-1312541-Test-isolation-of-cookies.patch

Comment 2

[Tim Huang[:timhuang]]

Comment on attachment 8804111 [details] [diff] [review]
0001-Bug-1312541-Test-isolation-of-cookies.patch

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

LGTM.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0303be04c579
Test isolation of cookies. r=baku, r=timhuang

Comment 4

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/48dbaf075fc6
Backed out changeset 0303be04c579 for es lint failures

Comment 5

[Arthur Edelstein [:arthur]]

Attachment: 0001-Bug-1312541-Test-isolation-of-cookies.patch

Fixing es-lint failure (and retaining r+).

Comment 6

[Arthur Edelstein [:arthur]]

Attachment: 0001-Bug-1312541-Test-isolation-of-cookies.-r-baku-r-timh.patch

(Another minor revision.)
Try results: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a51b06ec1efb

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/48d4f903157a
Test isolation of cookies. r=baku, r=timhuang

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for permafailing browser_favicon_firstParty.js (yes, I know this is a test-only change in a different test):

https://hg.mozilla.org/integration/mozilla-inbound/rev/ec5ec8b7f05cec8b28f38a0073772f4cb16fa8f3

Push which run the tests: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=eea142c25c76e6c9984b292a67b9c7f475af2a5f
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=38546484&repo=mozilla-inbound

[task 2016-11-02T12:09:27.302019Z] 12:09:27     INFO - TEST-PASS | browser/components/originattributes/test/browser/browser_favicon_firstParty.js | The loadingPrincipal of favicon loading from Places should be the content prinicpal - 
[task 2016-11-02T12:09:27.309056Z] 12:09:27     INFO - TEST-PASS | browser/components/originattributes/test/browser/browser_favicon_firstParty.js | The cookie of the favicon loading is correct. - 
[task 2016-11-02T12:09:27.312215Z] 12:09:27     INFO - TEST-PASS | browser/components/originattributes/test/browser/browser_favicon_firstParty.js | The loadInfo has correct first party domain - 
[task 2016-11-02T12:09:27.316181Z] 12:09:27     INFO - TEST-PASS | browser/components/originattributes/test/browser/browser_favicon_firstParty.js | The triggeringPrincipal of favicon loading from XUL should be the content principal. - 
[task 2016-11-02T12:09:27.319399Z] 12:09:27     INFO - TEST-UNEXPECTED-FAIL | browser/components/originattributes/test/browser/browser_favicon_firstParty.js | The cookie of the favicon loading is correct. - Got key0.22745010130693522=0.15460232550052333; 0.7508506903644762, expected 0.7508506903644762
[task 2016-11-02T12:09:27.321228Z] 12:09:27     INFO - Stack trace:
[task 2016-11-02T12:09:27.323169Z] 12:09:27     INFO - chrome://mochikit/content/browser-test.js:test_is:913
[task 2016-11-02T12:09:27.325257Z] 12:09:27     INFO - chrome://mochitests/content/browser/browser/components/originattributes/test/browser/browser_favicon_firstParty.js:observe:99

Comment 9

[Tim Huang[:timhuang]]

For resolving this problem, I suggest that you can use the cleanup function to clear cookies after test finished. Something like:

registerCleanupFunction(() => {
  Services.cookies.removeAll();
});

Comment 10

[Arthur Edelstein [:arthur]]

Attachment: 0001-Bug-1312541-Test-isolation-of-cookies.patch

(In reply to Tim Huang[:timhuang] from comment #9)
> For resolving this problem, I suggest that you can use the cleanup function
> to clear cookies after test finished. Something like:
> 
> registerCleanupFunction(() => {
>   Services.cookies.removeAll();
> });

That fixed it. Thanks, Tim and Sebastian!

Comment 11

[Arthur Edelstein [:arthur]]

Here are the clean try-server results:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9bfbf03e4c50

Comment 12

[Tim Huang[:timhuang]]

Comment on attachment 8806957 [details] [diff] [review]
0001-Bug-1312541-Test-isolation-of-cookies.patch

The patch here is not up-to-date.

Comment 13

[Arthur Edelstein [:arthur]]

Attachment: 0001-Bug-1312541-Test-isolation-of-cookies.-r-baku-r-timh.patch

(In reply to Tim Huang[:timhuang] from comment #12)
> Comment on attachment 8806957 [details] [diff] [review]
> 0001-Bug-1312541-Test-isolation-of-cookies.patch
> 
> The patch here is not up-to-date.

Oops -- sorry for that mistake, and thanks for catching it, Tim. Here's the up-to-date version.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc36f70ecd19
Test isolation of cookies. r=baku, r=timhuang

Comment 15

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/dc36f70ecd19