diff --git a/packages/nextjs/src/config/loaders/moduleMetadataInjectionLoader.ts b/packages/nextjs/src/config/loaders/moduleMetadataInjectionLoader.ts index 96c00569e06f..b26eb452e13b 100644 --- a/packages/nextjs/src/config/loaders/moduleMetadataInjectionLoader.ts +++ b/packages/nextjs/src/config/loaders/moduleMetadataInjectionLoader.ts @@ -1,5 +1,5 @@ import type { LoaderThis } from './types'; -import { SKIP_COMMENT_AND_DIRECTIVE_REGEX } from './valueInjectionLoader'; +import { findInjectionIndexAfterDirectives } from './valueInjectionLoader'; export type ModuleMetadataInjectionLoaderOptions = { applicationKey: string; @@ -39,7 +39,6 @@ export default function moduleMetadataInjectionLoader( `e._sentryModuleMetadata[(new e.Error).stack]=Object.assign({},e._sentryModuleMetadata[(new e.Error).stack],${metadata});` + '}catch(e){}}();'; - return userCode.replace(SKIP_COMMENT_AND_DIRECTIVE_REGEX, match => { - return match + injectedCode; - }); + const injectionIndex = findInjectionIndexAfterDirectives(userCode); + return `${userCode.slice(0, injectionIndex)}${injectedCode}${userCode.slice(injectionIndex)}`; } diff --git a/packages/nextjs/src/config/loaders/valueInjectionLoader.ts b/packages/nextjs/src/config/loaders/valueInjectionLoader.ts index 62cabcf818b8..d388b6bb4dd0 100644 --- a/packages/nextjs/src/config/loaders/valueInjectionLoader.ts +++ b/packages/nextjs/src/config/loaders/valueInjectionLoader.ts @@ -1,18 +1,151 @@ -// Rollup doesn't like if we put the directive regex as a literal (?). No idea why. -/* oxlint-disable sdk/no-regexp-constructor */ - import type { LoaderThis } from './types'; export type ValueInjectionLoaderOptions = { values: Record; }; -// We need to be careful not to inject anything before any `"use strict";`s or "use client"s or really any other directive. -// As an additional complication directives may come after any number of comments. -// This regex is shamelessly stolen from: https://github.com/getsentry/sentry-javascript-bundler-plugins/blob/7f984482c73e4284e8b12a08dfedf23b5a82f0af/packages/bundler-plugin-core/src/index.ts#L535-L539 -export const SKIP_COMMENT_AND_DIRECTIVE_REGEX = - // Note: CodeQL complains that this regex potentially has n^2 runtime. This likely won't affect realistic files. - new RegExp('^(?:\\s*|/\\*(?:.|\\r|\\n)*?\\*/|//.*[\\n\\r])*(?:"[^"]*";?|\'[^\']*\';?)?'); +// We need to be careful not to inject anything before any `"use strict";`s or "use client"s or really any other +// directives. A small scanner is easier to reason about than the previous regex and avoids regex backtracking concerns. +export function findInjectionIndexAfterDirectives(userCode: string): number { + let index = 0; + let lastDirectiveEndIndex: number | undefined; + + while (index < userCode.length) { + const statementStartIndex = skipWhitespaceAndComments(userCode, index); + if (statementStartIndex === undefined) { + return lastDirectiveEndIndex ?? 0; + } + + index = statementStartIndex; + if (statementStartIndex === userCode.length) { + return lastDirectiveEndIndex ?? statementStartIndex; + } + + const quote = userCode[statementStartIndex]; + if (quote !== '"' && quote !== "'") { + return lastDirectiveEndIndex ?? statementStartIndex; + } + + const stringEndIndex = findStringLiteralEnd(userCode, statementStartIndex); + if (stringEndIndex === undefined) { + return lastDirectiveEndIndex ?? statementStartIndex; + } + + const statementEndIndex = findDirectiveTerminator(userCode, stringEndIndex); + if (statementEndIndex === undefined) { + return lastDirectiveEndIndex ?? statementStartIndex; + } + + index = statementEndIndex; + lastDirectiveEndIndex = statementEndIndex; + } + + return lastDirectiveEndIndex ?? index; +} + +function skipWhitespaceAndComments(userCode: string, startIndex: number): number | undefined { + let index = startIndex; + + while (index < userCode.length) { + const char = userCode[index]; + + if (char && /\s/.test(char)) { + index += 1; + continue; + } + + if (userCode.startsWith('//', index)) { + const newlineIndex = userCode.indexOf('\n', index + 2); + index = newlineIndex === -1 ? userCode.length : newlineIndex + 1; + continue; + } + + if (userCode.startsWith('/*', index)) { + const commentEndIndex = userCode.indexOf('*/', index + 2); + if (commentEndIndex === -1) { + return undefined; + } + + index = commentEndIndex + 2; + continue; + } + + break; + } + + return index; +} + +function findStringLiteralEnd(userCode: string, startIndex: number): number | undefined { + const quote = userCode[startIndex]; + let index = startIndex + 1; + + while (index < userCode.length) { + const char = userCode[index]; + + if (char === '\\') { + index += 2; + continue; + } + + if (char === quote) { + return index + 1; + } + + if (char === '\n' || char === '\r') { + return undefined; + } + + index += 1; + } + + return undefined; +} + +function findDirectiveTerminator(userCode: string, startIndex: number): number | undefined { + let index = startIndex; + + // Only a bare string literal followed by a statement terminator counts as a directive. + while (index < userCode.length) { + const char = userCode[index]; + + if (char === ';') { + return index + 1; + } + + if (char === '\n' || char === '\r' || char === '}') { + return index; + } + + if (char && /\s/.test(char)) { + index += 1; + continue; + } + + if (userCode.startsWith('//', index)) { + return index; + } + + if (userCode.startsWith('/*', index)) { + const commentEndIndex = userCode.indexOf('*/', index + 2); + if (commentEndIndex === -1) { + return undefined; + } + + const comment = userCode.slice(index + 2, commentEndIndex); + if (comment.includes('\n') || comment.includes('\r')) { + return index; + } + + index = commentEndIndex + 2; + continue; + } + + return undefined; + } + + return index; +} /** * Set values on the global/window object at the start of a module. @@ -36,7 +169,6 @@ export default function valueInjectionLoader(this: LoaderThis `globalThis["${key}"] = ${JSON.stringify(value)};`) .join(''); - return userCode.replace(SKIP_COMMENT_AND_DIRECTIVE_REGEX, match => { - return match + injectedCode; - }); + const injectionIndex = findInjectionIndexAfterDirectives(userCode); + return `${userCode.slice(0, injectionIndex)}${injectedCode}${userCode.slice(injectionIndex)}`; } diff --git a/packages/nextjs/test/config/moduleMetadataInjectionLoader.test.ts b/packages/nextjs/test/config/moduleMetadataInjectionLoader.test.ts index 1a6a2cd14b71..f6c1c613bd00 100644 --- a/packages/nextjs/test/config/moduleMetadataInjectionLoader.test.ts +++ b/packages/nextjs/test/config/moduleMetadataInjectionLoader.test.ts @@ -131,4 +131,30 @@ describe('moduleMetadataInjectionLoader', () => { expect(result).toContain('"_sentryBundlerPluginAppKey:test-key-123":true'); }); + + it('should inject after multiple directives', () => { + const loaderThis = createLoaderThis('my-app'); + const userCode = '"use strict";\n"use client";\nimport React from \'react\';'; + + const result = moduleMetadataInjectionLoader.call(loaderThis, userCode); + + const metadataIndex = result.indexOf('_sentryModuleMetadata'); + const clientDirectiveIndex = result.indexOf('"use client"'); + const importIndex = result.indexOf("import React from 'react';"); + + expect(metadataIndex).toBeGreaterThan(clientDirectiveIndex); + expect(metadataIndex).toBeLessThan(importIndex); + }); + + it('should inject after comments between multiple directives', () => { + const loaderThis = createLoaderThis('my-app'); + const userCode = '"use strict";\n/* keep */\n"use client";\nimport React from \'react\';'; + + const result = moduleMetadataInjectionLoader.call(loaderThis, userCode); + + const metadataIndex = result.indexOf('_sentryModuleMetadata'); + const clientDirectiveIndex = result.indexOf('"use client"'); + + expect(metadataIndex).toBeGreaterThan(clientDirectiveIndex); + }); }); diff --git a/packages/nextjs/test/config/valueInjectionLoader.test.ts b/packages/nextjs/test/config/valueInjectionLoader.test.ts index 57b40b006baa..6d60819c9cd6 100644 --- a/packages/nextjs/test/config/valueInjectionLoader.test.ts +++ b/packages/nextjs/test/config/valueInjectionLoader.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vitest'; import type { LoaderThis } from '../../src/config/loaders/types'; import type { ValueInjectionLoaderOptions } from '../../src/config/loaders/valueInjectionLoader'; -import valueInjectionLoader from '../../src/config/loaders/valueInjectionLoader'; +import valueInjectionLoader, { findInjectionIndexAfterDirectives } from '../../src/config/loaders/valueInjectionLoader'; const defaultLoaderThis = { addDependency: () => undefined, @@ -149,4 +149,99 @@ describe.each([[clientConfigLoaderThis], [instrumentationLoaderThis]])('valueInj expect(result).toMatchSnapshot(); expect(result).toMatch(';globalThis["foo"] = "bar";'); }); + + it('should correctly insert values after multiple directives', () => { + const userCode = ` + "use strict"; + "use client"; + import * as Sentry from '@sentry/nextjs'; + Sentry.init(); + `; + + const result = valueInjectionLoader.call(loaderThis, userCode); + + const injectionIndex = result.indexOf(';globalThis["foo"] = "bar";'); + const clientDirectiveIndex = result.indexOf('"use client"'); + const importIndex = result.indexOf("import * as Sentry from '@sentry/nextjs';"); + + expect(injectionIndex).toBeGreaterThan(clientDirectiveIndex); + expect(injectionIndex).toBeLessThan(importIndex); + }); + + it('should correctly insert values after comments between multiple directives', () => { + const userCode = ` + "use strict"; + /* keep */ + "use client"; + import * as Sentry from '@sentry/nextjs'; + Sentry.init(); + `; + + const result = valueInjectionLoader.call(loaderThis, userCode); + + const injectionIndex = result.indexOf(';globalThis["foo"] = "bar";'); + const clientDirectiveIndex = result.indexOf('"use client"'); + + expect(injectionIndex).toBeGreaterThan(clientDirectiveIndex); + }); + + it('should correctly insert values after semicolon-free directives', () => { + const userCode = ` + "use strict" + "use client" + import * as Sentry from '@sentry/nextjs'; + Sentry.init(); + `; + + const result = valueInjectionLoader.call(loaderThis, userCode); + + const injectionIndex = result.indexOf(';globalThis["foo"] = "bar";'); + const clientDirectiveIndex = result.indexOf('"use client"'); + + expect(injectionIndex).toBeGreaterThan(clientDirectiveIndex); + }); +}); + +describe('findInjectionIndexAfterDirectives', () => { + it('returns the position immediately after the last directive', () => { + const userCode = '"use strict";\n"use client";\nimport React from \'react\';'; + + expect(userCode.slice(findInjectionIndexAfterDirectives(userCode))).toBe("\nimport React from 'react';"); + }); + + it('returns the end of the input when the last directive reaches EOF', () => { + const userCode = '"use strict";\n"use client";'; + + expect(findInjectionIndexAfterDirectives(userCode)).toBe(userCode.length); + }); + + it('does not skip a string literal that is not a directive', () => { + const userCode = '"use client" + suffix;'; + + expect(findInjectionIndexAfterDirectives(userCode)).toBe(0); + }); + + it('does not treat an escaped quote at EOF as a closed directive', () => { + const userCode = '"use client\\"'; + + expect(findInjectionIndexAfterDirectives(userCode)).toBe(0); + }); + + it('returns 0 for an unterminated leading block comment', () => { + const userCode = '/* unterminated'; + + expect(findInjectionIndexAfterDirectives(userCode)).toBe(0); + }); + + it('returns the last complete directive when followed by an unterminated block comment', () => { + const userCode = '"use client"; /* unterminated'; + + expect(findInjectionIndexAfterDirectives(userCode)).toBe('"use client";'.length); + }); + + it('treats a block comment without a line break as part of the same statement', () => { + const userCode = '"use client" /* comment */ + suffix;'; + + expect(findInjectionIndexAfterDirectives(userCode)).toBe(0); + }); });