Skip to content

Commit

Permalink
Fix DecodeError from invalid URI causes 500 with middleware vs 400 wi…
Browse files Browse the repository at this point in the history
…thout (#36993)

* Excluded `DecodeError` error from runMiddleware function

* Fix merge error/check and add test

Co-authored-by: JJ Kasper <jj@jjsweb.site>
  • Loading branch information
FLCN-16 and ijjk authored May 22, 2022
1 parent 5b89366 commit 7e57432
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 3 deletions.
3 changes: 3 additions & 0 deletions packages/next/server/dev/next-dev-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
10 changes: 7 additions & 3 deletions packages/next/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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 } from '../shared/lib/utils'
import { addRequestMeta, getRequestMeta } from './request-meta'

import {
Expand Down Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions test/integration/middleware-general/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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, {
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 7e57432

Please sign in to comment.