From a2732a4f10caa2f9a20e468fe42a6394d25778a5 Mon Sep 17 00:00:00 2001 From: "Micael Levi (@micalevisk)" Date: Thu, 2 Jun 2022 18:51:37 -0400 Subject: [PATCH 1/4] fix(core): prevents renaming global providers and modules by marking them as a read-only properties of `globalThis` obj --- packages/core/repl/repl-context.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/core/repl/repl-context.ts b/packages/core/repl/repl-context.ts index cf0e81ce2dc..d1c3bd428bb 100644 --- a/packages/core/repl/repl-context.ts +++ b/packages/core/repl/repl-context.ts @@ -59,7 +59,6 @@ export class ReplContext { } private initializeContext() { - const globalRef = globalThis; const modules = this.container.getModules(); modules.forEach(moduleRef => { @@ -67,14 +66,18 @@ export class ReplContext { if (moduleName === InternalCoreModule.name) { return; } - if (globalRef[moduleName]) { + if (globalThis[moduleName]) { moduleName += ` (${moduleRef.token})`; } this.introspectCollection(moduleRef, moduleName, 'providers'); this.introspectCollection(moduleRef, moduleName, 'controllers'); - globalRef[moduleName] = moduleRef.metatype; + // For in REPL auto-complete functionality + Object.defineProperty(globalThis, moduleName, { + value: moduleRef.metatype, + configurable: false, + }); }); } @@ -88,12 +91,16 @@ export class ReplContext { const stringifiedToken = this.stringifyToken(token); if ( stringifiedToken === ApplicationConfig.name || - stringifiedToken === moduleRef.metatype.name + stringifiedToken === moduleRef.metatype.name || + globalThis[stringifiedToken] ) { return; } // For in REPL auto-complete functionality - globalThis[stringifiedToken] = token; + Object.defineProperty(globalThis, stringifiedToken, { + value: token, + configurable: false, + }); if (stringifiedToken === ModuleRef.name) { return; From 30618bf703865298ec166845c88d64ccffcda555 Mon Sep 17 00:00:00 2001 From: "Micael Levi (@micalevisk)" Date: Sat, 4 Jun 2022 18:01:54 -0400 Subject: [PATCH 2/4] feat(core): drop `globalThis` usage from `ReplContext` --- integration/repl/e2e/repl.spec.ts | 5 +- .../load-native-functions-into-context.ts | 33 -------- packages/core/repl/repl-context.ts | 76 +++++++++++++------ packages/core/repl/repl.ts | 14 +++- 4 files changed, 66 insertions(+), 62 deletions(-) delete mode 100644 packages/core/repl/load-native-functions-into-context.ts diff --git a/integration/repl/e2e/repl.spec.ts b/integration/repl/e2e/repl.spec.ts index c57cb828646..8c20d3a79b7 100644 --- a/integration/repl/e2e/repl.spec.ts +++ b/integration/repl/e2e/repl.spec.ts @@ -12,7 +12,6 @@ import { import { expect } from 'chai'; import * as sinon from 'sinon'; import { AppModule } from '../src/app.module'; -import { UsersModule } from '../src/users/users.module'; const PROMPT = '\u001b[1G\u001b[0J> \u001b[3G'; @@ -28,13 +27,11 @@ describe('REPL', () => { }); afterEach(() => { sinon.restore(); - delete globalThis[AppModule.name]; - delete globalThis[UsersModule.name]; }); it('get()', async () => { const server = await repl(AppModule); - + server.context let outputText = ''; sinon.stub(process.stdout, 'write').callsFake(text => { outputText += text; diff --git a/packages/core/repl/load-native-functions-into-context.ts b/packages/core/repl/load-native-functions-into-context.ts deleted file mode 100644 index 6dd0a5c4e02..00000000000 --- a/packages/core/repl/load-native-functions-into-context.ts +++ /dev/null @@ -1,33 +0,0 @@ -import * as _repl from 'repl'; -import { ReplContext } from './repl-context'; -import { ReplFunction } from './repl-function'; -import { ReplFunctionClass } from './repl.interfaces'; - -export function loadNativeFunctionsIntoContext( - replServerContext: _repl.REPLServer['context'], - replContext: ReplContext, -) { - const registerFunctionToReplServerContext = ( - nativeFunction: InstanceType, - ): void => { - // Bind the method to REPL's context: - replServerContext[nativeFunction.fnDefinition.name] = - nativeFunction.action.bind(nativeFunction); - - // Load the help trigger as a `help` getter on each native function: - const functionBoundRef: ReplFunction['action'] = - replServerContext[nativeFunction.fnDefinition.name]; - Object.defineProperty(functionBoundRef, 'help', { - enumerable: false, - configurable: false, - get: () => - // Dynamically builds the help message as will unlikely to be called - // several times. - replContext.writeToStdout(nativeFunction.makeHelpMessage()), - }); - }; - - for (const [, nativeFunction] of replContext.nativeFunctions) { - registerFunctionToReplServerContext(nativeFunction); - } -} diff --git a/packages/core/repl/repl-context.ts b/packages/core/repl/repl-context.ts index d1c3bd428bb..7a8696cae40 100644 --- a/packages/core/repl/repl-context.ts +++ b/packages/core/repl/repl-context.ts @@ -11,6 +11,7 @@ import { ResolveReplFn, SelectReplFn, } from './native-functions'; +import { ReplFunction } from './repl-function'; import type { ReplFunctionClass } from './repl.interfaces'; type ModuleKey = string; @@ -19,9 +20,12 @@ export type ModuleDebugEntry = { providers: Record; }; +type ReplScope = Record; + export class ReplContext { public readonly logger = new Logger(ReplContext.name); public debugRegistry: Record = {}; + public readonly globalScope: ReplScope = Object.create(null); public readonly nativeFunctions = new Map< string, InstanceType @@ -33,6 +37,7 @@ export class ReplContext { nativeFunctionsClassRefs?: ReplFunctionClass[], ) { this.container = (app as any).container; // Using `any` because `app.container` is not public. + this.initializeContext(); this.initializeNativeFunctions(nativeFunctionsClassRefs || []); } @@ -41,23 +46,6 @@ export class ReplContext { process.stdout.write(text); } - public addNativeFunction(NativeFunction: ReplFunctionClass): void { - const nativeFunction = new NativeFunction(this); - - this.nativeFunctions.set(nativeFunction.fnDefinition.name, nativeFunction); - - nativeFunction.fnDefinition.aliases?.forEach(aliaseName => { - const aliasNativeFunction: InstanceType = - Object.create(nativeFunction); - aliasNativeFunction.fnDefinition = { - name: aliaseName, - description: aliasNativeFunction.fnDefinition.description, - signature: aliasNativeFunction.fnDefinition.signature, - }; - this.nativeFunctions.set(aliaseName, aliasNativeFunction); - }); - } - private initializeContext() { const modules = this.container.getModules(); @@ -66,7 +54,7 @@ export class ReplContext { if (moduleName === InternalCoreModule.name) { return; } - if (globalThis[moduleName]) { + if (this.globalScope[moduleName]) { moduleName += ` (${moduleRef.token})`; } @@ -74,9 +62,10 @@ export class ReplContext { this.introspectCollection(moduleRef, moduleName, 'controllers'); // For in REPL auto-complete functionality - Object.defineProperty(globalThis, moduleName, { + Object.defineProperty(this.globalScope, moduleName, { value: moduleRef.metatype, configurable: false, + enumerable: true, }); }); } @@ -92,14 +81,15 @@ export class ReplContext { if ( stringifiedToken === ApplicationConfig.name || stringifiedToken === moduleRef.metatype.name || - globalThis[stringifiedToken] + this.globalScope[stringifiedToken] ) { return; } // For in REPL auto-complete functionality - Object.defineProperty(globalThis, stringifiedToken, { + Object.defineProperty(this.globalScope, stringifiedToken, { value: token, configurable: false, + enumerable: true, }); if (stringifiedToken === ModuleRef.name) { @@ -122,6 +112,47 @@ export class ReplContext { : token; } + private addNativeFunction( + NativeFunctionRef: ReplFunctionClass, + ): InstanceType { + const nativeFunction = new NativeFunctionRef(this); + + this.nativeFunctions.set(nativeFunction.fnDefinition.name, nativeFunction); + + nativeFunction.fnDefinition.aliases?.forEach(aliaseName => { + const aliasNativeFunction: InstanceType = + Object.create(nativeFunction); + aliasNativeFunction.fnDefinition = { + name: aliaseName, + description: aliasNativeFunction.fnDefinition.description, + signature: aliasNativeFunction.fnDefinition.signature, + }; + this.nativeFunctions.set(aliaseName, aliasNativeFunction); + }); + + return nativeFunction; + } + + private registerFunctionIntoGlobalScope( + nativeFunction: InstanceType, + ) { + // Bind the method to REPL's context: + this.globalScope[nativeFunction.fnDefinition.name] = + nativeFunction.action.bind(nativeFunction); + + // Load the help trigger as a `help` getter on each native function: + const functionBoundRef: ReplFunction['action'] = + this.globalScope[nativeFunction.fnDefinition.name]; + Object.defineProperty(functionBoundRef, 'help', { + enumerable: false, + configurable: false, + get: () => + // Dynamically builds the help message as will unlikely to be called + // several times. + this.writeToStdout(nativeFunction.makeHelpMessage()), + }); + } + private initializeNativeFunctions( nativeFunctionsClassRefs: ReplFunctionClass[], ): void { @@ -137,7 +168,8 @@ export class ReplContext { builtInFunctionsClassRefs .concat(nativeFunctionsClassRefs) .forEach(NativeFunction => { - this.addNativeFunction(NativeFunction); + const nativeFunction = this.addNativeFunction(NativeFunction); + this.registerFunctionIntoGlobalScope(nativeFunction); }); } } diff --git a/packages/core/repl/repl.ts b/packages/core/repl/repl.ts index 1c780ba7bfc..14fa6575943 100644 --- a/packages/core/repl/repl.ts +++ b/packages/core/repl/repl.ts @@ -3,10 +3,19 @@ import * as _repl from 'repl'; import { clc } from '@nestjs/common/utils/cli-colors.util'; import { NestFactory } from '../nest-factory'; import { REPL_INITIALIZED_MESSAGE } from './constants'; -import { loadNativeFunctionsIntoContext } from './load-native-functions-into-context'; import { ReplContext } from './repl-context'; import { ReplLogger } from './repl-logger'; +function copyInto(target, source): void { + Object.defineProperties( + target, + Object.keys(source).reduce((descriptors, key) => { + descriptors[key] = Object.getOwnPropertyDescriptor(source, key); + return descriptors; + }, Object.create(null)), + ); +} + export async function repl(module: Type) { const app = await NestFactory.create(module, { abortOnError: false, @@ -21,8 +30,7 @@ export async function repl(module: Type) { prompt: clc.green('> '), ignoreUndefined: true, }); - - loadNativeFunctionsIntoContext(replServer.context, replContext); + copyInto(replServer.context, replContext.globalScope); return replServer; } From 2c4aa9f0b4766fd1604d43c1e11a6b848b98e480 Mon Sep 17 00:00:00 2001 From: "Micael Levi (@micalevisk)" Date: Sat, 4 Jun 2022 18:43:06 -0400 Subject: [PATCH 3/4] refactor(core): fix typo on spec files name --- .../{debug-relp-fn.spec.ts => debug-repl-fn.spec.ts} | 0 .../native-functions/{get-relp-fn.spec.ts => get-repl-fn.spec.ts} | 0 .../{help-relp-fn.spec.ts => help-repl-fn.spec.ts} | 0 .../{methods-relp-fn.spec.ts => methods-repl-fn.spec.ts} | 0 .../{resolve-relp-fn.spec.ts => resolve-repl-fn.spec.ts} | 0 .../{select-relp-fn.spec.ts => select-repl-fn.spec.ts} | 0 6 files changed, 0 insertions(+), 0 deletions(-) rename packages/core/test/repl/native-functions/{debug-relp-fn.spec.ts => debug-repl-fn.spec.ts} (100%) rename packages/core/test/repl/native-functions/{get-relp-fn.spec.ts => get-repl-fn.spec.ts} (100%) rename packages/core/test/repl/native-functions/{help-relp-fn.spec.ts => help-repl-fn.spec.ts} (100%) rename packages/core/test/repl/native-functions/{methods-relp-fn.spec.ts => methods-repl-fn.spec.ts} (100%) rename packages/core/test/repl/native-functions/{resolve-relp-fn.spec.ts => resolve-repl-fn.spec.ts} (100%) rename packages/core/test/repl/native-functions/{select-relp-fn.spec.ts => select-repl-fn.spec.ts} (100%) diff --git a/packages/core/test/repl/native-functions/debug-relp-fn.spec.ts b/packages/core/test/repl/native-functions/debug-repl-fn.spec.ts similarity index 100% rename from packages/core/test/repl/native-functions/debug-relp-fn.spec.ts rename to packages/core/test/repl/native-functions/debug-repl-fn.spec.ts diff --git a/packages/core/test/repl/native-functions/get-relp-fn.spec.ts b/packages/core/test/repl/native-functions/get-repl-fn.spec.ts similarity index 100% rename from packages/core/test/repl/native-functions/get-relp-fn.spec.ts rename to packages/core/test/repl/native-functions/get-repl-fn.spec.ts diff --git a/packages/core/test/repl/native-functions/help-relp-fn.spec.ts b/packages/core/test/repl/native-functions/help-repl-fn.spec.ts similarity index 100% rename from packages/core/test/repl/native-functions/help-relp-fn.spec.ts rename to packages/core/test/repl/native-functions/help-repl-fn.spec.ts diff --git a/packages/core/test/repl/native-functions/methods-relp-fn.spec.ts b/packages/core/test/repl/native-functions/methods-repl-fn.spec.ts similarity index 100% rename from packages/core/test/repl/native-functions/methods-relp-fn.spec.ts rename to packages/core/test/repl/native-functions/methods-repl-fn.spec.ts diff --git a/packages/core/test/repl/native-functions/resolve-relp-fn.spec.ts b/packages/core/test/repl/native-functions/resolve-repl-fn.spec.ts similarity index 100% rename from packages/core/test/repl/native-functions/resolve-relp-fn.spec.ts rename to packages/core/test/repl/native-functions/resolve-repl-fn.spec.ts diff --git a/packages/core/test/repl/native-functions/select-relp-fn.spec.ts b/packages/core/test/repl/native-functions/select-repl-fn.spec.ts similarity index 100% rename from packages/core/test/repl/native-functions/select-relp-fn.spec.ts rename to packages/core/test/repl/native-functions/select-repl-fn.spec.ts From 1cc12acaf6b7172174191efcfa8d3ce709756751 Mon Sep 17 00:00:00 2001 From: "Micael Levi (@micalevisk)" Date: Tue, 14 Jun 2022 08:48:35 -0400 Subject: [PATCH 4/4] refactor(repl): extract utility from repl main file --- packages/core/repl/assign-to-object.util.ts | 14 ++++++++ packages/core/repl/repl.ts | 13 ++----- .../test/repl/assign-to-object.util.spec.ts | 36 +++++++++++++++++++ 3 files changed, 52 insertions(+), 11 deletions(-) create mode 100644 packages/core/repl/assign-to-object.util.ts create mode 100644 packages/core/test/repl/assign-to-object.util.spec.ts diff --git a/packages/core/repl/assign-to-object.util.ts b/packages/core/repl/assign-to-object.util.ts new file mode 100644 index 00000000000..16f9379992f --- /dev/null +++ b/packages/core/repl/assign-to-object.util.ts @@ -0,0 +1,14 @@ +/** + * Similar to `Object.assign` but copying properties descriptors from `source` + * as well. + */ +export function assignToObject(target: T, source: U): T & U { + Object.defineProperties( + target, + Object.keys(source).reduce((descriptors, key) => { + descriptors[key] = Object.getOwnPropertyDescriptor(source, key); + return descriptors; + }, Object.create(null)), + ); + return target as T & U; +} diff --git a/packages/core/repl/repl.ts b/packages/core/repl/repl.ts index 14fa6575943..079f16f1fe2 100644 --- a/packages/core/repl/repl.ts +++ b/packages/core/repl/repl.ts @@ -5,16 +5,7 @@ import { NestFactory } from '../nest-factory'; import { REPL_INITIALIZED_MESSAGE } from './constants'; import { ReplContext } from './repl-context'; import { ReplLogger } from './repl-logger'; - -function copyInto(target, source): void { - Object.defineProperties( - target, - Object.keys(source).reduce((descriptors, key) => { - descriptors[key] = Object.getOwnPropertyDescriptor(source, key); - return descriptors; - }, Object.create(null)), - ); -} +import { assignToObject } from './assign-to-object.util'; export async function repl(module: Type) { const app = await NestFactory.create(module, { @@ -30,7 +21,7 @@ export async function repl(module: Type) { prompt: clc.green('> '), ignoreUndefined: true, }); - copyInto(replServer.context, replContext.globalScope); + assignToObject(replServer.context, replContext.globalScope); return replServer; } diff --git a/packages/core/test/repl/assign-to-object.util.spec.ts b/packages/core/test/repl/assign-to-object.util.spec.ts new file mode 100644 index 00000000000..812343aef90 --- /dev/null +++ b/packages/core/test/repl/assign-to-object.util.spec.ts @@ -0,0 +1,36 @@ +import { expect } from 'chai'; +import { assignToObject } from '@nestjs/core/repl/assign-to-object.util'; + +describe('assignToObject', () => { + it('should copy all enumerable properties and their descriptors', () => { + const sourceObj = {}; + Object.defineProperty(sourceObj, 'foo', { + value: 123, + configurable: true, + enumerable: true, + writable: true, + }); + Object.defineProperty(sourceObj, 'bar', { + value: 456, + configurable: true, + enumerable: true, + writable: false, + }); + const targetObj = {}; + + assignToObject(targetObj, sourceObj); + + expect(Object.getOwnPropertyDescriptor(targetObj, 'foo')).to.be.eql({ + value: 123, + configurable: true, + enumerable: true, + writable: true, + }); + expect(Object.getOwnPropertyDescriptor(targetObj, 'bar')).to.be.eql({ + value: 456, + configurable: true, + enumerable: true, + writable: false, + }); + }); +});