From 164a2da5171c3c2eb7a7df2638cd29c2fb909bcd Mon Sep 17 00:00:00 2001 From: veryspry Date: Wed, 3 Nov 2021 19:21:24 -0500 Subject: [PATCH 1/7] fix: ensure that writing node manifests to disk does not break on Windows --- packages/gatsby/src/utils/node-manifest.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/gatsby/src/utils/node-manifest.ts b/packages/gatsby/src/utils/node-manifest.ts index 9f6290e941715..f00e1cd09f149 100644 --- a/packages/gatsby/src/utils/node-manifest.ts +++ b/packages/gatsby/src/utils/node-manifest.ts @@ -279,7 +279,7 @@ export async function processNodeManifest( const gatsbySiteDirectory = store.getState().program.directory // write out the manifest file - const manifestFilePath = path.join( + let manifestFilePath = path.join( gatsbySiteDirectory, `public`, `__node-manifests`, @@ -287,6 +287,20 @@ export async function processNodeManifest( `${inputManifest.manifestId}.json` ) + /** + * Commas are considered special characters on Windows and are not valid in file paths with the exception of + * the hard disk partition name at the beginning of a file path (i.e. C:). + * + * During local development, node manifests can be written to disk but are generally unused as they are only used + * for Content Sync which runs in Gatsby cloud. Gatsby cloud is a Linux environment in which colons are valid in + * filepaths. To avoid errors on Windows, we replace all ":" instances in the filepath (with the exception of the + * hard disk partition name) with "-" to ensure that local Windows development setups do not break when attempting + * to write one of these manifests to disk. + */ + if (process.platform === `win32`) { + manifestFilePath = manifestFilePath.replace(/(? Date: Thu, 4 Nov 2021 13:13:51 -0500 Subject: [PATCH 2/7] refactor: use joinPath helper when creating node manifest file name --- packages/gatsby/src/utils/node-manifest.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/gatsby/src/utils/node-manifest.ts b/packages/gatsby/src/utils/node-manifest.ts index f00e1cd09f149..da8176a9d308e 100644 --- a/packages/gatsby/src/utils/node-manifest.ts +++ b/packages/gatsby/src/utils/node-manifest.ts @@ -1,4 +1,5 @@ import type { ErrorId } from "gatsby-cli/lib/structured-errors/error-map" +import { joinPath } from "gatsby-core-utils" import { getNode } from "./../datastore" import { IGatsbyPage, INodeManifest } from "./../redux/types" import reporter from "gatsby-cli/lib/reporter" @@ -279,7 +280,7 @@ export async function processNodeManifest( const gatsbySiteDirectory = store.getState().program.directory // write out the manifest file - let manifestFilePath = path.join( + let manifestFilePath = joinPath( gatsbySiteDirectory, `public`, `__node-manifests`, From d46fd2583e70fc64b5fc367f9a189b0cd01f78bd Mon Sep 17 00:00:00 2001 From: veryspry Date: Thu, 4 Nov 2021 13:44:43 -0500 Subject: [PATCH 3/7] feat: replace all invalid Windows file path chars when creating node manifest files --- packages/gatsby/src/utils/node-manifest.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/gatsby/src/utils/node-manifest.ts b/packages/gatsby/src/utils/node-manifest.ts index da8176a9d308e..a5f22de8e201a 100644 --- a/packages/gatsby/src/utils/node-manifest.ts +++ b/packages/gatsby/src/utils/node-manifest.ts @@ -289,17 +289,24 @@ export async function processNodeManifest( ) /** - * Commas are considered special characters on Windows and are not valid in file paths with the exception of - * the hard disk partition name at the beginning of a file path (i.e. C:). + * Windows has a handful of special/reserved characters that are not valid in a file path + * @reference https://superuser.com/questions/358855/what-characters-are-safe-in-cross-platform-file-names-for-linux-windows-and-os + * + * The two exceptions to the list linked above are + * - the colon that is part of the hard disk partition name at the beginning of a file path (i.e. C:) + * - backslashes. We don't want to replace backslashes because those are used to delineate what the actual file path is * * During local development, node manifests can be written to disk but are generally unused as they are only used - * for Content Sync which runs in Gatsby cloud. Gatsby cloud is a Linux environment in which colons are valid in - * filepaths. To avoid errors on Windows, we replace all ":" instances in the filepath (with the exception of the + * for Content Sync which runs in Gatsby Cloud. Gatsby cloud is a Linux environment in which these special chars are valid in + * filepaths. To avoid errors on Windows, we replace all instances of the special chars in the filepath (with the exception of the * hard disk partition name) with "-" to ensure that local Windows development setups do not break when attempting * to write one of these manifests to disk. */ if (process.platform === `win32`) { - manifestFilePath = manifestFilePath.replace(/(?|\|/g, + `-` + ) } const manifestFileDir = path.dirname(manifestFilePath) From ea64f944bcbee7073022faae57debd51ab4a1bbc Mon Sep 17 00:00:00 2001 From: veryspry Date: Mon, 8 Nov 2021 10:32:19 -0600 Subject: [PATCH 4/7] test: use joinPath helper in node manifest tests --- packages/gatsby/src/utils/__tests__/node-manifest.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/gatsby/src/utils/__tests__/node-manifest.ts b/packages/gatsby/src/utils/__tests__/node-manifest.ts index 2c1aded68a754..ffeae26cd5418 100644 --- a/packages/gatsby/src/utils/__tests__/node-manifest.ts +++ b/packages/gatsby/src/utils/__tests__/node-manifest.ts @@ -1,6 +1,7 @@ import { foundPageByToLogIds } from "./../node-manifest" import path from "path" import fs from "fs-extra" +import { joinPath } from "gatsby-core-utils" import reporter from "gatsby-cli/lib/reporter" import { store } from "../../redux" import { getNode } from "../../datastore" @@ -315,7 +316,7 @@ describe(`processNodeManifests`, () => { expect(fs.writeJSON).toHaveBeenNthCalledWith( index + 1, - `${path.join( + `${joinPath( process.cwd(), `public`, `__node-manifests`, From ff3fe327e3989787a2537d887be61e97b2d27e84 Mon Sep 17 00:00:00 2001 From: veryspry Date: Tue, 9 Nov 2021 10:15:58 -0600 Subject: [PATCH 5/7] feat: only replace special windows chars in the manifest filename --- .../src/utils/__tests__/node-manifest.ts | 3 +-- packages/gatsby/src/utils/node-manifest.ts | 21 ++++++++++--------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/gatsby/src/utils/__tests__/node-manifest.ts b/packages/gatsby/src/utils/__tests__/node-manifest.ts index ffeae26cd5418..2c1aded68a754 100644 --- a/packages/gatsby/src/utils/__tests__/node-manifest.ts +++ b/packages/gatsby/src/utils/__tests__/node-manifest.ts @@ -1,7 +1,6 @@ import { foundPageByToLogIds } from "./../node-manifest" import path from "path" import fs from "fs-extra" -import { joinPath } from "gatsby-core-utils" import reporter from "gatsby-cli/lib/reporter" import { store } from "../../redux" import { getNode } from "../../datastore" @@ -316,7 +315,7 @@ describe(`processNodeManifests`, () => { expect(fs.writeJSON).toHaveBeenNthCalledWith( index + 1, - `${joinPath( + `${path.join( process.cwd(), `public`, `__node-manifests`, diff --git a/packages/gatsby/src/utils/node-manifest.ts b/packages/gatsby/src/utils/node-manifest.ts index a5f22de8e201a..c1b3868eb5049 100644 --- a/packages/gatsby/src/utils/node-manifest.ts +++ b/packages/gatsby/src/utils/node-manifest.ts @@ -1,5 +1,4 @@ import type { ErrorId } from "gatsby-cli/lib/structured-errors/error-map" -import { joinPath } from "gatsby-core-utils" import { getNode } from "./../datastore" import { IGatsbyPage, INodeManifest } from "./../redux/types" import reporter from "gatsby-cli/lib/reporter" @@ -279,14 +278,7 @@ export async function processNodeManifest( const gatsbySiteDirectory = store.getState().program.directory - // write out the manifest file - let manifestFilePath = joinPath( - gatsbySiteDirectory, - `public`, - `__node-manifests`, - inputManifest.pluginName, - `${inputManifest.manifestId}.json` - ) + let fileNameBase = inputManifest.manifestId /** * Windows has a handful of special/reserved characters that are not valid in a file path @@ -303,12 +295,21 @@ export async function processNodeManifest( * to write one of these manifests to disk. */ if (process.platform === `win32`) { - manifestFilePath = manifestFilePath.replace( + fileNameBase = fileNameBase.replace( /((?|\|/g, `-` ) } + // write out the manifest file + const manifestFilePath = path.join( + gatsbySiteDirectory, + `public`, + `__node-manifests`, + inputManifest.pluginName, + `${fileNameBase}.json` + ) + const manifestFileDir = path.dirname(manifestFilePath) await fs.ensureDir(manifestFileDir) From 5d166886cc106b59bf0d02b5b2712b4c3d610d3c Mon Sep 17 00:00:00 2001 From: veryspry Date: Tue, 9 Nov 2021 12:21:03 -0600 Subject: [PATCH 6/7] feat: do not use negative look behind for Windows hard drive name --- packages/gatsby/src/utils/node-manifest.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/gatsby/src/utils/node-manifest.ts b/packages/gatsby/src/utils/node-manifest.ts index c1b3868eb5049..58ddd0b66dcab 100644 --- a/packages/gatsby/src/utils/node-manifest.ts +++ b/packages/gatsby/src/utils/node-manifest.ts @@ -295,10 +295,7 @@ export async function processNodeManifest( * to write one of these manifests to disk. */ if (process.platform === `win32`) { - fileNameBase = fileNameBase.replace( - /((?|\|/g, - `-` - ) + fileNameBase = fileNameBase.replace(/:|\/|\*|\?|"|<|>|\||\\/g, `-`) } // write out the manifest file From 8d3a6248c2fba69e909418c09a540e8eb6116974 Mon Sep 17 00:00:00 2001 From: veryspry Date: Tue, 9 Nov 2021 12:22:25 -0600 Subject: [PATCH 7/7] test: add tests for windows node manifest file name character replacing --- .../src/utils/__tests__/node-manifest.ts | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/packages/gatsby/src/utils/__tests__/node-manifest.ts b/packages/gatsby/src/utils/__tests__/node-manifest.ts index 2c1aded68a754..beaee349369e0 100644 --- a/packages/gatsby/src/utils/__tests__/node-manifest.ts +++ b/packages/gatsby/src/utils/__tests__/node-manifest.ts @@ -12,6 +12,10 @@ import { processNodeManifests, } from "../node-manifest" +function itWindows(name: string, fn: () => void): void { + return process.platform === `win32` ? it(name, fn) : xit(name, fn) +} + jest.mock(`fs-extra`, () => { return { ensureDir: jest.fn(), @@ -342,4 +346,57 @@ describe(`processNodeManifests`, () => { await testProcessNodeManifests() process.env.NODE_ENV = `test` }) + + itWindows(`replaces reserved Windows characters with a dash`, async () => { + const nodes = [ + { id: `1`, usePageContextId: true }, + { id: `2`, useOwnerNodeId: true }, + { id: `3`, useQueryTracking: true }, + ] + mockGetNodes( + new Map(nodes.map(node => [`${node.id}`, { id: `${node.id}` }])) + ) + + store.getState.mockImplementation(() => { + return { + ...storeState, + pages: new Map([ + [ + `/test`, + { + path: `/test`, + context: { id: `1` }, + }, + ], + [ + `/test-2`, + { + path: `/test-2`, + context: { id: `2` }, + }, + ], + ]), + nodeManifests: [ + { + pluginName: `test`, + node: { id: `1` }, + // A manifest id containing all of the reserved windows characters that we check + // for and replace + manifestId: `\\*"<>/:?|`, + }, + ], + queries: { + byNode: new Map([ + [`1`, new Set([`/test`, `/test-2`])], + [`2`, new Set([`/test`, `/test-2`])], + ]), + }, + } + }) + + await processNodeManifests() + const nodeManifestFileName = path.basename(fs.writeJSON.mock.calls[0][0]) + + expect(nodeManifestFileName).toEqual(`---------.json`) + }) })