1143848 - Query Document from Parent in case of nested workers

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[Christoph Kerschbaumer [:ckerschb}]

When creating a channel for a worker we should either have a document [1] or we should put an assertion that the principal used to create the channel is the systemPrincipal.

[1] http://mxr.mozilla.org/mozilla-central/source/dom/workers/ScriptLoader.cpp#111

Comment 1

[Christoph Kerschbaumer [:ckerschb}]

Initially we thought we just have to query the document from the parent, in case we don't have one available. Altering the code for GetDocument() to something like the following:

> template <class Derived>
> nsIDocument*
> WorkerPrivateParent<Derived>::GetDocument() const
> {
>  AssertIsOnMainThread();
>  if (mLoadInfo.mWindow) {
>    return mLoadInfo.mWindow->GetExtantDoc();
>  }
>  // if we don't have a document, we should get the document from the parent
>  WorkerPrivate* parent = mParent;
>  while (parent) {
>    if (parent->mLoadInfo.mWindow) {
>      return parent->mLoadInfo.mWindow->GetExtantDoc();
>    }
>    parent = parent->GetParent();
>  }
>  // couldn't query a document, give up and return nullptr
>  return nullptr;
}


Apparently the parent is also null in this cases and we can not query a document from the parent either.
One of the problems is here, where we set the parent to 'null':
  http://mxr.mozilla.org/mozilla-central/source/dom/workers/WorkerPrivate.cpp#4280

Jonas, any ideas how we could proceed and fix?

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

The description here says "private worker", but comment 0 just says "worker".  Can you elaborate on what this means?

Do you just want to check for the existence of a document or do you actually need to actively call methods on the document?

This would also cause you troubles as it clears mWindow of any worker that has finished loading:

  http://mxr.mozilla.org/mozilla-central/source/dom/workers/WorkerPrivate.cpp#3711

Comment 3

[Christoph Kerschbaumer [:ckerschb}]

Just chatted with Jonas on IRC; we think that adding an assertion that the principal is the systemPrincipal when creating the channel *could* be added for:
a) dedicated workers [yes]
b) shared workers [mabye]
c) service workers [probably not]

Comment 4

[Christoph Kerschbaumer [:ckerschb}]

(In reply to Ben Kelly [:bkelly] from comment #2)
> The description here says "private worker", but comment 0 just says
> "worker".  Can you elaborate on what this means?

Sorry for the confusion, I think comment 3 should answer your question.

> Do you just want to check for the existence of a document or do you actually
> need to actively call methods on the document?

We would pass the document along to be set on the 'loadInfo of the channel', which would happen here:
http://mxr.mozilla.org/mozilla-central/source/dom/workers/ScriptLoader.cpp#112

> This would also cause you troubles as it clears mWindow of any worker that
> has finished loading:

Agreed, but our usecase is slightly different. And the loadInfo would keep the reference to the window.

Comment 5

[Christoph Kerschbaumer [:ckerschb}]

Attachment: bug_1143848_workers.patch

Comment 6

[Christoph Kerschbaumer [:ckerschb}]

We decided it would be great to have the document in case of a nested worker in which case updating ::GetDocument() is what we want in the end.

Comment 7

[Christoph Kerschbaumer [:ckerschb}]

https://hg.mozilla.org/integration/mozilla-inbound/rev/73655764d8b4

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/73655764d8b4