svnlabs / google-caja

Automatically exported from code.google.com/p/google-caja
0 stars 1 forks source link

handleUncaughtException is broken for non-DOM guests #1942

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What revision of the cajoler exhibits the problem?  On what browser and OS?
- Mac OS X
- R5702
- Chrome 38
- Firefox 34

Was working fine in r5687 on all browsers (Chrome 37 and below)

What steps will reproduce the problem?
1.  Pass in tamed object api and run
2.  Reference any tamed object in script
3.  Caja throws error such as "Cannot read property 'log' of undefined" for the 
example below
4.  gman.domicile in ses-single-frame.js is undefined and browser throws 
undefined error - line 26993, r5702 

 Q.when(promise, function (compiledFunc) {
      var result = undefined;
      try {
        result = compiledFunc(imports);
      } catch (e) {
        gman.domicile.handleUncaughtException(e, gman.getUrl());  <---- gman.domicile undefined - line 26993
      }
      if (opt_runDone) {
        opt_runDone(result);
      }
    }, function (failure) {
      config.console.log('Failed to load guest content: ' + failure);
    });

//// EXAMPLE CODE

            load: function (container, contentPath, contentType, content, api, uriPolicy)
            {

                var consoleService = {
                    log: function ()
                    {
                        console.log.apply(console, arguments);
                    }
                };

                caja.markReadOnlyRecord(consoleService);  
                caja.markFunction(consoleService.log);
                var tamedConsoleService = caja.tame(consoleService);

                var defer = $q.defer();
                setTimeout(doLoad, 0);
                return defer.promise;

                function doLoad()
                {
                    caja.load(undefined, undefined, function (frame)
                    {
                        defer.notify(frame);
                        var _api = { console: tamedConsoleService};
                        frame.code('/dummy.js',
                                contentType || 'application/javascript', 'console.log("1234")' )
                            .api(_api)
                            .run(function ()
                            {
                                defer.resolve();
                            }, function (err)
                            {
                                defer.reject(err);
                            });
                    });
                }
            }

Original issue reported on code.google.com by t...@xemware.com on 20 Oct 2014 at 7:55

GoogleCodeExporter commented 9 years ago
Firefox 33
Safari 8
IE 10+
Chrome 37 and below

Original comment by t...@xemware.com on 20 Oct 2014 at 7:57

GoogleCodeExporter commented 9 years ago
Caja init:

<script type="text/javascript" src="/client/caja/caja.js"></script>
<script type="text/javascript">
    caja.initialize({
        cajaServer: '/client/caja/',
        maxAcceptableSeverity: 'NO_KNOWN_EXPLOIT_SPEC_VIOLATION',
        debug: true,
        es5Mode: true
    }, null, function(err)
    {
        alert('Error');
    });
    caja.whenReady(function()
    {
        angular.module('lazy').makeLazy(angular.module('app'));
        angular.bootstrap(document,['app']);
    })
</script>

Original comment by t...@xemware.com on 20 Oct 2014 at 7:58

GoogleCodeExporter commented 9 years ago
I'll take a look.

If this is a new bug then it is likely to have been introduced in r5701, which 
created the domicile.handleUncaughtException method.

Original comment by kpreid@google.com on 20 Oct 2014 at 11:34

GoogleCodeExporter commented 9 years ago
Thanks Kevin.  

All I can say is it got introduced after r5687 as that is the previous version 
I was using that needed an upgrade for Chrome 38.

Original comment by t...@xemware.com on 20 Oct 2014 at 11:37

GoogleCodeExporter commented 9 years ago
I've prepared a fix for the problem with gman.domicile: 
https://codereview.appspot.com/157300043/

However, that doesn't explain the entire problem, because that would only 
trigger if there was an uncaught error (from the guest code) to report. I tried 
to reproduce the problem based on your provided code, but it ran successfully 
(I don't see the "Cannot read property 'log' of undefined" part of your error).

It would help if you could provide a self-contained example; that is, a single 
HTML file which loads Caja and triggers the error, and does not use Angular or 
Q libraries.

Original comment by kpreid@google.com on 21 Oct 2014 at 8:01

GoogleCodeExporter commented 9 years ago
Thanks Kevin, will do that today.

Original comment by t...@xemware.com on 21 Oct 2014 at 8:07

GoogleCodeExporter commented 9 years ago
Fix for headless onerror @r5703, leaving this issue open for further 
troubleshooting

Original comment by kpreid@google.com on 21 Oct 2014 at 9:06

GoogleCodeExporter commented 9 years ago
This works with 5687, broken in 5702+, don't know when it broke as jumped to 
5702.

Error:
Uncaught script error: Uncaught TypeError: console is undefined in source: 
"/dummy.js" at line: -1

<html>
    <head>
    </head>
    <body>

    <script type="text/javascript" src="/caja/caja.js"></script>
    <script type="text/javascript">
        caja.initialize({
            cajaServer: '/caja/',
            maxAcceptableSeverity: 'NO_KNOWN_EXPLOIT_SPEC_VIOLATION',
            debug: true,
            es5Mode: true
        }, null, function(err)
        {
            alert('Error');
        });
        caja.whenReady(function()
        {
            doTest();
        })

        function doTest()       
        {
                var consoleService = {
                    log: function ()
                    {
                        console.log.apply(console, arguments);
                    }
                };
        caja.markReadOnlyRecord(consoleService);  
                caja.markFunction(consoleService.log);
                var tamedConsoleService = caja.tame(consoleService);
        doLoad()
                function doLoad()
                {
                    caja.load(undefined, undefined, function (frame)
                    {
                        var _api = { console: tamedConsoleService};
                        frame.code('/dummy.js',  'application/javascript', 'console.log("1234")' )
                            .api(_api)
                            .run(function ()
                            {

                            }, function (err)
                            {

                            });
                    });
                }                       
        }
    </script>   
    </body>
</html>

Original comment by t...@xemware.com on 22 Oct 2014 at 1:29

GoogleCodeExporter commented 9 years ago
I found the problem in your code: .run() does not take two callback functions. 
It takes one callback function which is called whether or not there was an 
error. Change the code to pass only one function to .run() and it works.

Passing two arguments causes the first argument to be interpreted as your 
imports ("api") object (I believe this is compatibility for a deprecated 
caja.js API version), which is why your guest code found console == undefined.

(Closing this issue as fixed for the handleUncaughtException bug.)

Original comment by kpreid@google.com on 24 Oct 2014 at 9:26

GoogleCodeExporter commented 9 years ago
Thanks Kevin, appreciate your time.

That code is the same since I implemented a few months ago and was working fine 
with 5687, comparing the run() definition shows that r5687 supported three 
args, and that was removed after r5867.

I call that an API breaking change!

Original comment by t...@xemware.com on 25 Oct 2014 at 12:02

GoogleCodeExporter commented 9 years ago
OK, I'll take a closer look at the history of the API and see what we can do 
with the current version for maximum compatibility and usability.

Original comment by kpreid@google.com on 29 Oct 2014 at 6:31

GoogleCodeExporter commented 9 years ago
I wouldn't bother if no-one else is complaining, works fine now.

Original comment by t...@xemware.com on 29 Oct 2014 at 7:17