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

Fix app render: escape segment value #42626

Closed
wants to merge 10 commits into from
31 changes: 31 additions & 0 deletions packages/next/client/components/app-router-headers.ts
@@ -1,5 +1,36 @@
import {
DynamicParamTypesShort,
FlightRouterState,
} from '../../server/app-render'

export const RSC = 'RSC' as const
export const NEXT_ROUTER_STATE_TREE = 'Next-Router-State-Tree' as const
export const NEXT_ROUTER_PREFETCH = 'Next-Router-Prefetch' as const
export const RSC_VARY_HEADER =
`${RSC}, ${NEXT_ROUTER_STATE_TREE}, ${NEXT_ROUTER_PREFETCH}` as const

export const escapeFlightRouterState = (
state: FlightRouterState
): FlightRouterState => {
const [segment, parallelRoutes, ...restState] = state
const escapedParallelRoutes: typeof parallelRoutes = Object.create({})
let escapedSegment: typeof segment = segment

if (typeof segment !== 'string') {
const [param, value, type] = segment
const escapedSegmentValue = encodeURIComponent(value)
escapedSegment = [
param,
escapedSegmentValue,
type as DynamicParamTypesShort,
]
}

Object.keys(parallelRoutes).forEach((key) => {
const childState = parallelRoutes[key]

escapedParallelRoutes[key] = escapeFlightRouterState(childState)
})

return [escapedSegment, escapedParallelRoutes, ...restState]
}
23 changes: 12 additions & 11 deletions packages/next/client/components/app-router.tsx
@@ -1,18 +1,18 @@
'use client'

import type { ReactNode } from 'react'
import React, { useEffect, useMemo, useCallback } from 'react'
import React, { useCallback, useEffect, useMemo } from 'react'
import { createFromFetch } from 'next/dist/compiled/react-server-dom-webpack/client'
import type {
AppRouterInstance,
CacheNode,
} from '../../shared/lib/app-router-context'
import {
AppRouterContext,
LayoutRouterContext,
GlobalLayoutRouterContext,
LayoutRouterContext,
} from '../../shared/lib/app-router-context'
import type {
CacheNode,
AppRouterInstance,
} from '../../shared/lib/app-router-context'
import type { FlightRouterState, FlightData } from '../../server/app-render'
import type { FlightData, FlightRouterState } from '../../server/app-render'
import {
ACTION_NAVIGATE,
ACTION_PREFETCH,
Expand All @@ -22,14 +22,13 @@ import {
reducer,
} from './reducer'
import {
SearchParamsContext,
// ParamsContext,
PathnameContext,
// LayoutSegmentsContext,
SearchParamsContext,
} from '../../shared/lib/hooks-client-context'
import { useReducerWithReduxDevtools } from './use-reducer-with-devtools'
import { ErrorBoundary, GlobalErrorComponent } from './error-boundary'
import {
escapeFlightRouterState,
NEXT_ROUTER_PREFETCH,
NEXT_ROUTER_STATE_TREE,
RSC,
Expand Down Expand Up @@ -65,7 +64,9 @@ export async function fetchServerResponse(
// Enable flight response
[RSC]: '1',
// Provide the current router state
[NEXT_ROUTER_STATE_TREE]: JSON.stringify(flightRouterState),
[NEXT_ROUTER_STATE_TREE]: JSON.stringify(
escapeFlightRouterState(flightRouterState)
),
}
if (prefetch) {
// Enable prefetch response
Expand Down
12 changes: 12 additions & 0 deletions test/production/app-dir-prefetch-non-iso-url/app/[slug]/page.js
@@ -0,0 +1,12 @@
export default function Slug(props) {
return (
<>
<p id="page">/[slug]</p>
<p id="props">{JSON.stringify(props)}</p>
</>
)
}

export function generateStaticParams() {
return [{ slug: 'iso-url' }, { slug: 'кириллица' }]
}
7 changes: 7 additions & 0 deletions test/production/app-dir-prefetch-non-iso-url/app/layout.js
@@ -0,0 +1,7 @@
export default function Layout({ children }) {
return (
<html>
<body>{children}</body>
</html>
)
}
18 changes: 18 additions & 0 deletions test/production/app-dir-prefetch-non-iso-url/app/page.js
@@ -0,0 +1,18 @@
import Link from 'next/link'

export default function Page(props) {
return (
<>
<p id="page">index</p>
<p id="props">{JSON.stringify(props)}</p>
<Link href="/iso-url" id="to-iso">
/iso-url
</Link>
<br />
<Link href="/кириллица" id="to-non-iso">
/кириллица
</Link>
<br />
</>
)
}
58 changes: 58 additions & 0 deletions test/production/app-dir-prefetch-non-iso-url/index.test.ts
@@ -0,0 +1,58 @@
import { createNext, FileRef } from 'e2e-utils'
import { NextInstance } from 'test/lib/next-modes/base'
import { join } from 'path'
import { BrowserInterface } from '../../lib/browsers/base'
import webdriver from 'next-webdriver'
import { waitFor } from 'next-test-utils'

describe('app-dir-prefetch-non-iso-url', () => {
let next: NextInstance

beforeAll(async () => {
next = await createNext({
files: {
'next.config.js': new FileRef(join(__dirname, 'next.config.js')),
app: new FileRef(join(__dirname, 'app')),
},
})
})
afterAll(() => next.destroy())

it('should go to iso url', async () => {
let browser: BrowserInterface

try {
browser = await webdriver(next.appPort, '/')
await browser.elementByCss('#to-iso').click()

const text = await browser.elementByCss('#page').text()

await waitFor(3000)

expect(text).toBe('/[slug]')
} finally {
if (browser) {
await browser.close()
}
}
})

it('should go to non-iso url', async () => {
let browser: BrowserInterface

try {
browser = await webdriver(next.appPort, '/')
await browser.elementByCss('#to-non-iso').click()

const text = await browser.elementByCss('#page').text()

await waitFor(3000)

expect(text).toBe('/[slug]')
} finally {
if (browser) {
await browser.close()
}
}
})
})
6 changes: 6 additions & 0 deletions test/production/app-dir-prefetch-non-iso-url/next.config.js
@@ -0,0 +1,6 @@
/** @type {import("next").NextConfig} */
const nextConfig = {
experimental: { appDir: true },
}

module.exports = nextConfig
95 changes: 95 additions & 0 deletions test/unit/app-router-headers.test.ts
@@ -0,0 +1,95 @@
import { escapeFlightRouterState } from 'next/client/components/app-router-headers'
import { FlightRouterState } from 'next/server/app-render'

const state1: FlightRouterState = [
'',
{
children: [
['slug', 'свитер-urbain', 'd'],
{
children: ['', {}],
},
],
},
null,
'refetch',
]
const state1Escaped: FlightRouterState = [
'',
{
children: [
['slug', '%D1%81%D0%B2%D0%B8%D1%82%D0%B5%D1%80-urbain', 'd'],
{
children: ['', {}],
},
],
},
null,
'refetch',
]

const state2: FlightRouterState = [
'',
{
children: [
['slug', 'свитер-urbain', 'd'],
{
children: ['', {}],
},
],
},
null,
null,
true,
]
const state2Escaped: FlightRouterState = [
'',
{
children: [
['slug', '%D1%81%D0%B2%D0%B8%D1%82%D0%B5%D1%80-urbain', 'd'],
{
children: ['', {}],
},
],
},
null,
null,
true,
]

const state3: FlightRouterState = [
'',
{
children: [
['slug', 'свитер-urbain', 'd'],
{
children: ['', {}],
},
null,
'refetch',
],
},
]
const state3Escaped: FlightRouterState = [
'',
{
children: [
['slug', '%D1%81%D0%B2%D0%B8%D1%82%D0%B5%D1%80-urbain', 'd'],
{
children: ['', {}],
},
null,
'refetch',
],
},
]

describe('escapeFlightRouterState', () => {
it.each([
{ state: state1, escapedState: state1Escaped },
{ state: state2, escapedState: state2Escaped },
{ state: state3, escapedState: state3Escaped },
])('should escape non-iso segment value', ({ state, escapedState }) => {
expect(escapeFlightRouterState(state)).toMatchObject(escapedState)
})
})