1347817 - Principal should always have a valid origin

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Description

[Andrea Marchesini [:baku]]

Initially these patches were part of bug 1340163 but it's better that they have a own bug.
See comments in bug 1340163.

Comment 1

[Andrea Marchesini [:baku]]

Attachment: part 0 - renaming GetOriginInternal to GetOriginNoSuffixInternal

This patch has been already reviewed in bug 1340163 by qdot.

Comment 2

[Andrea Marchesini [:baku]]

Attachment: part 1 - Setting OriginAttributes in FinishInit()

Patch reviewed by bholley

Comment 3

[Andrea Marchesini [:baku]]

Attachment: part 2 - originNoSuffix in FinishInit

Patch reviewed by bholley

Comment 4

[Andrea Marchesini [:baku]]

Attachment: part 3 - Origin passed as argument when a principal is created

already reviewed by bholley

Comment 5

[Andrea Marchesini [:baku]]

Attachment: part 4 - ContentPrincipalInfo - comment fixed

Comment 6

[Andrea Marchesini [:baku]]

Attachment: part 5 - fix tests

This is a WIP. There is still 1 broken test.

Comment 8

[(no longer active)]

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?

Comment 9

[Pulsebot]

Pushed by [e-mail]:
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

Comment 10

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

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

Comment 11

[Pulsebot]

Pushed by [e-mail]:
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

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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