Closed Bug 1320404 Opened 8 years ago Closed 6 years ago

Remove appId from origin attributes

Categories

(Core :: Security: CAPS, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: ehsan.akhgari, Assigned: baku)

References

(Blocks 2 open bugs)

Details

Attachments

(4 files)

There is now only one real consumer for OriginAttributes::mAppId, which we can port to use firstPartyDomain (see bug 1320402). After that, I think we can remove mAppId and shrink down OriginAttributes even more. Kris, is this something that interests you? :-)
Flags: needinfo?(kmaglione+bmo)
Sure, why not.
Assignee: nobody → kmaglione+bmo
Flags: needinfo?(kmaglione+bmo)
\o/ Thank you!
Blocks: 1339081
No longer blocks: 1339081
No longer blocks: 1369194
Adding ddn, just to check the docs for this.
Keywords: dev-doc-needed
Kris, I hope you don't mind if I take this.
Assignee: kmaglione+bmo → valentin.gosu
:valentine can I take this?
Flags: needinfo?(valentin.gosu)
Sure!
Assignee: valentin.gosu → 1991manish.kumar
Flags: needinfo?(valentin.gosu)
seems OriginAttributes::mAppId is not available anywhere! https://searchfox.org/mozilla-central/search?q=OriginAttributes%3A%3AmAppId&path=
Flags: needinfo?(ehsan)
(In reply to Manish [:manishkk] from comment #7) > seems OriginAttributes::mAppId is not available anywhere! > https://searchfox.org/mozilla-central/ > search?q=OriginAttributes%3A%3AmAppId&path= https://searchfox.org/mozilla-central/rev/05d91d3e02a0780f44599371005591d7988e2809/dom/chrome-webidl/ChromeUtils.webidl#409,416
only need to remove these 2 lines? > https://searchfox.org/mozilla-central/rev/ > 05d91d3e02a0780f44599371005591d7988e2809/dom/chrome-webidl/ChromeUtils. > webidl#409,416
Flags: needinfo?(ehsan) → needinfo?(valentin.gosu)
(In reply to Manish [:manishkk] from comment #9) > only need to remove these 2 lines? > > > https://searchfox.org/mozilla-central/rev/ > > 05d91d3e02a0780f44599371005591d7988e2809/dom/chrome-webidl/ChromeUtils. > > webidl#409,416 No, there are quite a lot of other references that need to be removed as well: https://searchfox.org/mozilla-central/search?q=mAppId&case=true&path=
Flags: needinfo?(valentin.gosu)
I'll note, though, that we need to decide what to do about appId in the origin suffix parser: https://searchfox.org/mozilla-central/source/caps/OriginAttributes.cpp#209 When we removed the addonId origin attribute, we decided to just ignore that parameter when parsing origin attributes, since it's automatically generated from the URL, anyway. Any remaining references to principals with an appId in their origin attribute would likely cause problems if we decided to treat them as the same origin with the appId stripped. Which means we'll probably have to fail to parse in that case. I'm not entirely sure what side-effects that will have in all possible cases...
Assignee: 1991manish.kumar → nobody
Assignee: nobody → amarchesini
Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5a20b5f43280 Remove appId from origin attributes - part 1 - OriginAttributes and nsIPrincipal, r=Ehsan,flod https://hg.mozilla.org/integration/autoland/rev/557b586f774a Remove appId from origin attributes - part 2 - NO_APP_ID UNKNOWN_APP_ID, r=Ehsan https://hg.mozilla.org/integration/autoland/rev/fed7c475d75c Remove appId from origin attributes - part 3 - Permissionmanager, r=Ehsan https://hg.mozilla.org/integration/autoland/rev/fbacf18b6532 Remove appId from origin attributes - part 4 - necko, r=valentin

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Ctestfailed%2Cbusted%2Cexception&searchStr=xperf&revision=fbacf18b653259954711b20fcefad7c8a82ce2b1&selectedJob=244071797

Failure log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=244071797&repo=autoland&lineNumber=1678
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=244052943&repo=autoland&lineNumber=5112

Backout link: https://hg.mozilla.org/integration/autoland/rev/ad04ccedc21ed4373acff2d310388bc55725182b

01:37:29 INFO - Merged Etl: test.etl
01:39:54 INFO - reading etl filename: test.etl
01:39:54 INFO - etlparser: in readfile: test.etl.csv
01:39:54 INFO - TEST-UNEXPECTED-FAIL : xperf: File '{profile}\permissions.sqlite-journal' (normalized from 'C:\Users\task_1556757239\AppData\Local\Temp\tmps8ccdp\profile\permissions.sqlite-journal') was accessed and we were not expecting it. DiskReadCount: 0, DiskWriteCount: 18, DiskReadBytes: 0, DiskWriteBytes: 7208
01:39:56 INFO - extending with xperf!
01:39:56 INFO - Detected a regression for tp5n
01:39:56 INFO - TEST-UNEXPECTED-FAIL | tp5n | Talos has found a regression, if you have questions ask for help in irc on #perf
01:39:56 ERROR - Traceback (most recent call last):
01:39:56 INFO - File "Z:\task_1556757239\build\tests\talos\talos\run_tests.py", line 300, in run_tests
01:39:56 INFO - talos_results.add(mytest.runTest(browser_config, test))
01:39:56 INFO - File "Z:\task_1556757239\build\tests\talos\talos\ttest.py", line 64, in runTest
01:39:56 INFO - return self._runTest(browser_config, test_config, setup)
01:39:56 INFO - File "Z:\task_1556757239\build\tests\talos\talos\ttest.py", line 268, in _runTest
01:39:56 INFO - 'Talos has found a regression, if you have questions'
01:39:56 INFO - TalosRegression: Talos has found a regression, if you have questions ask for help in irc on #perf
01:39:56 INFO - TEST-INFO took 289967ms
01:39:56 INFO - SUITE-END | took 289s
01:39:56 ERROR - Return code: 1
01:39:56 WARNING - setting return code to 1

Flags: needinfo?(amarchesini)
Flags: needinfo?(amarchesini)
Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/de9073c57d20 Remove appId from origin attributes - part 1 - OriginAttributes and nsIPrincipal, r=Ehsan,flod https://hg.mozilla.org/integration/autoland/rev/dd741b25a244 Remove appId from origin attributes - part 2 - NO_APP_ID UNKNOWN_APP_ID, r=Ehsan https://hg.mozilla.org/integration/autoland/rev/a7e7c0251179 Remove appId from origin attributes - part 3 - Permissionmanager, r=Ehsan https://hg.mozilla.org/integration/autoland/rev/7c2f4e64d38e Remove appId from origin attributes - part 4 - necko, r=valentin

Backed out 4 changesets (Bug 1320404) for test_permmanager_load_invalid_entries.js failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=linux%2Cx64%2Casan%2Cxpcshell%2Ctests%2Ctest-linux64-asan%2Fopt-xpcshell-e10s-3%2Cx%28x3%29&fromchange=0ef6e3e9552df27c04a2e839d686aaa45ef094e2&tochange=04557fa70ce8a1dd168482b42b734647753c5b70&selectedJob=244393680

Backout link: https://hg.mozilla.org/integration/autoland/rev/04557fa70ce8a1dd168482b42b734647753c5b70

Failure link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=244393680&repo=autoland&lineNumber=2104

[task 2019-05-03T03:09:50.274Z] 03:09:50 INFO - TEST-START | dom/push/test/xpcshell/test_clearAll_successful.js
[task 2019-05-03T03:09:52.113Z] 03:09:52 INFO - TEST-PASS | dom/push/test/xpcshell/test_clearAll_successful.js | took 1843ms
[task 2019-05-03T03:09:52.117Z] 03:09:52 INFO - Retrying tests that failed when run in parallel.
[task 2019-05-03T03:09:52.125Z] 03:09:52 INFO - TEST-START | extensions/permissions/test/unit/test_permmanager_load_invalid_entries.js
[task 2019-05-03T03:09:52.881Z] 03:09:52 WARNING - TEST-UNEXPECTED-FAIL | extensions/permissions/test/unit/test_permmanager_load_invalid_entries.js | xpcshell return code: 0
[task 2019-05-03T03:09:52.882Z] 03:09:52 INFO - TEST-INFO took 754ms
[task 2019-05-03T03:09:52.883Z] 03:09:52 INFO - >>>>>>>
[task 2019-05-03T03:09:52.884Z] 03:09:52 INFO - (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2019-05-03T03:09:52.885Z] 03:09:52 INFO - TEST-PASS | extensions/permissions/test/unit/test_permmanager_load_invalid_entries.js | run_test - [run_test : 19] true == true
[task 2019-05-03T03:09:52.887Z] 03:09:52 WARNING - TEST-UNEXPECTED-FAIL | extensions/permissions/test/unit/test_permmanager_load_invalid_entries.js | run_test - [run_test : 124] 10 == 9
[task 2019-05-03T03:09:52.888Z] 03:09:52 INFO - /builds/worker/workspace/build/tests/xpcshell/tests/extensions/permissions/test/unit/test_permmanager_load_invalid_entries.js:run_test:124
[task 2019-05-03T03:09:52.889Z] 03:09:52 INFO - /builds/worker/workspace/build/tests/xpcshell/head.js:_execute_test:523
[task 2019-05-03T03:09:52.889Z] 03:09:52 INFO - -e:null:1
[task 2019-05-03T03:09:52.889Z] 03:09:52 INFO - exiting test
[task 2019-05-03T03:09:52.889Z] 03:09:52 INFO - "CONSOLE_MESSAGE: (info) No chrome package registered for chrome://branding/locale/brand.properties"
[task 2019-05-03T03:09:52.889Z] 03:09:52 INFO - <<<<<<<
[task 2019-05-03T03:09:52.891Z] 03:09:52 INFO - INFO | Result summary:
[task 2019-05-03T03:09:52.892Z] 03:09:52 INFO - INFO | Passed: 382
[task 2019-05-03T03:09:52.893Z] 03:09:52 WARNING - INFO | Failed: 1
[task 2019-05-03T03:09:52.893Z] 03:09:52 WARNING - One or more unittests failed.
[task 2019-05-03T03:09:52.894Z] 03:09:52 INFO - INFO | Todo: 0
[task 2019-05-03T03:09:52.895Z] 03:09:52 INFO - INFO | Retried: 1
[task 2019-05-03T03:09:52.895Z] 03:09:52 INFO - SUITE-END | took 230s
[task 2019-05-03T03:09:52.900Z] 03:09:52 INFO - Node moz-http2 server shutting down ...
[task 2019-05-03T03:09:52.973Z] 03:09:52 ERROR - Return code: 1
[task 2019-05-03T03:09:52.975Z] 03:09:52 INFO - TinderboxPrint: xpcshell-xpcshell<br/>382/<em class="testfail">1</em>/0
[task 2019-05-03T03:09:52.976Z] 03:09:52 WARNING - # TBPL FAILURE #
[task 2019-05-03T03:09:52.978Z] 03:09:52 WARNING - setting return code to 2
[task 2019-05-03T03:09:52.980Z] 03:09:52 WARNING - The xpcshell suite: xpcshell ran with return status: FAILURE

Flags: needinfo?(amarchesini)
Flags: needinfo?(amarchesini)
Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0ef64a5380eb Remove appId from origin attributes - part 1 - OriginAttributes and nsIPrincipal, r=Ehsan,flod https://hg.mozilla.org/integration/autoland/rev/34023e7e4908 Remove appId from origin attributes - part 2 - NO_APP_ID UNKNOWN_APP_ID, r=Ehsan https://hg.mozilla.org/integration/autoland/rev/b0f584907e03 Remove appId from origin attributes - part 3 - Permissionmanager, r=Ehsan https://hg.mozilla.org/integration/autoland/rev/e720637a07b8 Remove appId from origin attributes - part 4 - necko, r=valentin
Regressions: 1549912
Regressions: 1550004
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: