diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 8d43b76ceb884cd..0c4773780900ed0 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -3,7 +3,6 @@ import fs from 'fs' import Proxy from 'http-proxy' import { IncomingMessage, ServerResponse } from 'http' import { join, resolve, sep } from 'path' -import { compile as compilePathToRegex } from 'path-to-regexp' import { parse as parseQs, ParsedUrlQuery } from 'querystring' import { format as formatUrl, parse as parseUrl, UrlWithParsedQuery } from 'url' @@ -40,6 +39,7 @@ import Router, { Route, DynamicRoutes, PageChecker, + prepareDestination, } from './router' import { sendHTML } from './send-html' import { serveStatic } from './serve-static' @@ -284,10 +284,13 @@ export default class Server { } protected generateRoutes(): { - routes: Route[] + headers: Route[] + rewrites: Route[] fsRoutes: Route[] + redirects: Route[] catchAllRoute: Route pageChecker: PageChecker + useFileSystemPublicRoutes: boolean dynamicRoutes: DynamicRoutes | undefined } { this.customRoutes = this.getCustomRoutes() @@ -316,6 +319,10 @@ export default class Server { ] : [] + let headers: Route[] = [] + let rewrites: Route[] = [] + let redirects: Route[] = [] + const fsRoutes: Route[] = [ { match: route('/_next/static/:path*'), @@ -412,167 +419,109 @@ export default class Server { ...publicRoutes, ...staticFilesRoute, ] - const routes: Route[] = [] if (this.customRoutes) { - const { redirects, rewrites, headers } = this.customRoutes - const getCustomRoute = ( r: Rewrite | Redirect | Header, type: RouteType - ) => ({ - ...r, - type, - matcher: getCustomRouteMatcher(r.source), - }) + ) => + ({ + ...r, + type, + match: getCustomRouteMatcher(r.source), + name: type, + fn: async (req, res, params, parsedUrl) => ({ finished: false }), + } as Route & Rewrite & Header) // Headers come very first - routes.push( - ...headers.map(r => { - const route = getCustomRoute(r, 'header') - return { - check: true, - match: route.matcher, - type: route.type, - name: `${route.type} ${route.source} header route`, - fn: async (_req, res, _params, _parsedUrl) => { - for (const header of (route as Header).headers) { - res.setHeader(header.key, header.value) - } - return { finished: false } - }, - } as Route - }) - ) + headers = this.customRoutes.headers.map(r => { + const route = getCustomRoute(r, 'header') + return { + match: route.match, + type: route.type, + name: `${route.type} ${route.source} header route`, + fn: async (_req, res, _params, _parsedUrl) => { + for (const header of (route as Header).headers) { + res.setHeader(header.key, header.value) + } + return { finished: false } + }, + } as Route + }) - const customRoutes = [ - ...redirects.map(r => getCustomRoute(r, 'redirect')), - ...rewrites.map(r => getCustomRoute(r, 'rewrite')), - ] + redirects = this.customRoutes.redirects.map(redirect => { + const route = getCustomRoute(redirect, 'redirect') + return { + type: route.type, + match: route.match, + statusCode: route.statusCode, + name: `Redirect route`, + fn: async (_req, res, params, _parsedUrl) => { + const { parsedDestination } = prepareDestination( + route.destination, + params + ) + const updatedDestination = formatUrl(parsedDestination) - routes.push( - ...customRoutes.map(route => { - return { - check: true, - match: route.matcher, - type: route.type, - statusCode: (route as Redirect).statusCode, - name: `${route.type} ${route.source} route`, - fn: async (req, res, params, _parsedUrl) => { - const parsedDestination = parseUrl(route.destination, true) - const destQuery = parsedDestination.query - let destinationCompiler = compilePathToRegex( - `${parsedDestination.pathname!}${parsedDestination.hash || ''}` - ) - let newUrl - - Object.keys(destQuery).forEach(key => { - const val = destQuery[key] - if ( - typeof val === 'string' && - val.startsWith(':') && - params[val.substr(1)] - ) { - destQuery[key] = params[val.substr(1)] - } - }) + res.setHeader('Location', updatedDestination) + res.statusCode = getRedirectStatus(route as Redirect) - try { - newUrl = destinationCompiler(params) - } catch (err) { - if ( - err.message.match( - /Expected .*? to not repeat, but got an array/ - ) - ) { - throw new Error( - `To use a multi-match in the destination you must add \`*\` at the end of the param name to signify it should repeat. https://err.sh/zeit/next.js/invalid-multi-match` - ) - } - throw err - } + // Since IE11 doesn't support the 308 header add backwards + // compatibility using refresh header + if (res.statusCode === 308) { + res.setHeader('Refresh', `0;url=${updatedDestination}`) + } - const parsedNewUrl = parseUrl(newUrl) - const updatedDestination = formatUrl({ - ...parsedDestination, - pathname: parsedNewUrl.pathname, - hash: parsedNewUrl.hash, - search: undefined, - }) + res.end() + return { + finished: true, + } + }, + } as Route + }) - if (route.type === 'redirect') { - res.setHeader('Location', updatedDestination) - res.statusCode = getRedirectStatus(route as Redirect) - - // Since IE11 doesn't support the 308 header add backwards - // compatibility using refresh header - if (res.statusCode === 308) { - res.setHeader('Refresh', `0;url=${updatedDestination}`) - } - - res.end() - return { - finished: true, - } - } else { - // external rewrite, proxy it - if (parsedDestination.protocol) { - const proxy = new Proxy({ - target: updatedDestination, - changeOrigin: true, - ignorePath: true, - }) - proxy.web(req, res) - - proxy.on('error', (err: Error) => { - console.error( - `Error occurred proxying ${updatedDestination}`, - err - ) - }) - return { - finished: true, - } - } - ;(req as any)._nextDidRewrite = true - } + rewrites = this.customRoutes.rewrites.map(rewrite => { + const route = getCustomRoute(rewrite, 'rewrite') + return { + check: true, + type: route.type, + name: `Rewrite route`, + match: route.match, + fn: async (req, res, params, _parsedUrl) => { + const { newUrl, parsedDestination } = prepareDestination( + route.destination, + params + ) + + // external rewrite, proxy it + if (parsedDestination.protocol) { + const target = formatUrl(parsedDestination) + const proxy = new Proxy({ + target, + changeOrigin: true, + ignorePath: true, + }) + proxy.web(req, res) + proxy.on('error', (err: Error) => { + console.error(`Error occurred proxying ${target}`, err) + }) return { - finished: false, - pathname: newUrl, - query: parsedDestination.query, + finished: true, } - }, - } as Route - }) - ) - } - - const catchPublicDirectoryRoute: Route = { - match: route('/:path*'), - type: 'route', - name: 'Catch public directory route', - fn: async (req, res, params, parsedUrl) => { - const { pathname } = parsedUrl - if (!pathname) { - throw new Error('pathname is undefined') - } - - // Used in development to check public directory paths - if (await this._beforeCatchAllRender(req, res, params, parsedUrl)) { - return { - finished: true, - } - } + } + ;(req as any)._nextDidRewrite = true - return { - finished: false, - } - }, + return { + finished: false, + pathname: newUrl, + query: parsedDestination.query, + } + }, + } as Route + }) } - routes.push(catchPublicDirectoryRoute) - const catchAllRoute: Route = { match: route('/:path*'), type: 'route', @@ -602,20 +551,19 @@ export default class Server { }, } - if (this.nextConfig.useFileSystemPublicRoutes) { - this.dynamicRoutes = this.getDynamicRoutes() + const { useFileSystemPublicRoutes } = this.nextConfig - // It's very important to keep this route's param optional. - // (but it should support as many params as needed, separated by '/') - // Otherwise this will lead to a pretty simple DOS attack. - // See more: https://github.com/zeit/next.js/issues/2617 - routes.push(catchAllRoute) + if (useFileSystemPublicRoutes) { + this.dynamicRoutes = this.getDynamicRoutes() } return { - routes, + headers, fsRoutes, + rewrites, + redirects, catchAllRoute, + useFileSystemPublicRoutes, dynamicRoutes: this.dynamicRoutes, pageChecker: this.hasPage.bind(this), } diff --git a/packages/next/next-server/server/router.ts b/packages/next/next-server/server/router.ts index 75bc8faed8cad14..7d9d6b17d2e915f 100644 --- a/packages/next/next-server/server/router.ts +++ b/packages/next/next-server/server/router.ts @@ -1,5 +1,7 @@ import { IncomingMessage, ServerResponse } from 'http' -import { UrlWithParsedQuery } from 'url' +import { parse as parseUrl, UrlWithParsedQuery } from 'url' +import { compile as compilePathToRegex } from 'path-to-regexp' +import { stringify as stringifyQs } from 'querystring' import pathMatch from './lib/path-match' export const route = pathMatch() @@ -32,39 +34,91 @@ export type DynamicRoutes = Array<{ page: string; match: RouteMatch }> export type PageChecker = (pathname: string) => Promise +export const prepareDestination = (destination: string, params: Params) => { + const parsedDestination = parseUrl(destination, true) + const destQuery = parsedDestination.query + let destinationCompiler = compilePathToRegex( + `${parsedDestination.pathname!}${parsedDestination.hash || ''}` + ) + let newUrl + + Object.keys(destQuery).forEach(key => { + const val = destQuery[key] + if ( + typeof val === 'string' && + val.startsWith(':') && + params[val.substr(1)] + ) { + destQuery[key] = params[val.substr(1)] + } + }) + + try { + newUrl = destinationCompiler(params) + const [pathname, hash] = newUrl.split('#') + parsedDestination.pathname = pathname + parsedDestination.hash = `${hash ? '#' : ''}${hash || ''}` + parsedDestination.search = stringifyQs(parsedDestination.query) + parsedDestination.path = `${pathname}${parsedDestination.search}` + } catch (err) { + if (err.message.match(/Expected .*? to not repeat, but got an array/)) { + throw new Error( + `To use a multi-match in the destination you must add \`*\` at the end of the param name to signify it should repeat. https://err.sh/zeit/next.js/invalid-multi-match` + ) + } + throw err + } + return { + newUrl, + parsedDestination, + } +} + export default class Router { - routes: Route[] + headers: Route[] fsRoutes: Route[] + rewrites: Route[] + redirects: Route[] catchAllRoute: Route pageChecker: PageChecker dynamicRoutes: DynamicRoutes + useFileSystemPublicRoutes: boolean constructor({ - routes = [], + headers = [], fsRoutes = [], + rewrites = [], + redirects = [], catchAllRoute, dynamicRoutes = [], pageChecker, + useFileSystemPublicRoutes, }: { - routes: Route[] + headers: Route[] fsRoutes: Route[] + rewrites: Route[] + redirects: Route[] catchAllRoute: Route dynamicRoutes: DynamicRoutes | undefined pageChecker: PageChecker + useFileSystemPublicRoutes: boolean }) { - this.routes = routes + this.headers = headers this.fsRoutes = fsRoutes + this.rewrites = rewrites + this.redirects = redirects this.pageChecker = pageChecker this.catchAllRoute = catchAllRoute this.dynamicRoutes = dynamicRoutes + this.useFileSystemPublicRoutes = useFileSystemPublicRoutes } setDynamicRoutes(routes: DynamicRoutes = []) { this.dynamicRoutes = routes } - add(route: Route) { - this.routes.unshift(route) + addFsRoute(route: Route) { + this.fsRoutes.unshift(route) } async execute( @@ -85,7 +139,47 @@ export default class Router { let parsedUrlUpdated = parsedUrl - for (const route of [...this.fsRoutes, ...this.routes]) { + /* + Desired routes order + - headers + - redirects + - Check filesystem (including pages), if nothing found continue + - User rewrites (checking filesystem and pages each match) + */ + + const routes = [ + ...this.headers, + ...this.redirects, + ...this.fsRoutes, + // We only check the catch-all route if public page routes hasn't been + // disabled + ...(this.useFileSystemPublicRoutes + ? [ + { + type: 'route', + name: 'Page checker', + match: route('/:path*'), + fn: async (req, res, params, parsedUrl) => { + const { pathname } = parsedUrl + + if (!pathname) { + return { finished: false } + } + if (await this.pageChecker(pathname)) { + return this.catchAllRoute.fn(req, res, params, parsedUrl) + } + return { finished: false } + }, + } as Route, + ] + : []), + ...this.rewrites, + // We only check the catch-all route if public page routes hasn't been + // disabled + ...(this.useFileSystemPublicRoutes ? [this.catchAllRoute] : []), + ] + + for (const route of routes) { const newParams = route.match(parsedUrlUpdated.pathname) // Check if the match function matched diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts index 248b38f80acbe17..4339df293859aa2 100644 --- a/packages/next/server/next-dev-server.ts +++ b/packages/next/server/next-dev-server.ts @@ -106,7 +106,7 @@ export default class DevServer extends Server { const { page, query = {} } = exportPathMap[path] // We use unshift so that we're sure the routes is defined before Next's default routes - this.router.add({ + this.router.addFsRoute({ match: route(path), type: 'route', name: `${path} exportpathmap route`, @@ -336,13 +336,7 @@ export default class DevServer extends Server { } generateRoutes() { - const { - routes, - fsRoutes, - catchAllRoute, - dynamicRoutes, - pageChecker, - } = super.generateRoutes() + const { fsRoutes, ...otherRoutes } = super.generateRoutes() // In development we expose all compiled files for react-error-overlay's line show feature // We use unshift so that we're sure the routes is defined before Next's default routes @@ -359,7 +353,30 @@ export default class DevServer extends Server { }, }) - return { routes, fsRoutes, catchAllRoute, dynamicRoutes, pageChecker } + fsRoutes.push({ + match: route('/:path*'), + type: 'route', + name: 'Catchall public directory route', + fn: async (req, res, params, parsedUrl) => { + const { pathname } = parsedUrl + if (!pathname) { + throw new Error('pathname is undefined') + } + + // Used in development to check public directory paths + if (await this._beforeCatchAllRender(req, res, params, parsedUrl)) { + return { + finished: true, + } + } + + return { + finished: false, + } + }, + }) + + return { fsRoutes, ...otherRoutes } } // In development public files are not added to the router but handled as a fallback instead diff --git a/test/integration/custom-routes/next.config.js b/test/integration/custom-routes/next.config.js index db3512ce9596784..43079ebd7ae7833 100644 --- a/test/integration/custom-routes/next.config.js +++ b/test/integration/custom-routes/next.config.js @@ -7,6 +7,10 @@ module.exports = { source: '/to-another', destination: '/another/one', }, + { + source: '/nav', + destination: '/404', + }, { source: '/hello-world', destination: '/static/hello.txt', @@ -71,6 +75,10 @@ module.exports = { source: '/api-hello-param/:name', destination: '/api/hello?name=:name', }, + { + source: '/:path/post-321', + destination: '/with-params', + }, ] }, async redirects() { @@ -150,6 +158,11 @@ module.exports = { destination: '/:0', permanent: false, }, + { + source: '/redirect-override', + destination: '/thank-you-next', + permanent: false, + }, ] }, @@ -181,6 +194,15 @@ module.exports = { }, ], }, + { + source: '/:path*', + headers: [ + { + key: 'x-something', + value: 'applied-everywhere', + }, + ], + }, ] }, }, diff --git a/test/integration/custom-routes/pages/redirect-override.js b/test/integration/custom-routes/pages/redirect-override.js new file mode 100644 index 000000000000000..bb97ba14f21cb85 --- /dev/null +++ b/test/integration/custom-routes/pages/redirect-override.js @@ -0,0 +1 @@ +export default () => 'got to the page' diff --git a/test/integration/custom-routes/test/index.test.js b/test/integration/custom-routes/test/index.test.js index 1188e950aabdd20..24b7f68044e6aa2 100644 --- a/test/integration/custom-routes/test/index.test.js +++ b/test/integration/custom-routes/test/index.test.js @@ -45,6 +45,14 @@ const runTests = (isDev = false) => { expect(html).toMatch(/multi-rewrites/) }) + it('should not match dynamic route immediately after applying header', async () => { + const res = await fetchViaHTTP(appPort, '/blog/post-321') + expect(res.headers.get('x-something')).toBe('applied-everywhere') + + const $ = cheerio.load(await res.text()) + expect(JSON.parse($('p').text()).path).toBe('blog') + }) + it('should handle chained redirects successfully', async () => { const res1 = await fetchViaHTTP(appPort, '/redir-chain1', undefined, { redirect: 'manual', @@ -159,6 +167,22 @@ const runTests = (isDev = false) => { expect(html).toMatch(/second/) }) + // current routes order do not allow rewrites to override page + // but allow redirects to + it('should not allow rewrite to override page file', async () => { + const html = await renderViaHTTP(appPort, '/nav') + expect(html).toContain('to-hello') + }) + + it('show allow redirect to override the page', async () => { + const res = await fetchViaHTTP(appPort, '/redirect-override', undefined, { + redirect: 'manual', + }) + const { pathname } = url.parse(res.headers.get('location') || '') + expect(res.status).toBe(307) + expect(pathname).toBe('/thank-you-next') + }) + it('should work successfully on the client', async () => { const browser = await webdriver(appPort, '/nav') await browser.elementByCss('#to-hello').click() @@ -401,6 +425,12 @@ const runTests = (isDev = false) => { source: '/named-like-unnamed/:0', statusCode: 307, }, + { + destination: '/thank-you-next', + regex: normalizeRegEx('^\\/redirect-override$'), + source: '/redirect-override', + statusCode: 307, + }, ], headers: [ { @@ -431,6 +461,18 @@ const runTests = (isDev = false) => { regex: normalizeRegEx('^\\/my-headers(?:\\/(.*))$'), source: '/my-headers/(.*)', }, + { + headers: [ + { + key: 'x-something', + value: 'applied-everywhere', + }, + ], + regex: normalizeRegEx( + '^(?:\\/((?:[^\\/]+?)(?:\\/(?:[^\\/]+?))*))?$' + ), + source: '/:path*', + }, ], rewrites: [ { @@ -438,6 +480,11 @@ const runTests = (isDev = false) => { regex: normalizeRegEx('^\\/to-another$'), source: '/to-another', }, + { + destination: '/404', + regex: '^\\/nav$', + source: '/nav', + }, { source: '/hello-world', destination: '/static/hello.txt', @@ -526,6 +573,11 @@ const runTests = (isDev = false) => { regex: normalizeRegEx('^\\/api-hello-param(?:\\/([^\\/]+?))$'), source: '/api-hello-param/:name', }, + { + destination: '/with-params', + regex: normalizeRegEx('^(?:\\/([^\\/]+?))\\/post-321$'), + source: '/:path/post-321', + }, ], dynamicRoutes: [ { diff --git a/test/integration/invalid-multi-match/test/index.test.js b/test/integration/invalid-multi-match/test/index.test.js index 7bfddbefbb7faf9..cb4196330517ff7 100644 --- a/test/integration/invalid-multi-match/test/index.test.js +++ b/test/integration/invalid-multi-match/test/index.test.js @@ -19,7 +19,7 @@ let app const runTests = () => { it('should show error for invalid mulit-match', async () => { - await renderViaHTTP(appPort, '/hello') + await renderViaHTTP(appPort, '/random') expect(stderr).toContain( 'To use a multi-match in the destination you must add' ) @@ -27,7 +27,7 @@ const runTests = () => { }) } -describe('Custom routes', () => { +describe('Custom routes invalid multi-match', () => { describe('dev mode', () => { beforeAll(async () => { appPort = await findPort()