557663 - give add-ons access to their own bundled resources

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

It should be possible for add-ons to access resources bundled with the add-on package via a high-level API that lets them both read the content of resources into memory and generate a URL pointing to the resource that they can pass to URL-accepting APIs.

Comment 1

[Brian Warner [:warner :bwarner]]

Here's the plan:

Add-on code (i.e. any package that gets bundled with "cfx xpi") can do the
following:

 var resources = require("self").data
 var foo = resources.load("foo.txt")
 Panel({source: resources.url("main.html")})
 Menu({name:"first", icon:resources.url("first.png")})

If this code is loaded from PKGROOT/lib/main.js , then these "resources"
calls will reference PKGROOT/data/foo.txt (and PKGROOT/data/main.html and
PKGROOT/data/first.png). The load() method will return a string (or perhaps a
bytearray??). The url() method will return a URL instance that looks like:

 resource://jetpack/$JID/main.html

in which $JID is the unique identifier defined in JEP118.

We will use NSIContentPolicy to add a rule that says
resource://jetpack/JID1/* can only be loaded by content frames (or other
contexts) whose "origin" starts with the same resource://jetpack/JID1/
prefix. We will use this rule to make PKGROOT/data/ private to the code
inside PKGROOT/lib/ . We will be careful to control the origin attached to
content frames/etc that we create in the future, and only attach JID1 to a
frame that is created by code from JID1.

(note: this is a violation of obj-cap discipline, and opens us up to a
confused-deputy attack, wherein JID1's code might open a frame with contents
that come from somewhere else, and a script inside those contents may thus
acquire access to JID1's resources without an explicit grant. However, I'm
told that most of the internal APIs are designed to accept URLs, and URLs are
generally Universal except for the NSIContentPolicy check, so this may be the
best we can do for now.)

