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

mPrincipal is null when creating channel which updates ServiceWorker

RESOLVED INVALID

Status

()

Core
DOM: Security
RESOLVED INVALID
3 years ago
3 years ago

People

(Reporter: ckerschb, Assigned: ckerschb)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Updated

3 years ago
Assignee: nobody → mozilla
Blocks: 1137419
Status: NEW → ASSIGNED
(Assignee)

Comment 1

3 years ago
Created attachment 8588748 [details] [diff] [review]
bug_1151595.patch

Nikhil, in Bug 1137419 you slightly changed the behavior for creating a channel which updates a ServiceWorker. I agree with your change to use mPrincipal [1] but unfortunately ServiceWorkerRegisterJob has two constructors, one of them not initializing mPrincipal (see patch) - which is going to cause problems once we are going to move security checks of channels into AsyncOpen (which is about to happen soon).

Anwyay, the patch is just to highlight the problem, if you can tell me how to succeed, e.g. what principal to use I am going to finish the patch up. I don't think systemPrincipal is the right principal to use, but it would be great to set some other principal in the constructor so we have a nice separation of principals within the two (Register, and Update) constructors and we don't have to find a principal shortly before creating the channel in Update().

[1] http://mxr.mozilla.org/mozilla-central/source/dom/workers/ServiceWorkerManager.cpp#750
Attachment #8588748 - Flags: feedback?(nsm.nikhil)
Comment on attachment 8588748 [details] [diff] [review]
bug_1151595.patch

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

This code is going away in Bug 931249, but for now you can change it to |mRegistration->mPrincipal| instead of mPrincipal. This is always guaranteed to be set.
You can also get rid of the appcodebase block. Thanks!
Attachment #8588748 - Flags: feedback?(nsm.nikhil)
(Assignee)

Comment 3

3 years ago
Perfect - Bug 931249 landed this afternoon :-) which makes this bug invalid.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.