Skip to content

Commit

Permalink
feat(conditional-page-builds): track potentially unsafe Node.js built…
Browse files Browse the repository at this point in the history
…in modules usage (#29560)

* feat(conditional-page-builds): track potentially unsafe Node.js builtin modules usage

* test(artifacts): add test case for tracking usage of fs in ssr

* Update packages/gatsby/src/utils/webpack.config.js

Co-authored-by: Ward Peeters <ward@coding-tech.com>

* Update packages/gatsby/src/commands/build-html.ts

Co-authored-by: Ward Peeters <ward@coding-tech.com>

* Update packages/gatsby/src/commands/build-utils.ts

Co-authored-by: Ward Peeters <ward@coding-tech.com>

Co-authored-by: Ward Peeters <ward@coding-tech.com>
  • Loading branch information
pieh and wardpeet authored Feb 22, 2021
1 parent b61af20 commit fe737d0
Show file tree
Hide file tree
Showing 21 changed files with 627 additions and 324 deletions.
124 changes: 110 additions & 14 deletions integration-tests/artifacts/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,11 @@ function assertHTMLCorrectness(runNumber) {
? `component---src-templates-deps-page-query-js`
: `component---src-templates-deps-page-query-alternative-js`

const expectedBackground =
runNumber < 6 ? `white` : runNumber === 6 ? `yellow` : `green`

let pageDataContent
let htmlContent
beforeAll(() => {
pageDataContent = fs.readJsonSync(
path.join(
Expand All @@ -180,12 +184,8 @@ function assertHTMLCorrectness(runNumber) {
`page-data.json`
)
)
})

it(`html built is using correct template (${
runNumber <= 1 ? `default` : `alternative`
})`, () => {
const htmlContent = fs.readFileSync(
htmlContent = fs.readFileSync(
path.join(
process.cwd(),
`public`,
Expand All @@ -194,7 +194,20 @@ function assertHTMLCorrectness(runNumber) {
),
`utf-8`
)
})

it(`correct css is inlined in html file (${expectedBackground})`, () => {
expect(htmlContent).toMatch(
new RegExp(
`<style>\\s*body\\s*{\\s*background:\\s*${expectedBackground};\\s*}\\s*<\\/style>`,
`gm`
)
)
})

it(`html built is using correct template (${
runNumber <= 1 ? `default` : `alternative`
})`, () => {
expect(htmlContent).toContain(
runNumber <= 1
? `<h1>Default template for depPageQuery</h1>`
Expand Down Expand Up @@ -763,21 +776,16 @@ describe(`Sixth run (ssr-only change - only ssr compilation hash changes)`, () =
]

let changedFileOriginalContent
const changedFileAbspath = path.join(
process.cwd(),
`src`,
`components`,
`post-body-components-ssr.js`
)
const changedFileAbspath = path.join(process.cwd(), `gatsby-ssr.js`)

beforeAll(async () => {
// make change to some .js
// make change to gatsby-ssr
changedFileOriginalContent = fs.readFileSync(changedFileAbspath, `utf-8`)
filesToRevert[changedFileAbspath] = changedFileOriginalContent

const newContent = changedFileOriginalContent.replace(
/SSR/g,
`SSR (see I told you)`
`\`body {\\nbackground: white;\\n}\``,
`fs.readFileSync(\`./css-to-inline.css\`, \`utf-8\`)`
)

if (newContent === changedFileOriginalContent) {
Expand Down Expand Up @@ -839,3 +847,91 @@ describe(`Sixth run (ssr-only change - only ssr compilation hash changes)`, () =

assertHTMLCorrectness(runNumber)
})

describe(`Seventh run (no change in any file that is bundled, we change untracked file, but previous build used unsafe method so all should rebuild)`, () => {
const runNumber = 7

const expectedPages = [
`/stale-pages/only-not-in-first`,
`/page-query-dynamic-7/`,
]

const unexpectedPages = [
`/stale-pages/only-in-first/`,
`/page-query-dynamic-1/`,
`/page-query-dynamic-2/`,
`/page-query-dynamic-3/`,
`/page-query-dynamic-4/`,
`/page-query-dynamic-5/`,
`/page-query-dynamic-6/`,
]

let changedFileOriginalContent
const changedFileAbspath = path.join(process.cwd(), `css-to-inline.css`)

beforeAll(async () => {
// make change to gatsby-ssr
changedFileOriginalContent = fs.readFileSync(changedFileAbspath, `utf-8`)
filesToRevert[changedFileAbspath] = changedFileOriginalContent

const newContent = changedFileOriginalContent.replace(/yellow/g, `green`)

if (newContent === changedFileOriginalContent) {
throw new Error(`Test setup failed`)
}

fs.writeFileSync(changedFileAbspath, newContent)
await runGatsbyWithRunTestSetup(runNumber)()
})

describe(`html files`, () => {
const type = `html`

describe(`should have expected html files`, () => {
assertFileExistenceForPagePaths({
pagePaths: expectedPages,
type,
shouldExist: true,
})
})

describe(`shouldn't have unexpected html files`, () => {
assertFileExistenceForPagePaths({
pagePaths: unexpectedPages,
type,
shouldExist: false,
})
})

it(`should recreate all html files`, () => {
expect(manifest[runNumber].generated.sort()).toEqual(
manifest[runNumber].allPages.sort()
)
})
})

describe(`page-data files`, () => {
const type = `page-data`

describe(`should have expected page-data files`, () => {
assertFileExistenceForPagePaths({
pagePaths: expectedPages,
type,
shouldExist: true,
})
})

describe(`shouldn't have unexpected page-data files`, () => {
assertFileExistenceForPagePaths({
pagePaths: unexpectedPages,
type,
shouldExist: false,
})
})
})

// Seventh run - no bundle should change as we don't change anything that IS bundled
assertWebpackBundleChanges({ browser: false, ssr: false, runNumber })

assertHTMLCorrectness(runNumber)
})
3 changes: 3 additions & 0 deletions integration-tests/artifacts/css-to-inline.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
body {
background: yellow;
}
13 changes: 10 additions & 3 deletions integration-tests/artifacts/gatsby-ssr.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
import PostBodyComponents from "./src/components/post-body-components-ssr"
import * as React from "react"
import * as fs from "fs"

export function onRenderBody({ setPostBodyComponents }) {
setPostBodyComponents(PostBodyComponents)
export function onRenderBody({ setHeadComponents }) {
setHeadComponents(
<style
dangerouslySetInnerHTML={{
__html: `body {\nbackground: white;\n}`,
}}
/>
)
}

This file was deleted.

2 changes: 1 addition & 1 deletion packages/gatsby-cli/src/reporter/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export function getErrorFormatter(): PrettyError {
export async function createErrorFromString(
errorStr: string = ``,
sourceMapFile: string
): Promise<Error | ErrorWithCodeFrame> {
): Promise<ErrorWithCodeFrame> {
let [message, ...rest] = errorStr.split(/\r\n|[\n\r]/g)
// pull the message from the first line then remove the `Error:` prefix
// FIXME: when https://github.com/AriaMinaei/pretty-error/pull/49 is merged
Expand Down
2 changes: 1 addition & 1 deletion packages/gatsby-cli/src/reporter/prepare-stack-trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
} from "source-map"

export class ErrorWithCodeFrame extends Error {
codeFrame = ``
codeFrame?: string = ``

constructor(error: Error) {
super(error.message)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,23 @@ exports[`develop-static-entry onPreRenderHTML can be used to replace postBodyCom
exports[`develop-static-entry onPreRenderHTML can be used to replace preBodyComponents 1`] = `"<!DOCTYPE html><html><head><meta charSet=\\"utf-8\\"/><meta http-equiv=\\"x-ua-compatible\\" content=\\"ie=edge\\"/><meta name=\\"viewport\\" content=\\"width=device-width, initial-scale=1, shrink-to-fit=no\\"/><meta name=\\"note\\" content=\\"environment=development\\"/><script src=\\"/socket.io/socket.io.js\\"></script><link rel=\\"stylesheet\\" href=\\"/commons.css\\"/></head><body><div> div3 </div><div> div2 </div><div> div1 </div><div id=\\"___gatsby\\"></div><script src=\\"/polyfill.js\\" nomodule=\\"\\"></script><script src=\\"/commons.js\\"></script></body></html>"`;
exports[`static-entry onPreRenderHTML can be used to replace headComponents 1`] = `"<!DOCTYPE html><html><head><meta charSet=\\"utf-8\\"/><meta http-equiv=\\"x-ua-compatible\\" content=\\"ie=edge\\"/><meta name=\\"viewport\\" content=\\"width=device-width, initial-scale=1, shrink-to-fit=no\\"/><link as=\\"fetch\\" rel=\\"preload\\" href=\\"/page-data/app-data.json\\" crossorigin=\\"anonymous\\"/><link as=\\"fetch\\" rel=\\"preload\\" href=\\"/page-data/about/page-data.json\\" crossorigin=\\"anonymous\\"/><style> .style3 </style><style> .style2 </style><style> .style1 </style><meta name=\\"generator\\" content=\\"Gatsby 2.0.0\\"/></head><body><div id=\\"___gatsby\\"><div style=\\"outline:none\\" tabindex=\\"-1\\" id=\\"gatsby-focus-wrapper\\"></div><div id=\\"gatsby-announcer\\" style=\\"position:absolute;top:0;width:1px;height:1px;padding:0;overflow:hidden;clip:rect(0, 0, 0, 0);white-space:nowrap;border:0\\" aria-live=\\"assertive\\" aria-atomic=\\"true\\"></div></div><script id=\\"gatsby-script-loader\\">/*<![CDATA[*/window.pagePath=\\"/about/\\";/*]]>*/</script><script id=\\"gatsby-chunk-mapping\\">/*<![CDATA[*/window.___chunkMapping={};/*]]>*/</script></body></html>"`;
exports[`static-entry onPreRenderHTML can be used to replace postBodyComponents 1`] = `"<!DOCTYPE html><html><head><meta charSet=\\"utf-8\\"/><meta http-equiv=\\"x-ua-compatible\\" content=\\"ie=edge\\"/><meta name=\\"viewport\\" content=\\"width=device-width, initial-scale=1, shrink-to-fit=no\\"/><meta name=\\"generator\\" content=\\"Gatsby 2.0.0\\"/><link as=\\"fetch\\" rel=\\"preload\\" href=\\"/page-data/about/page-data.json\\" crossorigin=\\"anonymous\\"/><link as=\\"fetch\\" rel=\\"preload\\" href=\\"/page-data/app-data.json\\" crossorigin=\\"anonymous\\"/></head><body><div id=\\"___gatsby\\"><div style=\\"outline:none\\" tabindex=\\"-1\\" id=\\"gatsby-focus-wrapper\\"></div><div id=\\"gatsby-announcer\\" style=\\"position:absolute;top:0;width:1px;height:1px;padding:0;overflow:hidden;clip:rect(0, 0, 0, 0);white-space:nowrap;border:0\\" aria-live=\\"assertive\\" aria-atomic=\\"true\\"></div></div><script id=\\"gatsby-chunk-mapping\\">/*<![CDATA[*/window.___chunkMapping={};/*]]>*/</script><script id=\\"gatsby-script-loader\\">/*<![CDATA[*/window.pagePath=\\"/about/\\";/*]]>*/</script><div> div3 </div><div> div2 </div><div> div1 </div></body></html>"`;
exports[`static-entry onPreRenderHTML can be used to replace preBodyComponents 1`] = `"<!DOCTYPE html><html><head><meta charSet=\\"utf-8\\"/><meta http-equiv=\\"x-ua-compatible\\" content=\\"ie=edge\\"/><meta name=\\"viewport\\" content=\\"width=device-width, initial-scale=1, shrink-to-fit=no\\"/><meta name=\\"generator\\" content=\\"Gatsby 2.0.0\\"/><link as=\\"fetch\\" rel=\\"preload\\" href=\\"/page-data/about/page-data.json\\" crossorigin=\\"anonymous\\"/><link as=\\"fetch\\" rel=\\"preload\\" href=\\"/page-data/app-data.json\\" crossorigin=\\"anonymous\\"/></head><body><div> div3 </div><div> div2 </div><div> div1 </div><div id=\\"___gatsby\\"><div style=\\"outline:none\\" tabindex=\\"-1\\" id=\\"gatsby-focus-wrapper\\"></div><div id=\\"gatsby-announcer\\" style=\\"position:absolute;top:0;width:1px;height:1px;padding:0;overflow:hidden;clip:rect(0, 0, 0, 0);white-space:nowrap;border:0\\" aria-live=\\"assertive\\" aria-atomic=\\"true\\"></div></div><script id=\\"gatsby-script-loader\\">/*<![CDATA[*/window.pagePath=\\"/about/\\";/*]]>*/</script><script id=\\"gatsby-chunk-mapping\\">/*<![CDATA[*/window.___chunkMapping={};/*]]>*/</script></body></html>"`;
exports[`static-entry onPreRenderHTML can be used to replace headComponents 1`] = `
Object {
"html": "<!DOCTYPE html><html><head><meta charSet=\\"utf-8\\"/><meta http-equiv=\\"x-ua-compatible\\" content=\\"ie=edge\\"/><meta name=\\"viewport\\" content=\\"width=device-width, initial-scale=1, shrink-to-fit=no\\"/><link as=\\"fetch\\" rel=\\"preload\\" href=\\"/page-data/app-data.json\\" crossorigin=\\"anonymous\\"/><link as=\\"fetch\\" rel=\\"preload\\" href=\\"/page-data/about/page-data.json\\" crossorigin=\\"anonymous\\"/><style> .style3 </style><style> .style2 </style><style> .style1 </style><meta name=\\"generator\\" content=\\"Gatsby 2.0.0\\"/></head><body><div id=\\"___gatsby\\"><div style=\\"outline:none\\" tabindex=\\"-1\\" id=\\"gatsby-focus-wrapper\\"></div><div id=\\"gatsby-announcer\\" style=\\"position:absolute;top:0;width:1px;height:1px;padding:0;overflow:hidden;clip:rect(0, 0, 0, 0);white-space:nowrap;border:0\\" aria-live=\\"assertive\\" aria-atomic=\\"true\\"></div></div><script id=\\"gatsby-script-loader\\">/*<![CDATA[*/window.pagePath=\\"/about/\\";/*]]>*/</script><script id=\\"gatsby-chunk-mapping\\">/*<![CDATA[*/window.___chunkMapping={};/*]]>*/</script></body></html>",
"unsafeBuiltinsUsage": Array [],
}
`;
exports[`static-entry onPreRenderHTML can be used to replace postBodyComponents 1`] = `
Object {
"html": "<!DOCTYPE html><html><head><meta charSet=\\"utf-8\\"/><meta http-equiv=\\"x-ua-compatible\\" content=\\"ie=edge\\"/><meta name=\\"viewport\\" content=\\"width=device-width, initial-scale=1, shrink-to-fit=no\\"/><meta name=\\"generator\\" content=\\"Gatsby 2.0.0\\"/><link as=\\"fetch\\" rel=\\"preload\\" href=\\"/page-data/about/page-data.json\\" crossorigin=\\"anonymous\\"/><link as=\\"fetch\\" rel=\\"preload\\" href=\\"/page-data/app-data.json\\" crossorigin=\\"anonymous\\"/></head><body><div id=\\"___gatsby\\"><div style=\\"outline:none\\" tabindex=\\"-1\\" id=\\"gatsby-focus-wrapper\\"></div><div id=\\"gatsby-announcer\\" style=\\"position:absolute;top:0;width:1px;height:1px;padding:0;overflow:hidden;clip:rect(0, 0, 0, 0);white-space:nowrap;border:0\\" aria-live=\\"assertive\\" aria-atomic=\\"true\\"></div></div><script id=\\"gatsby-chunk-mapping\\">/*<![CDATA[*/window.___chunkMapping={};/*]]>*/</script><script id=\\"gatsby-script-loader\\">/*<![CDATA[*/window.pagePath=\\"/about/\\";/*]]>*/</script><div> div3 </div><div> div2 </div><div> div1 </div></body></html>",
"unsafeBuiltinsUsage": Array [],
}
`;
exports[`static-entry onPreRenderHTML can be used to replace preBodyComponents 1`] = `
Object {
"html": "<!DOCTYPE html><html><head><meta charSet=\\"utf-8\\"/><meta http-equiv=\\"x-ua-compatible\\" content=\\"ie=edge\\"/><meta name=\\"viewport\\" content=\\"width=device-width, initial-scale=1, shrink-to-fit=no\\"/><meta name=\\"generator\\" content=\\"Gatsby 2.0.0\\"/><link as=\\"fetch\\" rel=\\"preload\\" href=\\"/page-data/about/page-data.json\\" crossorigin=\\"anonymous\\"/><link as=\\"fetch\\" rel=\\"preload\\" href=\\"/page-data/app-data.json\\" crossorigin=\\"anonymous\\"/></head><body><div> div3 </div><div> div2 </div><div> div1 </div><div id=\\"___gatsby\\"><div style=\\"outline:none\\" tabindex=\\"-1\\" id=\\"gatsby-focus-wrapper\\"></div><div id=\\"gatsby-announcer\\" style=\\"position:absolute;top:0;width:1px;height:1px;padding:0;overflow:hidden;clip:rect(0, 0, 0, 0);white-space:nowrap;border:0\\" aria-live=\\"assertive\\" aria-atomic=\\"true\\"></div></div><script id=\\"gatsby-script-loader\\">/*<![CDATA[*/window.pagePath=\\"/about/\\";/*]]>*/</script><script id=\\"gatsby-chunk-mapping\\">/*<![CDATA[*/window.___chunkMapping={};/*]]>*/</script></body></html>",
"unsafeBuiltinsUsage": Array [],
}
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/* eslint-disable filenames/match-regex */
const { wrapModuleWithTracking } = require(`./tracking-unsafe-module-wrapper`)

module.exports = wrapModuleWithTracking(`child_process`)
3 changes: 3 additions & 0 deletions packages/gatsby/cache-dir/ssr-builtin-trackers/fs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const { wrapModuleWithTracking } = require(`./tracking-unsafe-module-wrapper`)

module.exports = wrapModuleWithTracking(`fs`)
3 changes: 3 additions & 0 deletions packages/gatsby/cache-dir/ssr-builtin-trackers/http.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const { wrapModuleWithTracking } = require(`./tracking-unsafe-module-wrapper`)

module.exports = wrapModuleWithTracking(`http`)
3 changes: 3 additions & 0 deletions packages/gatsby/cache-dir/ssr-builtin-trackers/http2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const { wrapModuleWithTracking } = require(`./tracking-unsafe-module-wrapper`)

module.exports = wrapModuleWithTracking(`http2`)
3 changes: 3 additions & 0 deletions packages/gatsby/cache-dir/ssr-builtin-trackers/https.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const { wrapModuleWithTracking } = require(`./tracking-unsafe-module-wrapper`)

module.exports = wrapModuleWithTracking(`https`)
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// initializing global here for unsafe builtin usage at import time
global.unsafeBuiltinUsage = []

function createProxyHandler(prefix) {
return {
get: function (target, key) {
const value = target[key]
if (typeof value === `function`) {
return function wrapper(...args) {
const myErrorHolder = {
name: `Unsafe builtin usage ${prefix}.${key}`,
}
Error.captureStackTrace(myErrorHolder, wrapper)

global.unsafeBuiltinUsage.push(myErrorHolder.stack)
return value.apply(target, args)
}
} else if (typeof value === `object` && value !== null) {
return new Proxy(
value,
createProxyHandler(
key && key.toString ? `${prefix}.${key.toString()}` : prefix
)
)
}

return value
},
}
}

function wrapModuleWithTracking(moduleName) {
const mod = require(moduleName)
return new Proxy(mod, createProxyHandler(moduleName))
}

exports.wrapModuleWithTracking = wrapModuleWithTracking
Loading

0 comments on commit fe737d0

Please sign in to comment.