Closed Bug 1312541 Opened 3 years ago Closed 3 years ago

Test first-party isolation of cookies

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: arthur, Assigned: arthur)

References

(Blocks 1 open bug)

Details

(Whiteboard: [tor][domsecurity-active])

Attachments

(1 file, 4 obsolete files)

Bug 1264567 added a tested for first-party isolation of localStorage. I'd like to do a similar test for cookies.
Status: NEW → ASSIGNED
Priority: -- → P1
Attachment #8804111 - Flags: review?(tihuang)
Attachment #8804111 - Flags: review?(amarchesini)
Attachment #8804111 - Flags: review?(amarchesini) → review+
Comment on attachment 8804111 [details] [diff] [review]
0001-Bug-1312541-Test-isolation-of-cookies.patch

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

LGTM.
Attachment #8804111 - Flags: review?(tihuang) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0303be04c579
Test isolation of cookies. r=baku, r=timhuang
Keywords: checkin-needed
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/48dbaf075fc6
Backed out changeset 0303be04c579 for es lint failures
Whiteboard: [tor] → [tor][domsecurity-active]
Fixing es-lint failure (and retaining r+).
Attachment #8804111 - Attachment is obsolete: true
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/48d4f903157a
Test isolation of cookies. r=baku, r=timhuang
Keywords: checkin-needed
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
Flags: needinfo?(arthuredelstein)
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();
});
(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!
Attachment #8806167 - Attachment is obsolete: true
Flags: needinfo?(arthuredelstein)
Keywords: checkin-needed
Comment on attachment 8806957 [details] [diff] [review]
0001-Bug-1312541-Test-isolation-of-cookies.patch

The patch here is not up-to-date.
(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.
Attachment #8806957 - Attachment is obsolete: true
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc36f70ecd19
Test isolation of cookies. r=baku, r=timhuang
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/dc36f70ecd19
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.