Breaks on bundled apps

[rileyjshaw]

Hi @asvd!

I'm trying to get Jailed running client-side using a Browserify bundle. I'm getting the following error when I attempt to require('jailed'):

http://localhost:8000/_JailedSite.js: Failed to load resource: the server responded with a status of 404 (Not Found)

Jailed seems to make assumptions about my file structure which don't hold true on a bundled app.

[asvd]

I did not test it with Browserify actually. Let's try to make it work :-)

1. Where do you put the bundle? (the path on your local http server)
2. Do you also provide other files shipped with Jailed next to the bundle? _JailedSite.js particularly, I beleive it is not bundled, since it's loaded on the fly.

[BerkeleyTrue]

I am putting my bundle in public/js. I can see that _JailedSite is indeed bundle with the app.

Currently I am manually copying _JailedSite over to public/js to get this to work, but that defeats the purpose of using browserify.

I don't see a clear path in order to make this work flawlessly with react. It could be possible to use fs.readFileSync with a brfs to get access to the raw script. Then whether detect window and require are undefined. That should tell you if you are in bundle environment.

then create a script element and append result from fs.readFileSync to the script tag. Then work as usual.

What do you think?

[asvd]

_JailedSite.js contains common code shared between the application and every plugin worker where you put the untrusted code. That means that even if you reuse the bundled code of _JailedSite in the project, the browser will load the file anyway on the worker's site upon a plugin initialization.

Thus from my perspective _JailedSite.js is not supposed to be bundled. Which means you should exclude it from a list of bundled files, and copy _JailedSite.js using the build-script, along with bundling everything else with browserify.

Please let me know if I miss some of your points.

[BerkeleyTrue]

I am not sure what you mean. From here looks like the code is setting the path as the src from a script tag.

My proposal would be to detect bundling and use fs.readFileSync to read the _JailedSite.js file into a string. Then, instead of setting src to a path, you setInnerHTML to the return from readFile. The result is the same. This is all in theory, though.

Having everything work in a bundle is a nice feature but not necessarily a must have. To close this issue, it seems like these files

_frame.html
_frame.js
_JailedSite.js
_pluginCore.js
_pluginWeb.js

Need to be included in the same directory that the bundled file is located in order to get Jailed to run code.

[asvd]

My proposal would be to detect bundling and use fs.readFileSync to read the _JailedSite.js file into a string.

This makes no sence. See what would happen then:

• I add fs.readFileSync instead of loading the _JailedSite.js on the fly, as you suggest;
• upon bundling, brfs recognizes the call to fs.readFileSync and substitutes the content of _JailedSite.js right into the bundled script, simply as a string;
• Then you launch the application, and the code of _JailedSite.js is substituted to the script tag and executed, so it kind of works, but:
• When you create a Plugin instance for running an untrusted code, a web-worker is created;
• The worker then loads some additional code from other files, including _JailedSite.js, and the browser still downloads _JailedSite.js

Therefore you load the same code of _JailedSite.js twice: first as a string inside the bundle, and second as a separate file for the web-worker of the Plugin. Such kind of unnecessary network overhead goes a little bit against the idea of bundling the project (in order to reduce the network overhead).

So still my suggestion is that you simply put all the files you mentioned next to the bundle. You may consider that it does not probably look very neat, but it provides better performance instead.

[BerkeleyTrue]

got it. It is working as you suggested. I would add a note in the README so that others using it in a bundle know the proper way to get it working.

[asvd]

Sure, will add it shortly, thanks for the contribution :-)

What do you think: are there any other tricky things about Jailed to be mentioned considering bundling, in addition to "bundle jailed.js, put everything else next to the bundle"?

Some additional thoughts: I'm thinking of that it is theoretically possible to put everything into jailed.js, including the code only needed for plugin, simply as a string. Then the common part (what is now in _JailedSite.js) can be evaluated on place, and additionally transfered to the plugin worker (along with other code needed for it to work, without loading it from separate files). I think that would be the perfect solution for you (meaning that all the code is loaded once and contained in a single file, kind of a "pure bundle"). But this is what cannot be easily done with Jailed, because that would require make significant changes to its structure, and finally the code will be strongly messed-up. And specially I do not like the idea of having the code in a string, even if it's inside a bundle. So I think I would never do it.

[BerkeleyTrue]

I am still having issues getting this to work.

It is working up to the point where I try to call a remote method from within the plugin.

I have a callback

  // cb is defined above
  var api = {
    callback: function(err, data) {
      if (err) { return cb(err); }
      data.input = removeLogs(data.input);
      cb(null, data);
    }
  };

Within the webworker I call it

application.remote.callback(err, data);

It then hits this route https://github.com/asvd/jailed/blob/master/lib/_pluginWeb.js#L86 And error with self undefined. It seems to hit this point before the call to remote and it works fine. It seems like whatever is calling it before is in the wrong context or overwrites self (don't see where self is defined).

[BerkeleyTrue]

NVM I found the issue.

What do you think: are there any other tricky things about Jailed to be mentioned considering bundling, in addition to "bundle jailed.js, put everything else next to the bundle"?

Nope, just that and it works :+1: Using something like uglifyify to remove deadcode should take care of stuff being bundled and not used.

[asvd]

self is a global scope inside a web-worker, like window on a web-page. Probably that means that the given code runs outside a worker somehow. Not sure if this is related to bundling the code..

[BerkeleyTrue]

Thats good to know. I am new to web workers.

Seems that I was setting self to null inside a function instead of creating a new var self equal to null. It is running dandy now.

[gravitypersists]

+1 - hit this issue with browserify and I can't imagine it's going to go over smoothly with webpack either.

[asvd]

@gravitypersists have you faced any other complications in addition to what was discussed here?

[gravitypersists]

Not much. jailed is a pretty good piece of work :+1: I've resorted to including _JailedSite at my root, but it makes my projects a bit less portable as now every one of them needs to do this as well.

I'm doing some pretty weird stuff over here as well. Essentially, what I want is for the chai lib to be available within the jailed sandbox, but the api object isn't exactly the same code I declared in my main app code.

I'm presuming this is because there is some async stuff that occurs with messaging? Or is there an easier way to let the parent define the lib and allow the sandbox access?

[asvd]

I've resorted to including _JailedSite at my root, but it makes my projects a bit less portable as now every one of them needs to do this as well.

yep, that's not very neat thing about jailed. As mentioned, alternative for me would be to copy the code of JailedSite for both application and sandbox, and therefore have it loaded twice. Maybe I should really go that way...

the api object isn't exactly the same code I declared in my main app code

Jailed does not provide the pure exporting:

https://github.com/asvd/jailed#usage

what you can only do, is export the particular asynchronous functions. Then you can transfer the objects as function arguments, but the objects should also be simple enough, since they will be serialized. For you that means that the transfered objects should not be instances of something complicated, and should not include functions.