Skip to main content
added 17 characters in body
Source Link
functionconst processFile = path => (file) => {
    const dom = new JSDOM(file)
    const scripts = generateHashes(dom, 'script', element => element.innerHTML)
    const styles = generateHashes(dom, 'style', element => element.innerHTML)
    const inlineStyles = generateHashes(dom, '[style]'element => element.getAttribute('style'))

    const webPath = path.replace(new RegExp(`^${buildDir}(.*)index\\.html$`), '$1')
    const cspString = buildCSPArray(mergedPolicies, setAllPolicies, {
        scriptSrc: scripts,
        styleSrc: [...styles, ...inlineStyles],
    }).join(' ')
    return { webPath, cspString };
}
const pathsAndCSPs = await Promise.all(
    paths.map(
        path => fs.promises.readFile(path).then(processFile(path))
    )
);
const combinedCSP = pathsAndCSPs
    .map(
        ({ webPath, cspString }) => `${webPath}\n  Content-Security-Policy: ${cspString}`
    )
    .join('\n');
function processFile(file) {
    const dom = new JSDOM(file)
    const scripts = generateHashes(dom, 'script', element => element.innerHTML)
    const styles = generateHashes(dom, 'style', element => element.innerHTML)
    const inlineStyles = generateHashes(dom, '[style]'element => element.getAttribute('style'))

    const webPath = path.replace(new RegExp(`^${buildDir}(.*)index\\.html$`), '$1')
    const cspString = buildCSPArray(mergedPolicies, setAllPolicies, {
        scriptSrc: scripts,
        styleSrc: [...styles, ...inlineStyles],
    }).join(' ')
    return { webPath, cspString };
}
const pathsAndCSPs = await Promise.all(
    paths.map(
        path => fs.promises.readFile(path).then(processFile)
    )
);
const combinedCSP = pathsAndCSPs
    .map(
        ({ webPath, cspString }) => `${webPath}\n  Content-Security-Policy: ${cspString}`
    )
    .join('\n');
const processFile = path => (file) => {
    const dom = new JSDOM(file)
    const scripts = generateHashes(dom, 'script', element => element.innerHTML)
    const styles = generateHashes(dom, 'style', element => element.innerHTML)
    const inlineStyles = generateHashes(dom, '[style]'element => element.getAttribute('style'))

    const webPath = path.replace(new RegExp(`^${buildDir}(.*)index\\.html$`), '$1')
    const cspString = buildCSPArray(mergedPolicies, setAllPolicies, {
        scriptSrc: scripts,
        styleSrc: [...styles, ...inlineStyles],
    }).join(' ')
    return { webPath, cspString };
}
const pathsAndCSPs = await Promise.all(
    paths.map(
        path => fs.promises.readFile(path).then(processFile(path))
    )
);
const combinedCSP = pathsAndCSPs
    .map(
        ({ webPath, cspString }) => `${webPath}\n  Content-Security-Policy: ${cspString}`
    )
    .join('\n');
deleted 210 characters in body
Source Link
  • const sha256 = require('js-sha256').sha256 simplifies to const { sha256 } = require('js-sha256')
  • I refactored it out above, but your initial paths.reduce has an initial value of an empty array, but then +=s to it and returns a string. You probably wanted an initial value of an empty string instead.
  • Depends on the OS, but make sure that buildDir doesn't have any backslashes. Or, if it does have backslashes, make sure that they get double-escaped, else there will be problems when passed to new RegExp.
  • The code works, but toKebabCase requires an input of something in camelCase, maybe call it camelCaseToKebabCase to be explicit.
  • const sha256 = require('js-sha256').sha256 simplifies to const { sha256 } = require('js-sha256')
  • I refactored it out above, but your initial paths.reduce has an initial value of an empty array, but then +=s to it and returns a string. You probably wanted an initial value of an empty string instead.
  • Depends on the OS, but make sure that buildDir doesn't have any backslashes. Or, if it does have backslashes, make sure that they get double-escaped, else there will be problems when passed to new RegExp.
  • The code works, but toKebabCase requires an input of something in camelCase, maybe call it camelCaseToKebabCase to be explicit.
  • const sha256 = require('js-sha256').sha256 simplifies to const { sha256 } = require('js-sha256')
  • Depends on the OS, but make sure that buildDir doesn't have any backslashes. Or, if it does have backslashes, make sure that they get double-escaped, else there will be problems when passed to new RegExp.
  • The code works, but toKebabCase requires an input of something in camelCase, maybe call it camelCaseToKebabCase to be explicit.
