Skip to content
This repository has been archived by the owner on Feb 27, 2022. It is now read-only.

Broken on Node.js 8, 9, and Master #14

Closed
MylesBorins opened this issue Dec 5, 2017 · 9 comments
Closed

Broken on Node.js 8, 9, and Master #14

MylesBorins opened this issue Dec 5, 2017 · 9 comments

Comments

@MylesBorins
Copy link

MylesBorins commented Dec 5, 2017

Different reasons for each one (compounding)

8.x:

it can load native_module and module
  not ok TypeError: require(...).internalBinding is not a function

9.x:

it can load native_module and module
not ok ReferenceError: internalBinding is not defined

master:

test/basic.js ......................................... 4/7
  it can load native_module and module
  not ok SyntaxError: Unexpected token %
    stack: |
      SyntaxError: Unexpected token %
          runInThisContext (index.js:2:206)
          req_ (index.js:13:1586)
          require (index.js:13:1162)
          util.js:29:52
          req_ (index.js:13:1762)
          require (index.js:13:1162)
          module.js:25:14
          req_ (index.js:13:1762)
          Object.req [as require] (index.js:9:302)
          test/basic.js:34:11
    test: it can load native_module and module
    message: 'SyntaxError: Unexpected token %'

  fs is like fs but not fs
  not ok SyntaxError: Unexpected token %
    stack: |
      SyntaxError: Unexpected token %
          runInThisContext (index.js:2:206)
          req_ (index.js:13:1586)
          require (index.js:13:1162)
          util.js:29:52
          req_ (index.js:13:1762)
          require (index.js:13:1162)
          fs.js:29:14
          req_ (index.js:13:1762)
          Object.req [as require] (index.js:9:302)
          test/basic.js:46:21
    test: fs is like fs but not fs
    message: 'SyntaxError: Unexpected token %'

  support a whitelist
  not ok SyntaxError: Unexpected token %
    stack: |
      SyntaxError: Unexpected token %
          runInThisContext (index.js:2:206)
          req_ (index.js:13:1586)
          require (index.js:13:1162)
          util.js:29:52
          req_ (index.js:13:1762)
          require (index.js:13:1162)
          fs.js:29:14
          req_ (index.js:13:1762)
          Object.req [as require] (index.js:9:302)
          test/basic.js:60:20
    test: support a whitelist
    message: 'SyntaxError: Unexpected token %'

total ................................................. 4/7

I'm digging into the bugs regarding internalBinding, in the mean time it looks like the other breakages on master are due to nodejs/node#13295

Will see if I can come up with a patch

@MylesBorins
Copy link
Author

we should likely consider adding this module to CITGM to catch the breakages sooner

@addaleax
Copy link
Contributor

addaleax commented Dec 5, 2017

@MylesBorins But we don’t support this, and the README here is quite clear about that.

internalBinding is happening because the module wrappers for userland and built-in modules have diverged, and the Unexpected token % bug is happening because we start Node with the --allow_natives_syntax flags now.

A workaround for the latter could be calling v8.setFlagsFromString('--allow_natives_syntax'); before the script compilation and something like this:

https://github.com/nodejs/node/blob/591a24b819d53a555463b1cbf9290a6d8bcc1bcb/lib/internal/bootstrap_node.js#L429-L434

@MylesBorins
Copy link
Author

@addaleax while we don't support it this module breaking will break gulp, and likely give us a bit of a better error when we do so.

I'm open to either... and appreciate the sentiment that perhaps we shouldn't be officially supporting this with our infra

@addaleax
Copy link
Contributor

addaleax commented Dec 5, 2017

while we don't support it this module breaking will break gulp, and likely give us a bit of a better error when we do so.

Idk, we’re getting the same error message from our existing testing of gulp. 😄

@MylesBorins
Copy link
Author

MylesBorins commented Dec 5, 2017

Idk, we’re getting the same error message

we didn't catch the internalBinding error for at least 2 majors 😄

@addaleax
Copy link
Contributor

addaleax commented Dec 5, 2017

… which means gulp isn’t affected by that particular issue, and that is we care about in the end, right?

@isaacs
Copy link
Owner

isaacs commented Dec 5, 2017

@addaleax Thanks for the tip.

But even with that flag, looks like node's finally managed to kill the ability to load native modules in userland.

The reason is that, even if the % stuff can be fixed, process._internalBinding is required by most of the native module code as of Node 8. However, at bootstrap time, process._internalBinding is stashed in a local variable and then deleted from the process object, making it inaccessible before any userland code is loaded.

I am pretty sure that graceful-fs v3 cannot be made to work in Node.js 8 and beyond as a result. Gulp is broken until they upgrade to graceful-fs 4 or higher. This module (and thus, graceful-fs@3) is dead, and not coming back, unless Node.js reverses the approach of making everything unnecessarily locked down.

In my opinion, this excessive freezing is user-hostile. It does not improve security, makes the entire system less useful, and in fact can prevent several beneficial security approaches in userland code.

@joevandyk
Copy link

I'm running into this or similar issue. Using node 11.

$ yarn start                                          
yarn run v1.10.1
$ gulp server
[10:45:18] Failed to load external module @babel/register
[10:45:18] Requiring external module babel-register
fs.js:25
'use strict';
^

ReferenceError: internalBinding is not defined
    at fs.js:25:1
    at req_ (/Users/joe/projects/joe-hugo-blog/node_modules/natives/index.js:140:5)
    at Object.req [as require] (/Users/joe/projects/joe-hugo-blog/node_modules/natives/index.js:54:10)
    at Object.<anonymous> (/Users/joe/projects/joe-hugo-blog/node_modules/vinyl-fs/node_modules/graceful-fs/fs.js:1:37)
    at Module._compile (internal/modules/cjs/loader.js:707:30)
    at Module._extensions..js (internal/modules/cjs/loader.js:718:10)
    at Object.require.extensions.(anonymous function) [as .js] (/Users/joe/projects/joe-hugo-blog/node_modules/babel-register/lib/node.js:152:7)
    at Module.load (internal/modules/cjs/loader.js:605:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:544:12)
    at Function.Module._load (internal/modules/cjs/loader.js:536:3)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

@addaleax
Copy link
Contributor

@joevandyk You might want to check that your dependencies are up to date, in particular natives should be at version 1.1.6.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants