If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Principal should always have a valid origin

RESOLVED FIXED in Firefox 55

Status

()

Core
DOM
RESOLVED FIXED
7 months ago
21 days ago

People

(Reporter: baku, Assigned: baku)

Tracking

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(6 attachments)

(Assignee)

Description

7 months ago
Initially these patches were part of bug 1340163 but it's better that they have a own bug.
See comments in bug 1340163.
(Assignee)

Comment 1

7 months ago
Created attachment 8847935 [details] [diff] [review]
part 0 - renaming GetOriginInternal to GetOriginNoSuffixInternal

This patch has been already reviewed in bug 1340163 by qdot.
Assignee: nobody → amarchesini
(Assignee)

Comment 2

7 months ago
Created attachment 8847936 [details] [diff] [review]
part 1 - Setting OriginAttributes in FinishInit()

Patch reviewed by bholley
(Assignee)

Comment 3

7 months ago
Created attachment 8847937 [details] [diff] [review]
part 2 - originNoSuffix in FinishInit

Patch reviewed by bholley
(Assignee)

Comment 4

7 months ago
Created attachment 8847938 [details] [diff] [review]
part 3 - Origin passed as argument when a principal is created

already reviewed by bholley
(Assignee)

Comment 5

7 months ago
Created attachment 8847964 [details] [diff] [review]
part 4 - ContentPrincipalInfo - comment fixed
Attachment #8847964 - Flags: review?(ehsan)
(Assignee)

Comment 6

7 months ago
Created attachment 8847965 [details] [diff] [review]
part 5 - fix tests

This is a WIP. There is still 1 broken test.
(Assignee)

Updated

7 months ago
Depends on: 1340163
Blocks: 1343389
No longer blocks: 1343389
Duplicate of this bug: 1343389
Blocks: 1294915

Comment 8

7 months ago
Comment on attachment 8847964 [details] [diff] [review]
part 4 - ContentPrincipalInfo - comment fixed

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

Your patch doesn't have a good (well, any) commit message, so technically it should be r-minused.  :-)  But in this case, I have been so late to review it that I will let this one slide.  But it *cannot* land with this commit message.

Also, sorry for the delay...

