Skip to content

Conversation

@fabiosantoscode
Copy link

Since binary extensions can do anything they please, return null when requiring them.

I haven't tested it, but according to the docs a file is only loaded as a binary extension if it's a .node file.

Since binary extensions can do anything they please, return null when requiring them.
@ryzokuken
Copy link
Member

@fabiosantoscode that seems fairly straightforward also. Would you like to make a PR for that?

@ryzokuken
Copy link
Member

Oh wait, my bad.

Copy link
Member

@ryzokuken ryzokuken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fair, thanks for the PR. I was thinking about this, and since we’re throwing the file into vm.compileFunction, it makes no sense to accept files which aren’t valid JavaScript files. Can you change the workflow so that if the extension isn’t a JavaScript extension (.js and .mjs, I guess), it bails to the standard require method?

@ryzokuken
Copy link
Member

Now that I think about it, forbidding requiring something with the current codebase is a bad idea since secureRequire takes over the require chain in the submodules and this would break the code if any of the submodules require anything which can’t be handled with secureRequire. For this reason, secureRequire shouldn’t throw on weird file types but just bail to standard require (which is what the user would try anyway).

If we do want to throw something, we should only throw it on the first level (direct requires).

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