From 764050b1a3a584c1086aa03c7d92c31b26eb89f5 Mon Sep 17 00:00:00 2001 From: The Falcon <57741232+FLCN-16@users.noreply.github.com> Date: Tue, 17 May 2022 21:35:33 +0530 Subject: [PATCH 1/2] Excluded `DecodeError` error from runMiddleware function --- packages/next/server/next-server.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/next/server/next-server.ts b/packages/next/server/next-server.ts index 3ce99c042e62b..a3212c182f066 100644 --- a/packages/next/server/next-server.ts +++ b/packages/next/server/next-server.ts @@ -18,7 +18,7 @@ import fs from 'fs' import { join, relative, resolve, sep } from 'path' import { IncomingMessage, ServerResponse } from 'http' -import { execOnce } from '../shared/lib/utils' +import { execOnce, DecodeError } from '../shared/lib/utils' import { addRequestMeta, getRequestMeta } from './request-meta' import { @@ -1253,11 +1253,17 @@ export default class NextNodeServer extends BaseServer { page.name = params.parsedUrl.pathname } else if (this.dynamicRoutes) { for (const dynamicRoute of this.dynamicRoutes) { - const matchParams = dynamicRoute.match(normalizedPathname) - if (matchParams) { - page.name = dynamicRoute.page - page.params = matchParams - break + try { + const matchParams = dynamicRoute.match(normalizedPathname) + if (matchParams) { + page.name = dynamicRoute.page + page.params = matchParams + break + } + } catch (err) { + if (err instanceof DecodeError === false) { + throw err + } } } } From 56eb1ecde75a85a194d2bb37a3adb6fb278af077 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Sat, 21 May 2022 22:04:15 -0500 Subject: [PATCH 2/2] Fix merge error/check and add test --- packages/next/server/dev/next-dev-server.ts | 3 + packages/next/server/next-server.ts | 143 +----------------- .../middleware-general/test/index.test.js | 11 ++ 3 files changed, 21 insertions(+), 136 deletions(-) diff --git a/packages/next/server/dev/next-dev-server.ts b/packages/next/server/dev/next-dev-server.ts index 42787442a60b8..ed4ab57806e2b 100644 --- a/packages/next/server/dev/next-dev-server.ts +++ b/packages/next/server/dev/next-dev-server.ts @@ -632,6 +632,9 @@ export default class DevServer extends Server { ) return result } catch (error) { + if (error instanceof DecodeError) { + throw error + } this.logErrorWithOriginalStack(error, undefined, 'edge-server') const preflight = diff --git a/packages/next/server/next-server.ts b/packages/next/server/next-server.ts index 3bfa1840721bf..02184f2f3c923 100644 --- a/packages/next/server/next-server.ts +++ b/packages/next/server/next-server.ts @@ -2,7 +2,7 @@ import './node-polyfill-fetch' import './node-polyfill-web-streams' import type { Route } from './router' -import type { CacheFs } from '../shared/lib/utils' +import { CacheFs, DecodeError, execOnce } from '../shared/lib/utils' import type { MiddlewareManifest } from '../build/webpack/plugins/middleware-plugin' import type RenderResult from './render-result' import type { FetchEventResult } from './web/types' @@ -18,8 +18,6 @@ import type { Params } from '../shared/lib/router/utils/route-matcher' import fs from 'fs' import { join, relative, resolve, sep } from 'path' import { IncomingMessage, ServerResponse } from 'http' - -import { execOnce, DecodeError } from '../shared/lib/utils' import { addRequestMeta, getRequestMeta } from './request-meta' import { @@ -1252,6 +1250,12 @@ export default class NextNodeServer extends BaseServer { return { finished: true } } + if (err instanceof DecodeError) { + res.statusCode = 400 + this.renderError(err, req, res, parsed.pathname || '') + return { finished: true } + } + const error = getProperError(err) console.error(error) res.statusCode = 500 @@ -1385,139 +1389,6 @@ export default class NextNodeServer extends BaseServer { } } - protected getMiddleware() { - const middleware = this.middlewareManifest?.middleware || {} - return ( - this.middlewareManifest?.sortedMiddleware.map((page) => ({ - match: getRouteMatcher( - getMiddlewareRegex(page, MIDDLEWARE_ROUTE.test(middleware[page].name)) - ), - page, - })) || [] - ) - } - - private middlewareBetaWarning = execOnce(() => { - Log.warn( - `using beta Middleware (not covered by semver) - https://nextjs.org/docs/messages/beta-middleware` - ) - }) - - protected async runMiddleware(params: { - request: BaseNextRequest - response: BaseNextResponse - parsedUrl: ParsedNextUrl - parsed: UrlWithParsedQuery - onWarning?: (warning: Error) => void - }): Promise { - this.middlewareBetaWarning() - const normalizedPathname = removePathTrailingSlash( - params.parsedUrl.pathname - ) - - // For middleware to "fetch" we must always provide an absolute URL - const url = getRequestMeta(params.request, '__NEXT_INIT_URL')! - if (!url.startsWith('http')) { - throw new Error( - 'To use middleware you must provide a `hostname` and `port` to the Next.js Server' - ) - } - - const page: { name?: string; params?: { [key: string]: string } } = {} - if (await this.hasPage(normalizedPathname)) { - page.name = params.parsedUrl.pathname - } else if (this.dynamicRoutes) { - for (const dynamicRoute of this.dynamicRoutes) { - try { - const matchParams = dynamicRoute.match(normalizedPathname) - if (matchParams) { - page.name = dynamicRoute.page - page.params = matchParams - break - } - } catch (err) { - if (err instanceof DecodeError === false) { - throw err - } - } - } - } - - const allHeaders = new Headers() - let result: FetchEventResult | null = null - const method = (params.request.method || 'GET').toUpperCase() - let originalBody = - method !== 'GET' && method !== 'HEAD' - ? clonableBodyForRequest(params.request.body) - : undefined - - for (const middleware of this.middleware || []) { - if (middleware.match(normalizedPathname)) { - if (!(await this.hasMiddleware(middleware.page, middleware.ssr))) { - console.warn(`The Edge Function for ${middleware.page} was not found`) - continue - } - - await this.ensureMiddleware(middleware.page, middleware.ssr) - const middlewareInfo = this.getMiddlewareInfo(middleware.page) - - result = await run({ - name: middlewareInfo.name, - paths: middlewareInfo.paths, - env: middlewareInfo.env, - wasm: middlewareInfo.wasm, - request: { - headers: params.request.headers, - method, - nextConfig: { - basePath: this.nextConfig.basePath, - i18n: this.nextConfig.i18n, - trailingSlash: this.nextConfig.trailingSlash, - }, - url: url, - page: page, - body: originalBody?.cloneBodyStream(), - }, - useCache: !this.nextConfig.experimental.runtime, - onWarning: (warning: Error) => { - if (params.onWarning) { - warning.message += ` "./${middlewareInfo.name}"` - params.onWarning(warning) - } - }, - }) - - for (let [key, value] of result.response.headers) { - if (key !== 'x-middleware-next') { - allHeaders.append(key, value) - } - } - - if (!this.renderOpts.dev) { - result.waitUntil.catch((error) => { - console.error(`Uncaught: middleware waitUntil errored`, error) - }) - } - - if (!result.response.headers.has('x-middleware-next')) { - break - } - } - } - - if (!result) { - this.render404(params.request, params.response, params.parsed) - } else { - for (let [key, value] of allHeaders) { - result.response.headers.set(key, value) - } - } - - await originalBody?.finalize() - - return result - } - private _cachedPreviewManifest: PrerenderManifest | undefined protected getPrerenderManifest(): PrerenderManifest { if (this._cachedPreviewManifest) { diff --git a/test/integration/middleware-general/test/index.test.js b/test/integration/middleware-general/test/index.test.js index 9a10010b50105..ebfd1106feb98 100644 --- a/test/integration/middleware-general/test/index.test.js +++ b/test/integration/middleware-general/test/index.test.js @@ -27,6 +27,7 @@ describe('Middleware Runtime', () => { describe('dev mode', () => { afterAll(() => killApp(context.app)) beforeAll(async () => { + context.dev = true context.appPort = await findPort() context.app = await launchApp(context.appDir, context.appPort, { env: { @@ -87,6 +88,7 @@ describe('Middleware Runtime', () => { stderr: build.stderr, stdout: build.stdout, } + context.dev = false context.appPort = await findPort() context.app = await nextStart(context.appDir, context.appPort, { @@ -133,6 +135,15 @@ describe('Middleware Runtime', () => { }) function tests(context, locale = '') { + it('should respond with 400 on decode failure', async () => { + const res = await fetchViaHTTP(context.appPort, `${locale}/%2`) + expect(res.status).toBe(400) + + if (!context.dev) { + expect(await res.text()).toContain('Bad Request') + } + }) + it('should set fetch user agent correctly', async () => { const res = await fetchViaHTTP( context.appPort,