Source Link

The main query is the two generateHashesFromElement and generateHashesFromElement functions, which are identical apart from one uses .innerHTML and the other uses .getAttribute('style'). If possible, I'd like to refactor these to be more DRY.

There are 2 differences: the if condition that gets checked, and the generation of the hash inside the if block. You can make a general function by passing it one callback, one which maps the element to the value you want to check on it:

function generateHashes (dom, selector, getPropertyValue) {
  const hashes = new Set()  
  for (const matchedElement of dom.window.document.querySelectorAll(selector)) {    
    const value = getPropertyValue(matchedElement)
    if (value.length) { 
      const hash = sha256.arrayBuffer(value)
      const base64hash = Buffer.from(hash).toString('base64')   
      hashes.add(`'sha256-${base64hash}'`)  
    }   
  } 
  return Array.from(hashes) 
}

Then use it with

generateHashes(dom, 'script', element => element.innerHTML)
generateHashes(dom, '[style]', element => element.getAttribute('style'))

Is this code readable, or should I add comments?

It looks pretty good. The consoles are useful in that they not only inform the user what's currently going on, but they also show the reader of the code the intent of that section. My only significant question while reading was: what's the function signature of onPostBuild? Maybe Netlify script-writers know at a glance, but I'm not one of them, and it wasn't immediately obvious what the type of the inputs object's values are. You tell us what they are in the question text, but not in the code. (But it's probably fine if you document them elsewhere and have a comment pointing to it)

  const { buildDir, exclude, policies, setAllPolicies } = 

A bit of JSDoc describing parameters and a section of example output might help to make things clearer, for onPostBuild and for buildCSPArray.

Is the code efficient?

Not so much. The main culprit is:

const file = paths.reduce((final, path) => {
  const file = fs.readFileSync(path, 'utf-8')
  // do stuff with file
  return final
}, [])

readFileSync is blocking; the script waits for files to be read one-by-one, and once one is read, it has to finish processing it completely before starting to read the next file. If there are lots of files to be read, this could take a moderate amount of time. Consider processing files in parallel rather than in serial. To do this, use fs.promises and Promise.all to wait for each file to finish, probably something like:

function processFile(file) {
    const dom = new JSDOM(file)
    const scripts = generateHashes(dom, 'script', element => element.innerHTML)
    const styles = generateHashes(dom, 'style', element => element.innerHTML)
    const inlineStyles = generateHashes(dom, '[style]'element => element.getAttribute('style'))

    const webPath = path.replace(new RegExp(`^${buildDir}(.*)index\\.html$`), '$1')
    const cspString = buildCSPArray(mergedPolicies, setAllPolicies, {
        scriptSrc: scripts,
        styleSrc: [...styles, ...inlineStyles],
    }).join(' ')
    return { webPath, cspString };
}
const pathsAndCSPs = await Promise.all(
    paths.map(
        path => fs.promises.readFile(path).then(processFile)
    )
);
const combinedCSP = pathsAndCSPs
    .map(
        ({ webPath, cspString }) => `${webPath}\n  Content-Security-Policy: ${cspString}`
    )
    .join('\n');

Other nitpicks:

  • const sha256 = require('js-sha256').sha256 simplifies to const { sha256 } = require('js-sha256')
  • I refactored it out above, but your initial paths.reduce has an initial value of an empty array, but then +=s to it and returns a string. You probably wanted an initial value of an empty string instead.
  • Depends on the OS, but make sure that buildDir doesn't have any backslashes. Or, if it does have backslashes, make sure that they get double-escaped, else there will be problems when passed to new RegExp.
  • The code works, but toKebabCase requires an input of something in camelCase, maybe call it camelCaseToKebabCase to be explicit.

buildCSPArray: I don't think reduce is appropriate when the accumulator being passed through each iteration is the same object. Here's a discussion by Chrome developers on the subject. I'd prefer declaring an array outside, then iterating with for..of. Or, even better, given what's going on inside the loop here, you could use filter and map - also, use Object.entries to retrieve both the key and value at once, instead of using Object.keys followed by obj[key] lookup. In addition, since the left and right side of the string being pushed are the same, and because optional chaining exists, you can construct the string in a much more concise manner:

return Object.entries(allPolicies)
    .filter(([key, defaultPolicy]) => hashes[key] || defaultPolicy || setAllPolicies)
    .map(([key, defaultPolicy]) => {
        const policy = `${hashes[key]?.join(' ') || ''} ${defaultPolicy};`;
        return `${camelCaseToKebabCase(key)} ${policy};`
    })