Skip to content

Commit

Permalink
Verify GS(S)P Serializability (#10857)
Browse files Browse the repository at this point in the history
* Verify GS(S)P Serializability

* Add support for cyclic refs

* Add unit tests

* Test for error in development mode

* Fix prerender preview tests

* Fix gssp preview tests

* fix 2x test cases

* Add desired test

* fix some more tests

* Fix route manifest expect

* Fix test expects

* Test that `getServerSideProps` does not error in production

* Test that getStaticProps is not checked in production

* Test serialization check during build

* Fix export detection for serverless

* Update test/unit/is-serializable-props.test.js

Co-Authored-By: JJ Kasper <jj@jjsweb.site>

Co-authored-by: JJ Kasper <jj@jjsweb.site>
  • Loading branch information
Timer and ijjk committed Mar 9, 2020
1 parent 745494a commit 8443a80
Show file tree
Hide file tree
Showing 16 changed files with 549 additions and 26 deletions.
7 changes: 4 additions & 3 deletions packages/next/build/webpack/loaders/next-serverless-loader.ts
Expand Up @@ -232,7 +232,8 @@ const nextServerlessLoader: loader.Loader = function() {
export const config = ComponentInfo['confi' + 'g'] || {}
export const _app = App
export async function renderReqToHTML(req, res, fromExport, _renderOpts, _params) {
export async function renderReqToHTML(req, res, renderMode, _renderOpts, _params) {
const fromExport = renderMode === 'export' || renderMode === true;
${
basePath
? `
Expand Down Expand Up @@ -327,7 +328,7 @@ const nextServerlessLoader: loader.Loader = function() {
let result = await renderToHTML(req, res, "${page}", Object.assign({}, getStaticProps ? {} : parsedUrl.query, nowParams ? nowParams : params, _params, isFallback ? { __nextFallback: 'true' } : {}), renderOpts)
if (_nextData && !fromExport) {
if (_nextData && !renderMode) {
const payload = JSON.stringify(renderOpts.pageData)
res.setHeader('Content-Type', 'application/json')
res.setHeader('Content-Length', Buffer.byteLength(payload))
Expand All @@ -349,7 +350,7 @@ const nextServerlessLoader: loader.Loader = function() {
)
}
if (fromExport) return { html: result, renderOpts }
if (renderMode) return { html: result, renderOpts }
return result
} catch (err) {
if (err.code === 'ENOENT') {
Expand Down
10 changes: 8 additions & 2 deletions packages/next/export/worker.js
Expand Up @@ -154,7 +154,13 @@ export default async function({
}

renderMethod = mod.renderReqToHTML
const result = await renderMethod(req, res, true, { ampPath }, params)
const result = await renderMethod(
req,
res,
'export',
{ ampPath },
params
)
curRenderOpts = result.renderOpts || {}
html = result.html
}
Expand Down Expand Up @@ -227,7 +233,7 @@ export default async function({
let ampHtml
if (serverless) {
req.url += (req.url.includes('?') ? '&' : '?') + 'amp=1'
ampHtml = (await renderMethod(req, res, true)).html
ampHtml = (await renderMethod(req, res, 'export')).html
} else {
ampHtml = await renderMethod(
req,
Expand Down
138 changes: 138 additions & 0 deletions packages/next/lib/is-serializable-props.ts
@@ -0,0 +1,138 @@
const regexpPlainIdentifier = /^[A-Za-z_$][A-Za-z0-9_$]*$/

function isPlainObject(value: any): boolean {
if (Object.prototype.toString.call(value) !== '[object Object]') {
return false
}

const prototype = Object.getPrototypeOf(value)
return prototype === null || prototype === Object.prototype
}

export function isSerializableProps(
page: string,
method: string,
input: any
): true {
if (!isPlainObject(input)) {
throw new SerializableError(
page,
method,
'',
`Props must be returned as a plain object from ${method}: \`{ props: { ... } }\`.`
)
}

const visited = new WeakSet()

function visit(value: any, path: string) {
if (visited.has(value)) {
throw new SerializableError(
page,
method,
path,
'Circular references cannot be expressed in JSON.'
)
}

visited.add(value)
}

function isSerializable(value: any, path: string): true {
const type = typeof value
if (
// `null` can be serialized, but not `undefined`.
value === null ||
// n.b. `bigint`, `function`, `symbol`, and `undefined` cannot be
// serialized.
//
// `object` is special-cased below, as it may represent `null`, an Array,
// a plain object, a class, et al.
type === 'boolean' ||
type === 'number' ||
type === 'string'
) {
return true
}

if (type === 'undefined') {
throw new SerializableError(
page,
method,
path,
'`undefined` cannot be serialized as JSON. Please use `null` or omit this value all together.'
)
}

if (isPlainObject(value)) {
visit(value, path)

if (
Object.entries(value).every(([key, value]) => {
const nextPath = regexpPlainIdentifier.test(key)
? `${path}.${key}`
: `${path}[${JSON.stringify(key)}]`

return (
isSerializable(key, nextPath) && isSerializable(value, nextPath)
)
})
) {
return true
}

throw new SerializableError(
page,
method,
path,
`invariant: Unknown error encountered in Object.`
)
}

if (Array.isArray(value)) {
visit(value, path)

if (
value.every((value, index) =>
isSerializable(value, `${path}[${index}]`)
)
) {
return true
}

throw new SerializableError(
page,
method,
path,
`invariant: Unknown error encountered in Array.`
)
}

// None of these can be expressed as JSON:
// const type: "bigint" | "symbol" | "object" | "function"
throw new SerializableError(
page,
method,
path,
'`' +
type +
'`' +
(type === 'object'
? ` ("${Object.prototype.toString.call(value)}")`
: '') +
' cannot be serialized as JSON. Please only return JSON serializable data types.'
)
}

return isSerializable(input, '')
}

export class SerializableError extends Error {
constructor(page: string, method: string, path: string, message: string) {
super(
path
? `Error serializing \`${path}\` returned from \`${method}\` in "${page}".\nReason: ${message}`
: `Error serializing props returned from \`${method}\` in "${page}".\nReason: ${message}`
)
}
}
4 changes: 2 additions & 2 deletions packages/next/next-server/server/next-server.ts
Expand Up @@ -925,7 +925,7 @@ export default class Server {
const renderResult = await (components.Component as any).renderReqToHTML(
req,
res,
true
'passthrough'
)

sendPayload(
Expand Down Expand Up @@ -1028,7 +1028,7 @@ export default class Server {
renderResult = await (components.Component as any).renderReqToHTML(
req,
res,
true
'passthrough'
)

html = renderResult.html
Expand Down
22 changes: 22 additions & 0 deletions packages/next/next-server/server/render.tsx
Expand Up @@ -8,6 +8,7 @@ import {
SERVER_PROPS_SSG_CONFLICT,
SSG_GET_INITIAL_PROPS_CONFLICT,
} from '../../lib/constants'
import { isSerializableProps } from '../../lib/is-serializable-props'
import { isInAmpMode } from '../lib/amp'
import { AmpStateContext } from '../lib/amp-context'
import {
Expand Down Expand Up @@ -329,6 +330,7 @@ export async function renderToHTML(
delete query.__nextFallback

const isSSG = !!getStaticProps
const isBuildTimeSSG = isSSG && renderOpts.nextExport
const defaultAppGetInitialProps =
App.getInitialProps === (App as any).origGetInitialProps

Expand Down Expand Up @@ -508,6 +510,16 @@ export async function renderToHTML(
throw new Error(invalidKeysMsg('getStaticProps', invalidKeys))
}

if (
(dev || isBuildTimeSSG) &&
!isSerializableProps(pathname, 'getStaticProps', data.props)
) {
// this fn should throw an error instead of ever returning `false`
throw new Error(
'invariant: getStaticProps did not return valid props. Please report this.'
)
}

if (typeof data.revalidate === 'number') {
if (!Number.isInteger(data.revalidate)) {
throw new Error(
Expand Down Expand Up @@ -567,6 +579,16 @@ export async function renderToHTML(
throw new Error(invalidKeysMsg('getServerSideProps', invalidKeys))
}

if (
(dev || isBuildTimeSSG) &&
!isSerializableProps(pathname, 'getServerSideProps', data.props)
) {
// this fn should throw an error instead of ever returning `false`
throw new Error(
'invariant: getServerSideProps did not return valid props. Please report this.'
)
}

props.pageProps = data.props
;(renderOpts as any).pageData = props
}
Expand Down
8 changes: 7 additions & 1 deletion test/integration/getserversideprops-preview/pages/index.js
@@ -1,5 +1,11 @@
export function getServerSideProps({ preview, previewData }) {
return { props: { hasProps: true, preview, previewData } }
return {
props: {
hasProps: true,
preview: !!preview,
previewData: previewData || null,
},
}
}

export default function({ hasProps, preview, previewData }) {
Expand Down
10 changes: 4 additions & 6 deletions test/integration/getserversideprops-preview/test/index.test.js
Expand Up @@ -53,14 +53,14 @@ function runTests(startServer = nextStart) {
const html = await renderViaHTTP(appPort, '/')
const { nextData, pre } = getData(html)
expect(nextData).toMatchObject({ isFallback: false })
expect(pre).toBe('undefined and undefined')
expect(pre).toBe('false and null')
})

it('should return page on second request', async () => {
const html = await renderViaHTTP(appPort, '/')
const { nextData, pre } = getData(html)
expect(nextData).toMatchObject({ isFallback: false })
expect(pre).toBe('undefined and undefined')
expect(pre).toBe('false and null')
})

let previewCookieString
Expand Down Expand Up @@ -198,9 +198,7 @@ function runTests(startServer = nextStart) {

await browser.get(`http://localhost:${appPort}/`)
await browser.waitForElementByCss('#props-pre')
expect(await browser.elementById('props-pre').text()).toBe(
'undefined and undefined'
)
expect(await browser.elementById('props-pre').text()).toBe('false and null')
})

afterAll(async () => {
Expand All @@ -219,7 +217,7 @@ const startServerlessEmulator = async (dir, port) => {
return initNextServerScript(scriptPath, /ready on/i, env)
}

describe('Prerender Preview Mode', () => {
describe('ServerSide Props Preview Mode', () => {
describe('Development Mode', () => {
beforeAll(async () => {
await fs.remove(nextConfigPath)
Expand Down
4 changes: 4 additions & 0 deletions test/integration/getserversideprops/pages/index.js
Expand Up @@ -14,6 +14,10 @@ const Page = ({ world, time }) => {
<>
<p>hello {world}</p>
<span>time: {time}</span>
<Link href="/non-json">
<a id="non-json">to non-json</a>
</Link>
<br />
<Link href="/another">
<a id="another">to another</a>
</Link>
Expand Down
11 changes: 11 additions & 0 deletions test/integration/getserversideprops/pages/non-json.js
@@ -0,0 +1,11 @@
export async function getServerSideProps() {
return {
props: { time: new Date() },
}
}

const Page = ({ time }) => {
return <p>hello {time.toString()}</p>
}

export default Page

0 comments on commit 8443a80

Please sign in to comment.