The intention is that this resource://jetpack/JID1 space is purely for use by
the JID1 package. Other add-ons and their packages should not expect to be
able to access this space. Web content from http URLs should certainly not be
able to access it. We will add resource publishing later, probably in the
form of a separate URL space (resource://jetpack-publish/JID1 ?), under which
requests are routed to a JS handler object, registered with something like
"require("url-publish").registerPublisher(publisher)" .

This mechanism may not be secure to start with: until we get the
NSIContentPolicy in place, all add-ons will be able to access each other's
data. But because we intend for it to become private eventually, we're
calling it "data" instead of "public-data", and treating the initial
lack-of-privacy as a bug, that will be fixed.

I'll be updating JEPs 106, 115, 117, and 118 to reflect this approach.
Incidentally, require("self").id is how the JEP118 unique-id will be
retrieved by add-on code.

Comment 2

[Atul Varma [:atul]]

Note that the last part of comment 1--"Incidentally, require("self").id is how the JEP118 unique-id will be retrieved by add-on code"--is basically bug 548870.

Comment 3

[Brian Warner [:warner :bwarner]]

I've been thinking about the right sort of access control for bundled
resources. This is highly influenced by our current packaging scheme, which
may (hopefully) change in the future. My current understanding of the way
we're building Jetpack XPI files is like this:

* a "package" is a directory that the SDK searches for "modules". The SDK
  currently ships with four packages, all in SDKROOT/packages/ :
  jetpack-core, development-mode, nsjetpack, and test-harness. The
  "jetpack-core" package is the most important one: it contains all the
  various APIs we're making available to Jetpack developers.

* A "module" is a single JS file, like packages/jetpack-core/lib/url.js ,
  and is loaded in a nominally-isolated sandbox, so it only gets access
  to a limited set of globals that are passed in by the loader.
  "Chrome modules", such as the ones in jetpack-core/, are given access
  to Components.classes and other powers, but others are not (or at
  least are not supposed to have access, and eventually we'll figure
  out how to take it away from them).

* The developer will create another package by making a new directory and
  populating it with a "package.json" file (and lib/, docs/, and /data
  subdirectories). They'll run the "cfx" tool from this directory, and the
  SDK will search it for modules just as it would any other package. The
  developer's package is nominally the top-level "main" piece. Eventually
  there will be third-party packages, not included in the SDK, which can be
  incorporated into add-ons.

* A package is the smallest unit of linking: if the SDK determines that it
  needs any module from package A, then it will include all modules from
  package A in the generated XPI.

* A package is also the smallest unit of resource bundling, because the
  $PACKAGE/data/ directory is shared between all modules in a package (i.e.
  there is no jetpack-core/data/url/foo.png or jetpack-core/url/data/foo.png
  but there could be jetpack-core/data/foo.png).

As a result, I think the best behavior for require("self").data("foo.png") is
to look for "data/foo.png" in the package that contains the module which did
the require() call. Two modules in the same package will be isolated from
each other (in terms of JS objects), but will be able to access the same
common bundled data.

The JID is common to the entire XPI: various packages are included in each
XPI, and there is not yet a concept of a per-package ID. This means that any
included module which uses (and is allowed to use) Simple Storage will be on
equal footing: top-level add-on code is vulnerable to misbehavior by packages
it includes, if those packages require() the Simple Storage module and we
don't do anything special in the loader to prevent that.

Having the JID be scoped to something larger than the package makes the
question of what URLs to assign more interesting. A URL which starts with
"resource://jetpack/$JID/" should certainly point to something in the XPI
bundle, but which package? "resource://jetpack/$JID/$packagename/" could be
used to narrow the scope to the resources included in a specific package.
What are the access rules? Ideally the URL that is returned is a proper
capability: if you know it, you can access the resource, and if you don't,
you can't, and you can give it to someone else to give them access.

I think we can start by saying that all the code in the XPI bundle can use
these URLs (I think that's the easiest thing to implement anyways). This
makes it hard for the top-level add-on code to hide its own bundled data from
included packages (at least from packages that are allowed to access
resource: URLs at all), but if that turns out to be a problem, we could
consider unguessable URL components.

Also, we should probably distinguish this part of the URL space from other
likely uses, which suggests we should have "data" in the URL somewhere. So
maybe "resource://jetpack/$JID/data/$packagename/$filename", with an eventual
NSIContentPolicy to limited access to other modules from the same bundle
(i.e. same $JID, even if the $packagename is different).

Comment 4

[Brian Warner [:warner :bwarner]]

Attachment: adds "self" module, docs and tests, and an example add-on

Here's a patch that should provide the API described earlier. The main difference is that the URLs it provides put all of the information in the "host" section (separated with hyphens instead of slashes), because that requires less code than writing a full-size resource: handler. Data access is scoped to the package that calls "self.data.load()" or "self.data.url()": the stack introspection is done in load/run. This will need to change, to be done at the time of require(), so that code in one package can correctly pass its 'data' object to code in a different package and have it work correctly.

And of course the JIDs are not well-formed yet. Add-ons which do not provide an "id" property in their package.json file will get a randomly-generated non-persistent one (when building an XPI), or a default test-harness-supplied value (when running "cfx test").

Comment 5

[Atul Varma [:atul]]

Thanks!

observations:

* I appreciate the comments on why stack introspection isn't ideal.

* it would be great if the example package/addon could call
  callbacks.quit() after it displayed the necessary information; this
  way, it'd be easy for us to write a test harness that executed it
  as an XPI until it terminated, and verified its actual output
  against its expected output.

* we should probably make a bug for adding the execution of
  the new example add-on--and any other example ones in the
  'examples' directory--to the sdk test suite (either 'cfx test' or
  'cfx testall').

* we should probably note in the docs that the Panel object doesn't
  yet exist, possibly linking to the JEP and/or the bug--otherwise
  people might start thinking they can make Panels! This might be
  good for a sidenote.

* 'self' refers to the things being created as addons, but the rest of
  the sdk documentation tries instead to refer to 'jetpack programs',
  in part because the sdk is actually agnostic re: whether it's
  building addons or apps. It'd be nice to use similar terminology
  unless the module is indeed addon-specific...

nits:

* it'd be nice to link "jep 118" to the jep's location on the wiki.

* a sidenote to the 'package specification' appendix alongside the
    data() docs would be nice.

Comment 6

[Brian Warner [:warner :bwarner]]

Attachment: adds "self" module, docs and tests, and an example add-on

New patch with most of your comments addressed:

> it would be great if the example package/addon could call
> callbacks.quit() after it displayed the necessary information

I tried that, copying the sample I found in
packages/test-harness/lib/run-tests.js, but when I ran 'cfx test' from
examples/reading-data, my 'exports.main = function (options, callbacks)'
failed with a 'callbacks is undefined' exception. I might be missing
something, but it feels like this 'callbacks' feature doesn't exist yet.

> we should probably note in the docs that the Panel object doesn't yet exist

Got it. I feel uneasy adding links to unstable URLs (the JEPs are all under
/Jetpack/Reboot/JEP/NNN right now), and adding notes to docs that will become
stale in the very next release, but it's a good warning to provide. I also
added a note to the main.js code.

> 'self' refers to the things being created as addons, but the rest of
> the sdk documentation tries instead to refer to 'jetpack programs',

I added a brief note at the start of the docs for this. I cannot yet
understand how non-addons access data or get an ID value.. I'll update this
module once we get a better handle on how non-addons might use "self".

> it'd be nice to link "jep 118" to the jep's location on the wiki.

Done, same uneasiness about Reboot/JEP/118 as above. We'll need to change the
links once we reorganize the wiki.

> a sidenote to the 'package specification' appendix alongside the
> data() docs would be nice.

Done.

Comments not addressed:

* create a bug for executing the example add-on: we should talk about how
  real add-ons should be written to enable testing. I kind of suspect that
  most top-level add-on functionality doesn't lend itself to easy unit
  testing, and I wanted to have a somewhat realistic example.


If this passes your review, I'll land it today in time for the 0.3 freeze.

Comment 7

[Atul Varma [:atul]]

(In reply to comment #6)
> > it would be great if the example package/addon could call
> > callbacks.quit() after it displayed the necessary information
> 
> I tried that, copying the sample I found in
> packages/test-harness/lib/run-tests.js, but when I ran 'cfx test' from
> examples/reading-data, my 'exports.main = function (options, callbacks)'
> failed with a 'callbacks is undefined' exception. I might be missing
> something, but it feels like this 'callbacks' feature doesn't exist yet.

Oh, actually, I meant that the example should just be a normal "jetpack program" which prints some stuff out to the console and then exits, and that is invoked via "cfx run" rather than "cfx test". A higher-level test harness in Python would then examine the console output and compare it to some expected result. Though I suppose that the jetpack program could also just assert things itself and do callbacks.quit("FAIL") if anything is amiss.

"cfx test" is actually a specialized version of "cfx run", which says "run the jetpack program in the package 'test-harness', include as a dependency the current working directory's package (and its dependencies), and also include all test directories". So in other words, "cfx test" is meant specifically for running the unit tests that come with a package, rather than running a package itself. Its downside is that it can't really be used to test the packaging infrastructure and the things the "self" module is trying to test, because some of the results of the "self" module will actually be relevant to the 'test-harness' package--which is actually the "jetpack program" that is being executed--rather than the package being tested.

I hope that makes some sense. It is complicated.

> Got it. I feel uneasy adding links to unstable URLs (the JEPs are all under
> /Jetpack/Reboot/JEP/NNN right now), and adding notes to docs that will become
> stale in the very next release, but it's a good warning to provide. I also
> added a note to the main.js code.

Yeah, makes sense--we should also probably add tests that spider the content of the docs and make sure none of them point to 404's and other "broken" things.

> I added a brief note at the start of the docs for this. I cannot yet
> understand how non-addons access data or get an ID value.. I'll update this
> module once we get a better handle on how non-addons might use "self".

Oh, I think some of this may ultimately involve changes in cfx. Not high-priority, but we should briefly discuss it sometime.

> * create a bug for executing the example add-on: we should talk about how
>   real add-ons should be written to enable testing. I kind of suspect that
>   most top-level add-on functionality doesn't lend itself to easy unit
>   testing, and I wanted to have a somewhat realistic example.

Makes sense, and I expect we'll want to loop in the MozMill folks here, since that kind of functional/behavioral testing is exactly what MozMill was made for.

This looks good! Feel free to land it.

Comment 8

[Brian Warner [:warner :bwarner]]

great! landed in ec66494b277d

Comment 10

[Myk Melez [:myk] [@mykmelez]]

The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".