Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix exception instead of failure when parsing CallExpressions #12

Merged
1 commit merged into from
Sep 28, 2016

Conversation

deathcap
Copy link
Contributor

@deathcap deathcap commented Jan 9, 2016

Transforming parsers generated with jison with brfs would crash when static-eval attempted to parse this code:

var source = require('fs').readFileSync(require('path').normalize(x), 'utf8');

TypeError: callee.apply is not a function while parsing file: /tmp/b
    at walk (/Users/admin/games/voxeljs/static-eval/index.js:89:27)
    at walk (/Users/admin/games/voxeljs/static-eval/index.js:92:23)
    at walk (/Users/admin/games/voxeljs/static-eval/index.js:77:26)
    at walk (/Users/admin/games/voxeljs/static-eval/index.js:85:25)
    at module.exports (/Users/admin/games/voxeljs/static-eval/index.js:114:7)
    at traverse (/Users/admin/games/voxeljs/static-module/index.js:256:23)
    at walk (/Users/admin/games/voxeljs/static-module/index.js:208:13)
    at walk (/Users/admin/games/voxeljs/static-module/node_modules/falafel/index.js:49:9)
    at /Users/admin/games/voxeljs/static-module/node_modules/falafel/index.js:46:17
    at forEach (/Users/admin/games/voxeljs/static-module/node_modules/falafel/node_modules/foreach/index.js:12:16)

This PR fixes the crash, and correctly returns FAIL so it won't be transformed, as expected (it cannot be, since it is dynamic).


Smaller (but nonsensical) test case:

var source = require('fs').readFileSync(require('path'));

require() parses as not a function, but an object { resolve: …}, and then static-eval tries to call it, causing this exception. These other cases parse correctly (not transformed, expected):

var path = require('path').normalize(x);
var source = require('fs').readFileSync(path, 'utf8');

function f() { return '/tmp/foo'; }
var source = require('fs').readFileSync(f());

only readFileSync(require…) causes the crash, which this PR fixes (by gracefully failing instead).

deathcap added a commit to deathcap/node-mojangson that referenced this pull request Jan 10, 2016
The jison parser generator by default will generate a command-line
interface built into the parser. node-mojangson only uses the parser
object and not the command-line interface, and has no way to access it,
so it can be safely disabled -- since it causes compatibility issues
with some browserify transforms (and is unused), see:

zaach/jison#300
browserify/static-eval#12
@nfriedly
Copy link

nfriedly commented Jul 12, 2016

This bug has been biting me all morning too. I'm trying to run some tests in karma with browserify and brfs and kept getting "TypeError: Cannot read property 'apply' of undefined while parsing file..." and eventually tracked it down to that same section of code trying to run callee.apply(ctx, args) when callee is undefined

@ghost ghost merged commit 982db1b into browserify:master Sep 28, 2016
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants