gotwarlost / istanbul-middleware

Connect middleware for server side code coverage using istanbul
Other
179 stars 96 forks source link

Add validations if client posts incorrect coverage JSON to the middleware #12

Closed noomorph closed 10 years ago

noomorph commented 10 years ago

Hi, I got error when collecting client-side coverage.

The error happened because some files did not have any branches. So here I fix the case when coverage entry has no b property because Object.keys fails on undefined.

See error trace:

TypeError: Object.keys called on non-object
   at Function.keys (native)
   at computeBranchTotals (/Users/noomorph/Projects/myproject/node_modules/istanbul-middleware/node_modules/istanbul/lib/object-utils.js:127:16)
   at Object.summarizeFileCoverage (/Users/noomorph/Projects/myproject/node_modules/istanbul-middleware/node_modules/istanbul/lib/object-utils.js:214:24)
   at /Users/noomorph/Projects/myproject/node_modules/istanbul-middleware/lib/core.js:132:54
   at Array.forEach (native)
   at getTreeSummary (/Users/noomorph/Projects/myproject/node_modules/istanbul-middleware/lib/core.js:131:23)
   at Object.render (/Users/noomorph/Projects/myproject/node_modules/istanbul-middleware/lib/core.js:173:19)
   at /Users/noomorph/Projects/myproject/node_modules/istanbul-middleware/lib/handlers.js:47:14
   at Layer.handle [as handle_request] (/Users/noomorph/Projects/myproject/node_modules/express/lib/router/layer.js:76:5)
   at next (/Users/noomorph/Projects/myproject/node_modules/express/lib/router/route.js:100:13)
gotwarlost commented 10 years ago

Something is not right here. The file-coverage object is always supposed to have a b key since an empty object is set to the b property at the time of instrumentation.

Could you give me a JSON dump of the object it is trying to process when it fails? If you are not comfortable attaching it to the PR, please send it to me via email at kananthmail-github AT yahoo.com.

In short if this is a genuine bug, it needs to be fixed in istanbul and not in this module.

noomorph commented 10 years ago

Hi. You gave a hint where to search.

The problem was in my code that POSTed to /coverage/client/.

Before:

         $.ajax({
             type: 'POST',
             url: '/coverage/client',
             data: coverage,
             dataType: 'application/json',
             success: callback
         });

After:

        $.ajax({
            type: 'POST',
            url: '/coverage/client',
            data: JSON.stringify(__coverage__),
            processData: false,
            contentType: 'application/json; charset=UTF-8',
            success: callback
        });

So, Zepto was misconfigured and serialized the request in wrong way. That's completely my issue.

gotwarlost commented 10 years ago

I should probably add some checks for client-side posts since it is arbitrary user input. I will change the bug description and track it to completion. Thanks for reporting this!

gotwarlost commented 10 years ago

Oh crap. There is a PR attached. Will close and reopen a new one.