elastic / require-in-the-middle

Module to hook into the Node.js require function
MIT License
168 stars 26 forks source link

fix: hooking "http2" with node 8.6 and "--expose-http2" would hit an assert #68

Closed trentm closed 1 year ago

trentm commented 1 year ago

For earlier versions of node -- before Module.isBuiltin() -- we use require('resolve').isCore(name) to determine if a module is built-in, a.k.a. is a "core" module. With https://github.com/elastic/require-in-the-middle/pull/63 an assert was added to the caching logic if there was a surprise -- a core=false module that isn't cached in require.cache.

This missed a case where this can happen: with the built-in 'http2' module in older versions of Node.js v8 when it was behind the --expose-http2 flag. The result would be a crash like this:

% node --expose-http2 http2-is-builtin.js
TAP version 13
# using http2
assert.js:41
  throw new errors.AssertionError({
  ^

AssertionError [ERR_ASSERTION]: unexpected that there is no Module entry for "http2" in require.cache
    at ExportsCache.set (/Users/trentm/el/require-in-the-middle4/index.js:72:7)
    at Module.Hook._require.Module.require (/Users/trentm/el/require-in-the-middle4/index.js:232:17)
    at require (internal/module.js:11:18)
    at Test.<anonymous> (/Users/trentm/el/require-in-the-middle4/test/http2-is-builtin.js:15:13)
    at Test.bound [as _cb] (/Users/trentm/el/require-in-the-middle4/node_modules/tape/lib/test.js:85:17)
    at Test.run (/Users/trentm/el/require-in-the-middle4/node_modules/tape/lib/test.js:104:7)
    at Test.bound [as run] (/Users/trentm/el/require-in-the-middle4/node_modules/tape/lib/test.js:85:17)
    at Immediate.next (/Users/trentm/el/require-in-the-middle4/node_modules/tape/lib/results.js:151:7)
    at runCallback (timers.js:781:20)
    at tryOnImmediate (timers.js:743:5)

The resolve module doesn't consider a module to be "core" if it is still behind a flag. (The discussion for http2: https://github.com/browserify/resolve/issues/139)

% node86 -e 'console.log(process.version, require("resolve").isCore("http2"))'
v8.6.0 false

% node8 -e 'console.log(process.version, require("resolve").isCore("http2"))'
v8.17.0 true

% node16 -e 'console.log(process.version, require("resolve").isCore("http2"))'
v16.19.0 true

The solution here is to special case 'http2' handling for those versions of node. If there were a lot of "behind a flag core node modules", then a more general fix would be nice, but I think this is the only one.