Skip to content

Commit

Permalink
Add handling for repeated slashes (#27738)
Browse files Browse the repository at this point in the history
This adds handling for repeated forward/back slashes in Next.js, when these slashes are detected in a request to Next.js we will automatically remove the additional slashes redirecting with a 308 status code which prevents duplicate content when being crawled by search engines. 

Fixes: #13011
Fixes: #23772
Closes: #15171
Closes: #25745
  • Loading branch information
ijjk committed Aug 3, 2021
1 parent 61586ed commit 4ab41ef
Show file tree
Hide file tree
Showing 13 changed files with 688 additions and 32 deletions.
7 changes: 1 addition & 6 deletions packages/next/client/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -188,12 +188,7 @@ class Container extends React.Component<{
// the asPath unexpectedly e.g. adding basePath when
// it wasn't originally present
page !== '/404' &&
!(
page === '/_error' &&
hydrateProps &&
hydrateProps.pageProps &&
hydrateProps.pageProps.statusCode === 404
) &&
page !== '/_error' &&
(isFallback ||
(data.nextExport &&
(isDynamicRoute(router.pathname) ||
Expand Down
27 changes: 25 additions & 2 deletions packages/next/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import {
isResSent,
NextApiRequest,
NextApiResponse,
normalizeRepeatedSlashes,
} from '../shared/lib/utils'
import {
apiResolver,
Expand Down Expand Up @@ -313,6 +314,18 @@ export default class Server {
res: ServerResponse,
parsedUrl?: UrlWithParsedQuery
): Promise<void> {
const urlParts = (req.url || '').split('?')
const urlNoQuery = urlParts[0]

if (urlNoQuery?.match(/(\\|\/\/)/)) {
const cleanUrl = normalizeRepeatedSlashes(req.url!)
res.setHeader('Location', cleanUrl)
res.setHeader('Refresh', `0;url=${cleanUrl}`)
res.statusCode = 308
res.end(cleanUrl)
return
}

setLazyProp({ req: req as any }, 'cookies', getCookieParser(req.headers))

// Parse url if parsedUrl not provided
Expand Down Expand Up @@ -811,7 +824,13 @@ export default class Server {

parsedDestination.search = stringifyQuery(req, query)

const updatedDestination = formatUrl(parsedDestination)
let updatedDestination = formatUrl(parsedDestination)

if (updatedDestination.startsWith('/')) {
updatedDestination = normalizeRepeatedSlashes(
updatedDestination
)
}

res.setHeader('Location', updatedDestination)
res.statusCode = getRedirectStatus(redirectRoute as Redirect)
Expand All @@ -822,7 +841,7 @@ export default class Server {
res.setHeader('Refresh', `0;url=${updatedDestination}`)
}

res.end()
res.end(updatedDestination)
return {
finished: true,
}
Expand Down Expand Up @@ -1519,6 +1538,10 @@ export default class Server {
redirect.destination = `${basePath}${redirect.destination}`
}

if (redirect.destination.startsWith('/')) {
redirect.destination = normalizeRepeatedSlashes(redirect.destination)
}

if (statusCode === PERMANENT_REDIRECT_STATUS) {
res.setHeader('Refresh', `0;url=${redirect.destination}`)
}
Expand Down
30 changes: 24 additions & 6 deletions packages/next/shared/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
getLocationOrigin,
getURL,
loadGetInitialProps,
normalizeRepeatedSlashes,
NextPageContext,
ST,
NEXT_DATA,
Expand Down Expand Up @@ -274,8 +275,29 @@ export function resolveHref(
): string {
// we use a dummy base url for relative urls
let base: URL
const urlAsString =
typeof href === 'string' ? href : formatWithValidation(href)
let urlAsString = typeof href === 'string' ? href : formatWithValidation(href)

// repeated slashes and backslashes in the URL are considered
// invalid and will never match a Next.js page/file
const urlProtoMatch = urlAsString.match(/^[a-zA-Z]{1,}:\/\//)
const urlAsStringNoProto = urlProtoMatch
? urlAsString.substr(urlProtoMatch[0].length)
: urlAsString

const urlParts = urlAsStringNoProto.split('?')

if ((urlParts[0] || '').match(/(\/\/|\\)/)) {
console.error(
`Invalid href passed to next/router: ${urlAsString}, repeated forward-slashes (//) or backslashes \\ are not valid in the href`
)
const normalizedUrl = normalizeRepeatedSlashes(urlAsStringNoProto)
urlAsString = (urlProtoMatch ? urlProtoMatch[0] : '') + normalizedUrl
}

// Return because it cannot be routed by the Next.js router
if (!isLocalURL(urlAsString)) {
return (resolveAs ? [urlAsString] : urlAsString) as string
}

try {
base = new URL(
Expand All @@ -286,10 +308,6 @@ export function resolveHref(
// fallback to / for invalid asPath values e.g. //
base = new URL('/', 'http://n')
}
// Return because it cannot be routed by the Next.js router
if (!isLocalURL(urlAsString)) {
return (resolveAs ? [urlAsString] : urlAsString) as string
}
try {
const finalUrl = new URL(urlAsString, base)
finalUrl.pathname = normalizePathTrailingSlash(finalUrl.pathname)
Expand Down
14 changes: 14 additions & 0 deletions packages/next/shared/lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,20 @@ export function isResSent(res: ServerResponse) {
return res.finished || res.headersSent
}

export function normalizeRepeatedSlashes(url: string) {
const urlParts = url.split('?')
const urlNoQuery = urlParts[0]

return (
urlNoQuery
// first we replace any non-encoded backslashes with forward
// then normalize repeated forward slashes
.replace(/\\/g, '/')
.replace(/\/\/+/g, '/') +
(urlParts[1] ? `?${urlParts.slice(1).join('?')}` : '')
)
}

export async function loadGetInitialProps<
C extends BaseContext,
IP = {},
Expand Down
6 changes: 0 additions & 6 deletions test/integration/500-page/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {
getPageFileFromPagesManifest,
getPagesManifest,
updatePagesManifest,
check,
} from 'next-test-utils'

jest.setTimeout(1000 * 60 * 2)
Expand Down Expand Up @@ -245,11 +244,6 @@ describe('500 Page Support', () => {
const browser = await webdriver(appPort, '/err?hello=world')
const initialTitle = await browser.eval('document.title')

await check(async () => {
const query = await browser.eval(`window.next.router.query`)
return query.hello === 'world' ? 'success' : 'not yet'
}, 'success')

const currentTitle = await browser.eval('document.title')

expect(initialTitle).toBe(currentTitle)
Expand Down
33 changes: 23 additions & 10 deletions test/integration/file-serving/test/index.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-env jest */

/* eslint-disable jest/no-identical-title */

import url from 'url'
import fs from 'fs-extra'
import { join } from 'path'
import {
Expand All @@ -22,19 +22,32 @@ let app
const expectStatus = async (path) => {
const containRegex = /(This page could not be found|Bad Request)/
// test base mount point `public/`
const res = await fetchViaHTTP(appPort, path)
expect(res.status === 400 || res.status === 404).toBe(true)
expect(await res.text()).toMatch(containRegex)
const checkRes = async (res) => {
if (res.status === 308) {
const redirectDest = res.headers.get('location')
const parsedUrl = url.parse(redirectDest, true)
expect(parsedUrl.hostname).toBe('localhost')
} else {
expect(res.status === 400 || res.status === 404).toBe(true)
expect(await res.text()).toMatch(containRegex)
}
}
const res = await fetchViaHTTP(appPort, path, undefined, {
redirect: 'manual',
})
await checkRes(res)

// test `/_next` mount point
const res2 = await fetchViaHTTP(appPort, `/_next/${path}`)
expect(res2.status === 400 || res2.status === 404).toBe(true)
expect(await res2.text()).toMatch(containRegex)
const res2 = await fetchViaHTTP(appPort, `/_next/${path}`, undefined, {
redirect: 'manual',
})
await checkRes(res2)

// test `/static` mount point
const res3 = await fetchViaHTTP(appPort, `/static/${path}`)
expect(res3.status === 400 || res3.status === 404).toBe(true)
expect(await res3.text()).toMatch(containRegex)
const res3 = await fetchViaHTTP(appPort, `/static/${path}`, undefined, {
redirect: 'manual',
})
await checkRes(res3)
}

const runTests = () => {
Expand Down
16 changes: 16 additions & 0 deletions test/integration/repeated-slashes/app/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
module.exports = {
redirects() {
return [
{
source: '/redirect-forward-slashes',
destination: '/test//google.com',
permanent: false,
},
{
source: '/redirect-back-slashes',
destination: '/test\\/google.com',
permanent: false,
},
]
},
}
7 changes: 7 additions & 0 deletions test/integration/repeated-slashes/app/pages/_error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
if (typeof window !== 'undefined') {
window.errorLoad = true
}

export default function Page() {
return <p id="error">custom error</p>
}
3 changes: 3 additions & 0 deletions test/integration/repeated-slashes/app/pages/another.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Another() {
return <p id="another">another page</p>
}
3 changes: 3 additions & 0 deletions test/integration/repeated-slashes/app/pages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Index(props) {
return <p id="index">index page</p>
}
58 changes: 58 additions & 0 deletions test/integration/repeated-slashes/app/pages/invalid.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import Link from 'next/link'

if (typeof window !== 'undefined') {
window.caughtErrors = []
const origError = window.console.error

window.console.error = function (...args) {
window.caughtErrors.push(args.join(' '))
return origError(...args)
}
}

export default function Invalid() {
return (
<>
<p id="invalid">invalid page</p>
<Link href="/another" as="//google.com">
<a id="page-with-as-slashes">to /another as //google.com</a>
</Link>
<br />

<Link href="//google.com">
<a id="href-with-slashes">to //google.com</a>
</Link>
<br />

<Link href="//google.com?hello=1">
<a id="href-with-slashes-query">to //google.com?hello=1</a>
</Link>
<br />

<Link href="//google.com#hello">
<a id="href-with-slashes-hash">to //google.com#hello</a>
</Link>
<br />

<Link href="/another" as="\/\/google.com">
<a id="page-with-as-backslashes">to /another as \\/\\/google.com</a>
</Link>
<br />

<Link href="\/\/google.com">
<a id="href-with-backslashes">to \\/\\/google.com</a>
</Link>
<br />

<Link href="\/\/google.com?hello=1">
<a id="href-with-backslashes-query">to \\/\\/google.com?hello=1</a>
</Link>
<br />

<Link href="\/\/google.com#hello">
<a id="href-with-backslashes-hash">to \\/\\/google.com#hello</a>
</Link>
<br />
</>
)
}

0 comments on commit 4ab41ef

Please sign in to comment.