fix(gatsby-source-filesystem): Replace special filename characters (#34249)

Co-authored-by: LekoArts <[email protected]>
Co-authored-by: marvinjude <[email protected]>
Co-authored-by: LekoArts <[email protected]>
Co-authored-by: Michal Piechowiak <[email protected]>

Author: 3 people

packages/gatsby-core-utils/src/__tests__/fetch-remote-file.js

@@ -124,14 +124,28 @@ const server = setupServer(
       ctx.body(content)
     )
   }),
+  rest.get(
+    `http://external.com/invalid:dog*name.jpg`,
+    async (req, res, ctx) => {
+      const { content, contentLength } = await getFileContent(
+        path.join(__dirname, `./fixtures/dog-thumbnail.jpg`),
+        req
+      )
+
+      return res(
+        ctx.set(`Content-Type`, `image/jpg`),
+        ctx.set(`Content-Length`, contentLength),
+        ctx.status(200),
+        ctx.body(content)
+      )
+    }
+  ),
   rest.get(`http://external.com/dog-304.jpg`, async (req, res, ctx) => {
     const { content, contentLength } = await getFileContent(
       path.join(__dirname, `./fixtures/dog-thumbnail.jpg`),
       req
     )
 
-    // console.log(req.headers)
-
     return res(
       ctx.set(`Content-Type`, `image/jpg`),
       ctx.set(`Content-Length`, contentLength),
@@ -301,6 +315,19 @@ describe(`fetch-remote-file`, () => {
     expect(gotStream).toBeCalledTimes(1)
   })
 
+  it(`downloads and create a jpg file that has invalid characters`, async () => {
+    const filePath = await fetchRemoteFile({
+      url: `http://external.com/invalid:dog*name.jpg`,
+      cache,
+    })
+
+    expect(path.basename(filePath, `.js`)).toContain(`invalid-dog-name`)
+    expect(getFileSize(filePath)).resolves.toBe(
+      await getFileSize(path.join(__dirname, `./fixtures/dog-thumbnail.jpg`))
+    )
+    expect(gotStream).toBeCalledTimes(1)
+  })
+
   it(`doesn't retry when no content-length is given`, async () => {
     const filePath = await fetchRemoteFile({
       url: `http://external.com/logo-gzip.svg?attempts=1&maxBytes=300&contentLength=false`,

packages/gatsby-core-utils/src/__tests__/filename-utils.ts

@@ -0,0 +1,40 @@
+import * as path from "path"
+import { createFilePath, createFileHash } from "../filename-utils"
+
+describe(`createFilePath`, () => {
+  it(`handles one instance of incorrect char`, () => {
+    const filename = `some:invalid`
+    const assert = path.join(
+      `dir`,
+      `some-invalid-${createFileHash(filename)}.png`
+    )
+    expect(createFilePath(`dir`, filename, `.png`)).toBe(assert)
+  })
+  it(`handles filename without forbibben chars correctly`, () => {
+    const filename = `some-valid`
+    const assert = path.join(`dir`, `${filename}.png`)
+    expect(createFilePath(`dir`, filename, `.png`)).toBe(assert)
+  })
+  it(`handles all instances of incorrect chars`, () => {
+    const filename = `a:b*c?d"e<f>`
+    const assert = path.join(
+      `dir`,
+      `a-b-c-d-e-f--${createFileHash(filename)}.png`
+    )
+    expect(createFilePath(`dir`, filename, `.png`)).toBe(assert)
+  })
+  it(`creates different files for possible duplicates`, () => {
+    const filename1 = `some:file`
+    const filename2 = `some*file`
+    const assert1 = path.join(
+      `dir`,
+      `some-file-${createFileHash(filename1)}.png`
+    )
+    const assert2 = path.join(
+      `dir`,
+      `some-file-${createFileHash(filename2)}.png`
+    )
+    expect(createFilePath(`dir`, filename1, `.png`)).toBe(assert1)
+    expect(createFilePath(`dir`, filename2, `.png`)).toBe(assert2)
+  })
+})

packages/gatsby-core-utils/src/filename-utils.ts

@@ -1,4 +1,5 @@
 import path from "path"
+import crypto from "crypto"
 import Url from "url"
 
 /**
@@ -31,14 +32,35 @@ export function getRemoteFileName(url: string): string {
   return getParsedPath(url).name
 }
 
+export function createFileHash(input: string, length: number = 8): string {
+  return crypto
+    .createHash(`sha1`)
+    .update(input)
+    .digest(`hex`)
+    .substring(0, length)
+}
+
+const filenamePurgeRegex = /:|\/|\*|\?|"|<|>|\||\\/g
+
 /**
  * createFilePath
  * --
+ * Gets full file path while replacing forbidden characters with a `-`
  */
 export function createFilePath(
   directory: string,
   filename: string,
   ext: string
 ): string {
-  return path.join(directory, `${filename}${ext}`)
+  const purgedFileName = filename.replace(filenamePurgeRegex, `-`)
+  const shouldAddHash = purgedFileName !== filename
+
+  if (shouldAddHash) {
+    return path.join(
+      directory,
+      `${purgedFileName}-${createFileHash(filename)}${ext}`
+    )
+  } else {
+    return path.join(directory, `${filename}${ext}`)
+  }
 }

packages/gatsby-core-utils/src/index.ts

@@ -16,3 +16,4 @@ export * from "./site-metadata"
 export * from "./page-data"
 export * from "./page-html"
 export { listPlugins } from "./list-plugins"
+export { createFilePath } from "./filename-utils"

packages/gatsby-source-filesystem/README.md

@@ -162,6 +162,8 @@ When building source plugins for remote data sources such as headless CMSs, thei
 
 The `createRemoteFileNode` helper makes it easy to download remote files and add them to your site's GraphQL schema.
 
+While downloading the assets, special characters (regex: `/:|\/|\*|\?|"|<|>|\||\\/g`) in filenames are replaced with a hyphen "-". When special characters are found a file hash is added to keep files unique e.g `a:file.jpg` becomes `a-file-73hd.jpg` (as otherwise `a:file.jpg` and `a*file.jpg` would overwrite themselves).
+
 ```javascript
 createRemoteFileNode({
   // The source url of the remote file

packages/gatsby-source-filesystem/src/__tests__/create-file-node-from-buffer.js

@@ -83,7 +83,7 @@ describe(`create-file-node-from-buffer`, () => {
       })
 
       const buffer = createMockBuffer(`buffer-content`)
-      await setup({ buffer })
+      await setup({ buffer, hash: `some-hash` })
 
       expect(ensureDir).toBeCalledTimes(1)
       expect(bufferEq(buffer, output)).toBe(true)
@@ -125,6 +125,8 @@ describe(`create-file-node-from-buffer`, () => {
       expect(() => {
         createFileNodeFromBuffer({
           ...defaultArgs,
+          buffer: createMockBuffer(`some binary content`),
+          hash: `some-hash`,
           createNode: undefined,
         })
       }).toThrowErrorMatchingInlineSnapshot(
@@ -136,6 +138,8 @@ describe(`create-file-node-from-buffer`, () => {
       expect(() => {
         createFileNodeFromBuffer({
           ...defaultArgs,
+          buffer: createMockBuffer(`some binary content`),
+          hash: `some-hash`,
           createNodeId: undefined,
         })
       }).toThrowErrorMatchingInlineSnapshot(
@@ -147,6 +151,8 @@ describe(`create-file-node-from-buffer`, () => {
       expect(() => {
         createFileNodeFromBuffer({
           ...defaultArgs,
+          buffer: createMockBuffer(`some binary content`),
+          hash: `some-hash`,
           cache: undefined,
           getCache: undefined,
         })
@@ -159,6 +165,8 @@ describe(`create-file-node-from-buffer`, () => {
       expect(() => {
         createFileNodeFromBuffer({
           ...defaultArgs,
+          buffer: createMockBuffer(`some binary content`),
+          hash: `some-hash`,
           getCache: () => createMockCache(),
         })
       }).not.toThrow()
@@ -168,6 +176,8 @@ describe(`create-file-node-from-buffer`, () => {
       expect(() => {
         createFileNodeFromBuffer({
           ...defaultArgs,
+          buffer: createMockBuffer(`some binary content`),
+          hash: `some-hash`,
           cache: createMockCache(),
         })
       }).not.toThrow()

packages/gatsby-source-filesystem/src/create-file-node-from-buffer.js

@@ -3,8 +3,7 @@ const path = require(`path`)
 const fileType = require(`file-type`)
 
 const { createFileNode } = require(`./create-file-node`)
-const { createFilePath } = require(`./utils`)
-const { createContentDigest } = require(`gatsby-core-utils`)
+const { createContentDigest, createFilePath } = require(`gatsby-core-utils`)
 const cacheId = hash => `create-file-node-from-buffer-${hash}`
 
 /********************

packages/gatsby-source-filesystem/src/create-remote-file-node.js

@@ -1,10 +1,14 @@
 const fs = require(`fs-extra`)
-const { createContentDigest, fetchRemoteFile } = require(`gatsby-core-utils`)
+const {
+  createContentDigest,
+  fetchRemoteFile,
+  createFilePath,
+} = require(`gatsby-core-utils`)
 const path = require(`path`)
 const { isWebUri } = require(`valid-url`)
 const Queue = require(`fastq`)
 const { createFileNode } = require(`./create-file-node`)
-const { getRemoteFileExtension, createFilePath } = require(`./utils`)
+const { getRemoteFileExtension } = require(`./utils`)
 
 let showFlagWarning = !!process.env.GATSBY_EXPERIMENTAL_REMOTE_FILE_PLACEHOLDER
 

packages/gatsby-source-filesystem/src/utils.js

@@ -1,5 +1,6 @@
 const path = require(`path`)
 const Url = require(`url`)
+const { createFilePath } = require(`gatsby-core-utils`)
 
 /**
  * getParsedPath
@@ -40,15 +41,7 @@ export function getRemoteFileName(url) {
   return getParsedPath(url).name
 }
 
-/**
- * createFilePath
- * --
- *
- * @param  {String} directory
- * @param  {String} filename
- * @param  {String} ext
- * @return {String}
- */
-export function createFilePath(directory, filename, ext) {
-  return path.join(directory, `${filename}${ext}`)
-}
+// createFilePath should be imported from `gatsby-core-utils`
+// but some plugins already do import it from `gatsby-source-filesystem/utils`
+// so just keeping re-export here for backward compatibility
+export { createFilePath }