::: dom/workers/ServiceWorkerRegistrar.cpp
@@ +137,5 @@
>  
> +  // Let's see if we have to migrate to a newer version
> +  if (!mDataVersion.EqualsLiteral(SERVICEWORKERREGISTRAR_VERSION)) {
> +    // Each entry must have the correct origin. In case of errors, we remove the
> +    // element from the list because we cannot consider that entry as valid.

Hmm, is there a case where we _expect_ the data to be wrong here?  If yes, can you please add it to this comment?
Attachment #8847964 - Flags: review?(ehsan) → review+

Comment 9

7 months ago
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a70b549ac35d
Principal must always have a valid origin - part 1 - renaming GetOriginInternal to GetOriginNoSuffixInternal, r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/c0e8522353bd
Principal must always have a valid origin - part 2 - move OriginAttributes to the BasePrincipal, r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/d71d95c73542
Principal must always have a valid origin - part 3 - move origin to BasePrincipal, r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/059bcee1ccda
Principal must always have a valid origin - part 4 - origin passed as argument when a principal is created, r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/054a0ab80767
Principal must always have a valid origin - part 5 - fixing a comment in ContentPrincipalInfo, r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/4af10700c64c
Principal must always have a valid origin - part 6 - fixing tests, r=ehsan
Backed out for failing test_websocket-transport.html on OSX 10.10 debug:

https://hg.mozilla.org/integration/mozilla-inbound/rev/933a668a6468f2805b64cd68ee08ad6dc123648b
https://hg.mozilla.org/integration/mozilla-inbound/rev/823199ec777a88dd78c4c22fd3c5021b9a801b28
https://hg.mozilla.org/integration/mozilla-inbound/rev/be4d4a7ef77f2e8ebab9fa5298899ec626cd8ada
https://hg.mozilla.org/integration/mozilla-inbound/rev/8bda12cfb5acbdb70fbad9856f43569849ecaebe
https://hg.mozilla.org/integration/mozilla-inbound/rev/8b74d843bf8e19124437c4846c341c63466c0b7f
https://hg.mozilla.org/integration/mozilla-inbound/rev/e5833e0db627ae2eda6d915841bed03ab69e83dd

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=4af10700c64c4f301158b43f713e2048fab4d3cc&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=87194079&repo=mozilla-inbound

00:12:02     INFO - TEST-START | devtools/shared/security/tests/chrome/test_websocket-transport.html
00:12:02     INFO - GECKO(1877) | [1877] WARNING: 'NS_FAILED(rv)', file /home/worker/workspace/build/src/caps/BasePrincipal.cpp, line 380
00:12:02     INFO - GECKO(1877) | ++DOMWINDOW == 16 (0x13152c800) [pid = 1877] [serial = 16] [outer = 0x13029f000]
00:12:02     INFO - GECKO(1877) | [1877] WARNING: 'NS_FAILED(rv)', file /home/worker/workspace/build/src/caps/BasePrincipal.cpp, line 380
00:12:02     INFO - GECKO(1877) | [1877] WARNING: 'NS_FAILED(rv)', file /home/worker/workspace/build/src/caps/BasePrincipal.cpp, line 380
00:12:03     INFO - GECKO(1877) | [1877] WARNING: 'NS_FAILED(rv)', file /home/worker/workspace/build/src/caps/BasePrincipal.cpp, line 380
00:12:03     INFO - GECKO(1877) | ++DOMWINDOW == 17 (0x1270cc000) [pid = 1877] [serial = 17] [outer = 0x13029f000]
00:12:04     INFO - GECKO(1877) | [1877] WARNING: '!innerWindow', file /home/worker/workspace/build/src/dom/base/WebSocket.cpp, line 1648
00:12:04     INFO - GECKO(1877) | [1877] WARNING: 'aRv.Failed()', file /home/worker/workspace/build/src/dom/base/WebSocket.cpp, line 1320
00:12:04     INFO - TEST-INFO | started process screencapture
00:12:04     INFO - TEST-INFO | screencapture: exit 0
00:12:04     INFO - Buffered messages logged at 00:12:04
00:12:04     INFO - SpawnTask.js | Entering test 
00:12:04     INFO - TEST-PASS | devtools/shared/security/tests/chrome/test_websocket-transport.html | 0 listening sockets 
00:12:04     INFO - TEST-PASS | devtools/shared/security/tests/chrome/test_websocket-transport.html | Socket listener created 
00:12:04     INFO - TEST-PASS | devtools/shared/security/tests/chrome/test_websocket-transport.html | 1 listening socket 
00:12:04     INFO - Buffered messages finished
00:12:04     INFO - TEST-UNEXPECTED-FAIL | devtools/shared/security/tests/chrome/test_websocket-transport.html | SecurityError: The operation is insecure. 
00:12:04     INFO - add_task/</<@chrome://mochikit/content/tests/SimpleTest/SpawnTask.js:282:15
00:12:04     INFO - onRejected@chrome://mochikit/content/tests/SimpleTest/SpawnTask.js:85:15
00:12:04     INFO - promise callback*next@chrome://mochikit/content/tests/SimpleTest/SpawnTask.js:104:45
00:12:04     INFO - onFulfilled@chrome://mochikit/content/tests/SimpleTest/SpawnTask.js:73:7
00:12:04     INFO - co/<@chrome://mochikit/content/tests/SimpleTest/SpawnTask.js:58:5
00:12:04     INFO - co@chrome://mochikit/content/tests/SimpleTest/SpawnTask.js:54:10
00:12:04     INFO - add_task/<@chrome://mochikit/content/tests/SimpleTest/SpawnTask.js:270:9
00:12:04     INFO - setTimeout handler*SimpleTest_setTimeoutShim@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:672:12
00:12:04     INFO - add_task@chrome://mochikit/content/tests/SimpleTest/SpawnTask.js:269:7
00:12:04     INFO - window.onload@chrome://mochitests/content/chrome/devtools/shared/security/tests/chrome/test_websocket-transport.html:28:3
00:12:04     INFO - EventHandlerNonNull*@chrome://mochitests/content/chrome/devtools/shared/security/tests/chrome/test_websocket-transport.html:14:1
Flags: needinfo?(amarchesini)
(Assignee)

Updated

7 months ago
Flags: needinfo?(amarchesini)

Comment 11

7 months ago
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ea39a16e4c97
Principal must always have a valid origin - part 1 - renaming GetOriginInternal to GetOriginNoSuffixInternal, r=qdot
https://hg.mozilla.org/integration/mozilla-inbound/rev/a303ac6e7f6b
Principal must always have a valid origin - part 2 - move OriginAttributes to the BasePrincipal, r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/918682c3ff30
Principal must always have a valid origin - part 3 - move origin to BasePrincipal, r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/7012be88341d
Principal must always have a valid origin - part 4 - origin passed as argument when a principal is created, r=bholley
https://hg.mozilla.org/integration/mozilla-inbound/rev/89b3b96c32fe
Principal must always have a valid origin - part 5 - fixing a comment in ContentPrincipalInfo, r=ehsan
https://hg.mozilla.org/integration/mozilla-inbound/rev/235e1d29916f
Principal must always have a valid origin - part 6 - fixing tests, r=ehsan

Comment 12

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ea39a16e4c97
https://hg.mozilla.org/mozilla-central/rev/a303ac6e7f6b
https://hg.mozilla.org/mozilla-central/rev/918682c3ff30
https://hg.mozilla.org/mozilla-central/rev/7012be88341d
https://hg.mozilla.org/mozilla-central/rev/89b3b96c32fe
https://hg.mozilla.org/mozilla-central/rev/235e1d29916f
Status: NEW → RESOLVED
Last Resolved: 7 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55

Updated

7 months ago
Depends on: 1351966
Depends on: 1352701
Depends on: 1353204
Depends on: 1357208

Updated

3 months ago
Depends on: 1384658

Updated

21 days ago
Depends on: 1403328
You need to log in before you can comment on or make changes to this bug.