From bb8a89c5bfc97ea5be76458a6a489f329b3bb091 Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Mon, 15 Jul 2024 10:39:53 +0200 Subject: [PATCH 1/4] feat: support process.getBuiltinModule --- index.js | 42 +++++++++++++++-- test/process-getbuiltinmodule.test.js | 65 +++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 4 deletions(-) create mode 100644 test/process-getbuiltinmodule.test.js diff --git a/index.js b/index.js index 966b9c9..a6683b7 100644 --- a/index.js +++ b/index.js @@ -122,6 +122,7 @@ function Hook (modules, options, onrequire) { this._unhooked = false this._origRequire = Module.prototype.require + this._origGetBuiltinModule = process.getBuiltinModule const self = this const patching = new Set() @@ -139,6 +140,24 @@ function Hook (modules, options, onrequire) { return self._origRequire.apply(this, arguments) } + return patchedRequire.call(this, arguments, false) + } + + this._getBuiltinModule = process.getBuiltinModule = function (id) { + if (self._unhooked === true) { + // if the patched process.getBuiltinModule function could not be removed because + // someone else patched it after it was patched here, we just abort and pass the + // request onwards to the original process.getBuiltinModule + debug('ignoring process.getBuiltinModule call - module is soft-unhooked') + return self._origGetBuiltinModule.apply(this, arguments) + } + + return patchedRequire.call(this, arguments, true) + } + + // Preserve the original require/process.getBuiltinModule arguments in `args` + function patchedRequire (args, coreOnly) { + const id = args[0] const core = isCore(id) let filename // the string used for caching if (core) { @@ -151,6 +170,11 @@ function Hook (modules, options, onrequire) { filename = idWithoutPrefix } } + } else if (coreOnly) { + // `coreOnly` is `true` if this was a call to `process.getBuiltinModule`, in which case + // we don't want to return anything if the requested `id` isn't a core module + debug('call to process.getBuiltinModule with unknown built-in id - returning undefined') + return undefined } else { try { filename = Module._resolveFilename(id, this) @@ -164,7 +188,7 @@ function Hook (modules, options, onrequire) { // where `@azure/functions-core` resolves to an internal object. // https://github.com/Azure/azure-functions-nodejs-worker/blob/v3.5.2/src/setupCoreModule.ts#L46-L54 debug('Module._resolveFilename("%s") threw %j, calling original Module.require', id, resolveErr.message) - return self._origRequire.apply(this, arguments) + return self._origRequire.apply(this, args) } } @@ -185,7 +209,9 @@ function Hook (modules, options, onrequire) { patching.add(filename) } - const exports = self._origRequire.apply(this, arguments) + const exports = coreOnly + ? self._origGetBuiltinModule.apply(this, args) + : self._origRequire.apply(this, args) // If it's already patched, just return it as-is. if (isPatching === true) { @@ -288,11 +314,19 @@ function Hook (modules, options, onrequire) { Hook.prototype.unhook = function () { this._unhooked = true + if (this._require === Module.prototype.require) { Module.prototype.require = this._origRequire - debug('unhook successful') + debug('require unhook successful') + } else { + debug('require unhook unsuccessful') + } + + if (this._getBuiltinModule === process.getBuiltinModule) { + process.getBuiltinModule = this._origGetBuiltinModule + debug('process.getBuiltinModule unhook successful') } else { - debug('unhook unsuccessful') + debug('process.getBuiltinModule unhook unsuccessful') } } diff --git a/test/process-getbuiltinmodule.test.js b/test/process-getbuiltinmodule.test.js new file mode 100644 index 0000000..00c9884 --- /dev/null +++ b/test/process-getbuiltinmodule.test.js @@ -0,0 +1,65 @@ +'use strict' + +const test = require('tape') + +const { Hook } = require('../') + +// The `process.getBuiltinModule(id) function was added in Node.js 22.3.0 +const skip = !process.getBuiltinModule + +test('process.getBuiltinModule should be patched', { skip }, function (t) { + let numOnRequireCalls = 0 + + const hook = new Hook(['http'], function (exports, name, basedir) { + numOnRequireCalls++ + return exports + }) + + const a = process.getBuiltinModule('http') + t.equal(numOnRequireCalls, 1) + + const b = require('http') + t.equal(numOnRequireCalls, 1) + + t.strictEqual(a, b, 'modules are the same') + + t.end() + hook.unhook() +}) + +test('patched process.getBuiltinModule should work with node: prefix', { skip }, function (t) { + let numOnRequireCalls = 0 + + const hook = new Hook(['http'], function (exports, name, basedir) { + numOnRequireCalls++ + return exports + }) + + process.getBuiltinModule('node:http') + t.equal(numOnRequireCalls, 1) + t.end() + hook.unhook() +}) + +test('patched process.getBuiltinModule should preserve default behavior for non-builtin modules', { skip }, function (t) { + const beforePatching = process.getBuiltinModule('ipp-printer') + + const hook = new Hook(['ipp-printer'], function (exports, name, basedir) { + t.fail('should not call hook') + }) + + const afterPatching = process.getBuiltinModule('ipp-printer') + + t.strictEqual(beforePatching, afterPatching, 'modules are the same') + t.end() + hook.unhook() +}) + +test('hook.unhook() works for process.getBuiltinModule', { skip }, function (t) { + const hook = new Hook(['http'], function (exports, name, basedir) { + t.fail('should not call onrequire') + }) + hook.unhook() + process.getBuiltinModule('http') + t.end() +}) From eb50f6d8849f2ae137370ebd30437cbd0fd8cf09 Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Fri, 19 Jul 2024 13:11:02 +0200 Subject: [PATCH 2/4] Address review comments --- README.md | 3 ++- index.js | 29 ++++++++++++++++------------- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 9aafd73..54fea71 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,7 @@ modules on-the-fly as they are being required. [![npm](https://img.shields.io/npm/v/require-in-the-middle.svg)](https://www.npmjs.com/package/require-in-the-middle) [![Test status](https://github.com/elastic/require-in-the-middle/workflows/Test/badge.svg)](https://github.com/elastic/require-in-the-middle/actions) +Also supports hooking into calls to `process.getBuiltinModule()`, which was introduced in Node.js v22.3.0. ## Installation @@ -72,7 +73,7 @@ argument). ### `hook.unhook()` Removes the `onrequire` callback so that it will not be triggerd by -subsequent calls to `require()`. +subsequent calls to `require()` or `process.getBuiltinModule()`. ## License diff --git a/index.js b/index.js index a6683b7..f4f520c 100644 --- a/index.js +++ b/index.js @@ -122,7 +122,6 @@ function Hook (modules, options, onrequire) { this._unhooked = false this._origRequire = Module.prototype.require - this._origGetBuiltinModule = process.getBuiltinModule const self = this const patching = new Set() @@ -143,16 +142,19 @@ function Hook (modules, options, onrequire) { return patchedRequire.call(this, arguments, false) } - this._getBuiltinModule = process.getBuiltinModule = function (id) { - if (self._unhooked === true) { - // if the patched process.getBuiltinModule function could not be removed because - // someone else patched it after it was patched here, we just abort and pass the - // request onwards to the original process.getBuiltinModule - debug('ignoring process.getBuiltinModule call - module is soft-unhooked') - return self._origGetBuiltinModule.apply(this, arguments) - } + if (typeof process.getBuiltinModule === 'function') { + this._origGetBuiltinModule = process.getBuiltinModule + this._getBuiltinModule = process.getBuiltinModule = function (id) { + if (self._unhooked === true) { + // if the patched process.getBuiltinModule function could not be removed because + // someone else patched it after it was patched here, we just abort and pass the + // request onwards to the original process.getBuiltinModule + debug('ignoring process.getBuiltinModule call - module is soft-unhooked') + return self._origGetBuiltinModule.apply(this, arguments) + } - return patchedRequire.call(this, arguments, true) + return patchedRequire.call(this, arguments, true) + } } // Preserve the original require/process.getBuiltinModule arguments in `args` @@ -172,9 +174,10 @@ function Hook (modules, options, onrequire) { } } else if (coreOnly) { // `coreOnly` is `true` if this was a call to `process.getBuiltinModule`, in which case - // we don't want to return anything if the requested `id` isn't a core module - debug('call to process.getBuiltinModule with unknown built-in id - returning undefined') - return undefined + // we don't want to return anything if the requested `id` isn't a core module. Falling + // back to default behaviour, which at the time of this wrting is simply returning `undefined` + debug('call to process.getBuiltinModule with unknown built-in id') + return self._origGetBuiltinModule.apply(this, args) } else { try { filename = Module._resolveFilename(id, this) From 93cf4ecf521254417433317211740f2d21dc68b3 Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Sat, 20 Jul 2024 08:17:39 +0200 Subject: [PATCH 3/4] Address review comments --- index.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index f4f520c..dfeea38 100644 --- a/index.js +++ b/index.js @@ -325,11 +325,13 @@ Hook.prototype.unhook = function () { debug('require unhook unsuccessful') } - if (this._getBuiltinModule === process.getBuiltinModule) { - process.getBuiltinModule = this._origGetBuiltinModule - debug('process.getBuiltinModule unhook successful') - } else { - debug('process.getBuiltinModule unhook unsuccessful') + if (process.getBuiltinModule !== undefined) { + if (this._getBuiltinModule === process.getBuiltinModule) { + process.getBuiltinModule = this._origGetBuiltinModule + debug('process.getBuiltinModule unhook successful') + } else { + debug('process.getBuiltinModule unhook unsuccessful') + } } } From ba454192689a2091504327d6eaf9830788d85a6a Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Thu, 25 Jul 2024 11:41:56 -0700 Subject: [PATCH 4/4] test: test with Node 22, drop v19 testing --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e2fdcb8..a574d76 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -11,7 +11,7 @@ jobs: test-vers: strategy: matrix: - node: ['8.6', '8', '10.0', '10', '12.0', '12', '14.0', '14', '16.0', '16', '18.0', '18', '19', '20'] + node: ['8.6', '8', '10.0', '10', '12.0', '12', '14.0', '14', '16.0', '16', '18.0', '18', '20', '22'] runs-on: ubuntu-latest steps: - uses: actions/checkout@v3