Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update middleware eval checks #30883

Merged
merged 7 commits into from Nov 5, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
142 changes: 124 additions & 18 deletions packages/next/build/webpack/plugins/middleware-plugin.ts
@@ -1,4 +1,4 @@
import { webpack, sources } from 'next/dist/compiled/webpack/webpack'
import { webpack, sources, webpack5 } from 'next/dist/compiled/webpack/webpack'
import { getMiddlewareRegex } from '../../../shared/lib/router/utils'
import { getSortedRoutes } from '../../../shared/lib/router/utils'
import {
Expand Down Expand Up @@ -39,7 +39,7 @@ export default class MiddlewarePlugin {
}

createAssets(
compilation: any,
compilation: webpack5.Compilation,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was recently changed to any but I don't remember why

cc @timneutkens @sokra

assets: any,
envPerRoute: Map<string, string[]>
) {
Expand All @@ -52,6 +52,7 @@ export default class MiddlewarePlugin {
}

for (const entrypoint of entrypoints.values()) {
if (!entrypoint.name) continue
const result = MIDDLEWARE_FULL_ROUTE_REGEX.exec(entrypoint.name)
const ssrEntryInfo = ssrEntries.get(entrypoint.name)

Expand Down Expand Up @@ -111,19 +112,21 @@ export default class MiddlewarePlugin {
)
}

apply(compiler: webpack.Compiler) {
apply(compiler: webpack5.Compiler) {
const { dev } = this
const wp = compiler.webpack
compiler.hooks.compilation.tap(
PLUGIN_NAME,
(compilation, { normalModuleFactory }) => {
const envPerRoute = new Map<string, string[]>()

compilation.hooks.finishModules.tap(PLUGIN_NAME, () => {
compilation.hooks.afterOptimizeModules.tap(PLUGIN_NAME, () => {
const { moduleGraph } = compilation as any
envPerRoute.clear()

for (const [name, info] of compilation.entries) {
if (name.match(MIDDLEWARE_ROUTE)) {
const middlewareEntries = new Set<webpack.Module>()
const middlewareEntries = new Set<webpack5.Module>()
const env = new Set<string>()

const addEntriesFromDependency = (dep: any) => {
Expand All @@ -133,19 +136,41 @@ export default class MiddlewarePlugin {
}
}

const runtime = wp.util.runtime.getEntryRuntime(compilation, name)

info.dependencies.forEach(addEntriesFromDependency)
info.includeDependencies.forEach(addEntriesFromDependency)

const queue = new Set(middlewareEntries)
for (const module of queue) {
const { buildInfo } = module as any
if (buildInfo?.usingIndirectEval) {
// @ts-ignore TODO: Remove ignore when webpack 5 is stable
const error = new webpack.WebpackError(
`\`eval\` not allowed in Middleware ${name}`
const { buildInfo } = module
if (
!dev &&
buildInfo &&
isUsedByExports({
module,
moduleGraph,
runtime,
usedByExports: buildInfo.usingIndirectEval,
})
) {
if (
/node_modules[\\/]regenerator-runtime[\\/]runtime\.js/.test(
module.identifier()
)
)
continue
const error = new wp.WebpackError(
`Dynamic Code Evaluation (e. g. 'eval', 'new Function') not allowed in Middleware ${name}${
typeof buildInfo.usingIndirectEval !== 'boolean'
? `\nUsed by ${Array.from(
buildInfo.usingIndirectEval
).join(', ')}`
: ''
}`
)
error.module = module
compilation.warnings.push(error)
compilation.errors.push(error)
}

if (buildInfo?.nextUsedEnvVars !== undefined) {
Expand All @@ -167,19 +192,82 @@ export default class MiddlewarePlugin {
}
})

const handler = (parser: any) => {
const flagModule = () => {
parser.state.module.buildInfo.usingIndirectEval = true
const handler = (parser: webpack5.javascript.JavascriptParser) => {
const wrapExpression = (expr: any) => {
if (dev) {
const dep1 = new wp.dependencies.ConstDependency(
'__next_eval__(function() { return ',
expr.range[0]
)
dep1.loc = expr.loc
parser.state.module.addPresentationalDependency(dep1)
const dep2 = new wp.dependencies.ConstDependency(
'})',
expr.range[1]
)
dep2.loc = expr.loc
parser.state.module.addPresentationalDependency(dep2)
}
expressionHandler()
return true
}

const flagModule = (
usedByExports: boolean | Set<string> | undefined
) => {
if (usedByExports === undefined) usedByExports = true
const old = parser.state.module.buildInfo.usingIndirectEval
if (old === true || usedByExports === false) return
if (!old || usedByExports === true) {
parser.state.module.buildInfo.usingIndirectEval = usedByExports
return
}
const set = new Set(old)
for (const item of usedByExports) {
set.add(item)
}
parser.state.module.buildInfo.usingIndirectEval = set
}

const expressionHandler = () => {
wp.optimize.InnerGraph.onUsage(parser.state, flagModule)
}

const ignore = () => {
return true
}

parser.hooks.expression.for('eval').tap(PLUGIN_NAME, flagModule)
parser.hooks.expression.for('Function').tap(PLUGIN_NAME, flagModule)
// wrapping
parser.hooks.call.for('eval').tap(PLUGIN_NAME, wrapExpression)
parser.hooks.call.for('global.eval').tap(PLUGIN_NAME, wrapExpression)
parser.hooks.call.for('Function').tap(PLUGIN_NAME, wrapExpression)
parser.hooks.call
.for('global.Function')
.tap(PLUGIN_NAME, wrapExpression)
parser.hooks.new.for('Function').tap(PLUGIN_NAME, wrapExpression)
parser.hooks.new
.for('global.Function')
.tap(PLUGIN_NAME, wrapExpression)

// fallbacks
parser.hooks.expression
.for('eval')
.tap(PLUGIN_NAME, expressionHandler)
parser.hooks.expression
.for('Function')
.tap(PLUGIN_NAME, expressionHandler)
parser.hooks.expression
.for('Function.prototype')
.tap(PLUGIN_NAME, ignore)
parser.hooks.expression
.for('global.eval')
.tap(PLUGIN_NAME, flagModule)
.tap(PLUGIN_NAME, expressionHandler)
parser.hooks.expression
.for('global.Function')
.tap(PLUGIN_NAME, flagModule)
.tap(PLUGIN_NAME, expressionHandler)
parser.hooks.expression
.for('global.Function.prototype')
.tap(PLUGIN_NAME, ignore)

const memberChainHandler = (_expr: any, members: string[]) => {
if (
Expand Down Expand Up @@ -237,3 +325,21 @@ export default class MiddlewarePlugin {
)
}
}

function isUsedByExports(args: {
module: webpack5.Module
moduleGraph: webpack5.ModuleGraph
runtime: any
usedByExports: boolean | Set<string> | undefined
}): boolean {
const { moduleGraph, runtime, module, usedByExports } = args
if (usedByExports === undefined) return false
if (typeof usedByExports === 'boolean') return usedByExports
const exportsInfo = moduleGraph.getExportsInfo(module)
const wp = webpack as unknown as typeof webpack5
for (const exportName of usedByExports) {
if (exportsInfo.getUsed(exportName, runtime) !== wp.UsageState.Unused)
return true
}
return false
}
22 changes: 17 additions & 5 deletions packages/next/server/dev/next-dev-server.ts
Expand Up @@ -514,7 +514,13 @@ export default class DevServer extends Server {
parsed: UrlWithParsedQuery
}): Promise<FetchEventResult | null> {
try {
const result = await super.runMiddleware(params)
const result = await super.runMiddleware({
...params,
onWarning: (warn) => {
this.logErrorWithOriginalStack(warn, 'warning', 'client')
},
})

result?.waitUntil.catch((error) =>
this.logErrorWithOriginalStack(error, 'unhandledRejection', 'client')
)
Expand Down Expand Up @@ -589,7 +595,7 @@ export default class DevServer extends Server {

private async logErrorWithOriginalStack(
err?: unknown,
type?: 'unhandledRejection' | 'uncaughtException',
type?: 'unhandledRejection' | 'uncaughtException' | 'warning',
stats: 'server' | 'client' = 'server'
) {
let usedOriginalStack = false
Expand Down Expand Up @@ -630,11 +636,15 @@ export default class DevServer extends Server {
const { file, lineNumber, column, methodName } = originalStackFrame

console.error(
chalk.red('error') +
(type === 'warning' ? chalk.yellow('warn') : chalk.red('error')) +
' - ' +
`${file} (${lineNumber}:${column}) @ ${methodName}`
)
console.error(`${chalk.red(err.name)}: ${err.message}`)
console.error(
`${(type === 'warning' ? chalk.yellow : chalk.red)(err.name)}: ${
err.message
}`
)
console.error(originalCodeFrame)
usedOriginalStack = true
}
Expand All @@ -647,7 +657,9 @@ export default class DevServer extends Server {
}

if (!usedOriginalStack) {
if (type) {
if (type === 'warning') {
Log.warn(err + '')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why no "warning" prefix?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@styfle The prefix is added by Log. The one that is interpolated in the line below is for unhandledRejection and uncaughtException. The message will show up prefixed.

} else if (type) {
Log.error(`${type}:`, err + '')
} else {
Log.error(err + '')
Expand Down
7 changes: 7 additions & 0 deletions packages/next/server/next-server.ts
Expand Up @@ -611,6 +611,7 @@ export default class Server {
response: ServerResponse
parsedUrl: ParsedNextUrl
parsed: UrlWithParsedQuery
onWarning?: (warning: Error) => void
}): Promise<FetchEventResult | null> {
this.middlewareBetaWarning()

Expand Down Expand Up @@ -672,6 +673,12 @@ export default class Server {
page: page,
},
ssr: !!this.nextConfig.experimental.concurrentFeatures,
onWarning: (warning: Error) => {
if (params.onWarning) {
warning.message += ` "./${middlewareInfo.name}"`
params.onWarning(warning)
}
},
})

for (let [key, value] of result.response.headers) {
Expand Down
34 changes: 32 additions & 2 deletions packages/next/server/web/sandbox/sandbox.ts
Expand Up @@ -10,9 +10,11 @@ import vm from 'vm'
let cache:
| {
context: { [key: string]: any }
onWarning: (warn: Error) => void
paths: Map<string, string>
require: Map<string, any>
sandbox: vm.Context
warnedEvals: Set<string>
}
| undefined

Expand All @@ -33,12 +35,14 @@ export function clearSandboxCache(path: string, content: Buffer | string) {

export async function run(params: {
name: string
onWarning: (warn: Error) => void
paths: string[]
request: RequestData
ssr: boolean
}): Promise<FetchEventResult> {
if (cache === undefined) {
const context: { [key: string]: any } = {
__next_eval__,
_ENTRIES: {},
atob: polyfills.atob,
Blob,
Expand Down Expand Up @@ -89,11 +93,21 @@ export async function run(params: {

cache = {
context,
onWarning: params.onWarning,
paths: new Map<string, string>(),
require: new Map<string, any>([
[require.resolve('next/dist/compiled/cookie'), { exports: cookie }],
]),
paths: new Map<string, string>(),
sandbox: vm.createContext(context),
sandbox: vm.createContext(context, {
codeGeneration:
process.env.NODE_ENV === 'production'
? {
strings: false,
wasm: false,
}
: undefined,
}),
warnedEvals: new Set(),
}

loadDependencies(cache.sandbox, [
Expand All @@ -110,6 +124,8 @@ export async function run(params: {
map: { Request: 'Request' },
},
])
} else {
cache.onWarning = params.onWarning
}

for (const paramPath of params.paths) {
Expand Down Expand Up @@ -218,3 +234,17 @@ function getFetchURL(input: RequestInfo, headers: NodeHeaders = {}): string {
function isRequestLike(obj: unknown): obj is Request {
return Boolean(obj && typeof obj === 'object' && 'url' in obj)
}

function __next_eval__(fn: Function) {
const key = fn.toString()
if (!cache?.warnedEvals.has(key)) {
const warning = new Error(
`Dynamic Code Evaluation (e. g. 'eval', 'new Function') not allowed in Middleware`
)
warning.name = 'DynamicCodeEvaluationWarning'
Error.captureStackTrace(warning, __next_eval__)
cache?.warnedEvals.add(key)
cache?.onWarning(warning)
}
return fn()
}
8 changes: 8 additions & 0 deletions test/integration/middleware/core/lib/utils.js
@@ -0,0 +1,8 @@
export function getTextWithEval() {
// eslint-disable-next-line no-eval
return eval('with some text')
}

export function getText() {
return 'with some text'
}
@@ -1,6 +1,7 @@
import { createElement } from 'react'
import { renderToString } from 'react-dom/server.browser'
import { NextResponse } from 'next/server'
import { getText } from '../../lib/utils'

export async function middleware(request, ev) {
// eslint-disable-next-line no-undef
Expand Down Expand Up @@ -36,7 +37,8 @@ export async function middleware(request, ev) {
ev.waitUntil(
(async () => {
writer.write(encoder.encode('this is a streamed '))
writer.write(encoder.encode('response'))
writer.write(encoder.encode('response '))
writer.write(encoder.encode(getText()))
writer.close()
})()
)
Expand Down
Expand Up @@ -254,7 +254,7 @@ function responseTests(locale = '') {
`${locale}/responses/stream-a-response`
)
const html = await res.text()
expect(html).toBe('this is a streamed response')
expect(html).toBe('this is a streamed response with some text')
})

it(`${locale} should respond with a body`, async () => {
Expand Down