fix(headers): add support for consistent * wildcard

src/utils/headers.js

@@ -1,94 +1,113 @@
 const fs = require('fs')
 
+const { get } = require('dot-prop')
+const escapeRegExp = require('lodash/escapeRegExp')
+const trimEnd = require('lodash/trimEnd')
+
 const TOKEN_COMMENT = '#'
 const TOKEN_PATH = '/'
 
-const matchPaths = function (rulePath, targetPath) {
-  const rulePathParts = rulePath.split('/').filter(Boolean)
-  const targetPathParts = targetPath.split('/').filter(Boolean)
-
-  if (
-    targetPathParts.length === 0 &&
-    (rulePathParts.length === 0 || (rulePathParts.length === 1 && rulePathParts[0] === '*'))
-  ) {
-    return true
+// Our production logic uses regex too
+const getRulePattern = (rule) => {
+  const ruleParts = rule.split('/').filter(Boolean)
+  if (ruleParts.length === 0) {
+    return `^/$`
   }
 
-  for (let index = 0; index < rulePathParts.length; index++) {
-    if (index >= targetPathParts.length) return false
-
-    const rulePart = rulePathParts[index]
-    const target = targetPathParts[index]
+  let pattern = '^'
 
-    if (rulePart === '*') return true
-
-    if (rulePart.startsWith(':')) {
-      if (index === rulePathParts.length - 1) {
-        return index === targetPathParts.length - 1
-      }
-      if (index === targetPathParts.length - 1) {
-        return false
+  ruleParts.forEach((part) => {
+    if (part.startsWith(':')) {
+      // :placeholder wildcard (e.g. /segment/:placeholder/test) - match everything up to a /
+      pattern += '/([^/]+)/?'
+    } else if (part === '*') {
+      // standalone asterisk wildcard (e.g. /segment/*) - match everything
+      if (pattern === '^') {
+        pattern += '/?(.*)/?'
+      } else {
+        pattern = trimEnd(pattern, '/?')
+        pattern += '(?:|/|/(.*)/?)'
       }
-    } else {
-      return rulePart === target
+    } else if (part.includes('*')) {
+      // non standalone asterisk wildcard (e.g. /segment/hello*world/test)
+      pattern += `/${part.replace(/\*/g, '(.*)')}`
+    } else if (part.trim() !== '') {
+      // not a wildcard
+      pattern += `/${escapeRegExp(part)}/?`
     }
-  }
+  })
+
+  pattern += '$'
+
+  return pattern
+}
 
-  return false
+const matchesPath = (rule, path) => {
+  const pattern = getRulePattern(rule)
+  return new RegExp(pattern, 'i').test(path)
 }
 
-const objectForPath = function (rules, pathname) {
-  return Object.entries(rules).reduce(
-    (prev, [rulePath, pathHeaders]) => ({ ...prev, ...(matchPaths(rulePath, pathname) && pathHeaders) }),
-    {},
-  )
+/**
+ * Get the matching headers for `path` given a set of `rules`.
+ *
+ * @param {Object<string,Object<string,string[]>>!} rules
+ * The rules to use for matching.
+ *
+ * @param {string!} path
+ * The path to match against.
+ *
+ * @returns {Object<string,string[]>}
+ */
+const headersForPath = function (rules, path) {
+  const matchingHeaders = Object.entries(rules)
+    .filter(([rule]) => matchesPath(rule, path))
+    .map(([, headers]) => headers)
+
+  const pathObject = Object.assign({}, ...matchingHeaders)
+  return pathObject
 }
 
 const parseHeadersFile = function (filePath) {
-  const rules = {}
-  if (!fs.existsSync(filePath)) return rules
+  if (!fs.existsSync(filePath)) {
+    return {}
+  }
   if (fs.statSync(filePath).isDirectory()) {
     console.warn('expected _headers file but found a directory at:', filePath)
-    return rules
+    return {}
   }
 
-  const lines = fs.readFileSync(filePath, { encoding: 'utf8' }).split('\n')
-  if (lines.length === 0) return rules
+  const lines = fs
+    .readFileSync(filePath, { encoding: 'utf8' })
+    .split('\n')
+    .map((line, index) => ({ line: line.trim(), index }))
+    .filter(({ line }) => Boolean(line) && !line.startsWith(TOKEN_COMMENT))
 
   let path
-  for (let index = 0; index <= lines.length; index++) {
-    if (!lines[index]) continue
-
-    const line = lines[index].trim()
-
-    if (line.startsWith(TOKEN_COMMENT) || line.length === 0) continue
+  let rules = {}
+  for (const { line, index } of lines) {
     if (line.startsWith(TOKEN_PATH)) {
-      if (line.includes('*') && line.indexOf('*') !== line.length - 1) {
-        throw new Error(
-          `invalid rule (A path rule cannot contain anything after * token) at line: ${index}\n${lines[index]}\n`,
-        )
-      }
       path = line
       continue
     }
 
-    if (!path) throw new Error('path should come before headers')
+    if (!path) {
+      throw new Error('path should come before headers')
+    }
 
     if (line.includes(':')) {
-      const sepIndex = line.indexOf(':')
-      if (sepIndex < 1) throw new Error(`invalid header at line: ${index}\n${lines[index]}\n`)
-
-      const key = line.slice(0, sepIndex).trim()
-      const value = line.slice(sepIndex + 1).trim()
-
-      if (Object.prototype.hasOwnProperty.call(rules, path)) {
-        if (Object.prototype.hasOwnProperty.call(rules[path], key)) {
-          rules[path][key].push(value)
-        } else {
-          rules[path][key] = [value]
-        }
-      } else {
-        rules[path] = { [key]: [value] }
+      const [key = '', value = ''] = line.split(':', 2)
+      const [trimmedKey, trimmedValue] = [key.trim(), value.trim()]
+      if (trimmedKey.length === 0 || trimmedValue.length === 0) {
+        throw new Error(`invalid header at line: ${index}\n${line}\n`)
+      }
+
+      const currentHeaders = get(rules, `${path}.${trimmedKey}`) || []
+      rules = {
+        ...rules,
+        [path]: {
+          ...rules[path],
+          [trimmedKey]: [...currentHeaders, trimmedValue],
+        },
       }
     }
   }
@@ -97,6 +116,7 @@ const parseHeadersFile = function (filePath) {
 }
 
 module.exports = {
-  objectForPath,
+  matchesPath,
+  headersForPath,
   parseHeadersFile,
 }

src/utils/headers.test.js

@@ -4,7 +4,7 @@ const test = require('ava')
 
 const { createSiteBuilder } = require('../../tests/utils/site-builder')
 
-const { parseHeadersFile, objectForPath } = require('./headers.js')
+const { parseHeadersFile, headersForPath, matchesPath } = require('./headers.js')
 
 const headers = [
   { path: '/', headers: ['X-Frame-Options: SAMEORIGIN'] },
@@ -21,13 +21,38 @@ const headers = [
     ],
   },
   { path: '/:placeholder/index.html', headers: ['X-Frame-Options: SAMEORIGIN'] },
+  /**
+   * Do not force * to appear at end of path.
+   *
+   * @see https://github.com/netlify/next-on-netlify/issues/151
+   * @see https://github.com/netlify/cli/issues/1148
+   */
+  {
+    path: '/*/_next/static/chunks/*',
+    headers: ['cache-control: public', 'cache-control: max-age=31536000', 'cache-control: immutable'],
+  },
+  {
+    path: '/directory/*/test.html',
+    headers: ['X-Frame-Options: test'],
+  },
 ]
 
 test.before(async (t) => {
   const builder = createSiteBuilder({ siteName: 'site-for-detecting-server' })
-  builder.withHeadersFile({
-    headers,
-  })
+  builder
+    .withHeadersFile({
+      headers,
+    })
+    .withContentFile({
+      path: '_invalid_headers',
+      content: `
+/
+  # This is valid
+  X-Frame-Options: SAMEORIGIN
+  # This is not valid
+  X-Frame-Thing:
+`,
+    })
 
   await builder.buildAsync()
 
@@ -38,7 +63,20 @@ test.after(async (t) => {
   await t.context.builder.cleanupAsync()
 })
 
-test('_headers: validate correct parsing', (t) => {
+/**
+ * Pass if we can load the test headers without throwing an error.
+ */
+test('_headers: syntax validates as expected', (t) => {
+  parseHeadersFile(path.resolve(t.context.builder.directory, '_headers'))
+})
+
+test('_headers: throws on invalid syntax', (t) => {
+  t.throws(() => parseHeadersFile(path.resolve(t.context.builder.directory, '_invalid_headers')), {
+    message: /invalid header at line: 5/,
+  })
+})
+
+test('_headers: validate rules', (t) => {
   const rules = parseHeadersFile(path.resolve(t.context.builder.directory, '_headers'))
   t.deepEqual(rules, {
     '/': {
@@ -55,32 +93,109 @@ test('_headers: validate correct parsing', (t) => {
     '/:placeholder/index.html': {
       'X-Frame-Options': ['SAMEORIGIN'],
     },
+    '/*/_next/static/chunks/*': {
+      'cache-control': ['public', 'max-age=31536000', 'immutable'],
+    },
+    '/directory/*/test.html': {
+      'X-Frame-Options': ['test'],
+    },
   })
 })
 
-test('_headers: rulesForPath testing', (t) => {
+test('_headers: headersForPath testing', (t) => {
   const rules = parseHeadersFile(path.resolve(t.context.builder.directory, '_headers'))
-  t.deepEqual(objectForPath(rules, '/'), {
+  t.deepEqual(headersForPath(rules, '/'), {
     'X-Frame-Options': ['SAMEORIGIN'],
     'X-Frame-Thing': ['SAMEORIGIN'],
   })
-  t.deepEqual(objectForPath(rules, '/placeholder'), {
+  t.deepEqual(headersForPath(rules, '/placeholder'), {
     'X-Frame-Thing': ['SAMEORIGIN'],
   })
-  t.deepEqual(objectForPath(rules, '/static-path/placeholder'), {
+  t.deepEqual(headersForPath(rules, '/static-path/placeholder'), {
     'X-Frame-Thing': ['SAMEORIGIN'],
     'X-Frame-Options': ['DENY'],
     'X-XSS-Protection': ['1; mode=block'],
     'cache-control': ['max-age=0', 'no-cache', 'no-store', 'must-revalidate'],
   })
-  t.deepEqual(objectForPath(rules, '/static-path'), {
+  t.deepEqual(headersForPath(rules, '/static-path'), {
     'X-Frame-Thing': ['SAMEORIGIN'],
     'X-Frame-Options': ['DENY'],
     'X-XSS-Protection': ['1; mode=block'],
     'cache-control': ['max-age=0', 'no-cache', 'no-store', 'must-revalidate'],
   })
-  t.deepEqual(objectForPath(rules, '/placeholder/index.html'), {
+  t.deepEqual(headersForPath(rules, '/placeholder/index.html'), {
     'X-Frame-Options': ['SAMEORIGIN'],
     'X-Frame-Thing': ['SAMEORIGIN'],
   })
+  t.deepEqual(headersForPath(rules, '/placeholder/_next/static/chunks/placeholder'), {
+    'X-Frame-Thing': ['SAMEORIGIN'],
+    'cache-control': ['public', 'max-age=31536000', 'immutable'],
+  })
+  t.deepEqual(headersForPath(rules, '/directory/placeholder/test.html'), {
+    'X-Frame-Thing': ['SAMEORIGIN'],
+    'X-Frame-Options': ['test'],
+  })
+})
+
+/**
+ * The bulk of the _headers logic concerns testing whether or not a path matches
+ * a rule - focus on testing `matchesPath` over `headersForPath` (the latter just
+ * straightforwardly combines a bunch of objects).
+ */
+test('_headers: matchesPath matches rules as expected', (t) => {
+  t.assert(matchesPath('/', '/'))
+  t.assert(matchesPath('/*', '/'))
+
+  /**
+   * Make sure (:placeholder) will NOT match root dir.
+   */
+  t.assert(!matchesPath('/:placeholder', '/'))
+
+  /**
+   * Make sure (:placeholder) will NOT recursively match subdirs.
+   */
+  t.assert(!matchesPath('/path/to/:placeholder', '/path/two/dir/one/two/three'))
+
+  /**
+   * (:placeholder) wildcard tests.
+   */
+  t.assert(matchesPath('/directory/:placeholder', '/directory/test'))
+  t.assert(matchesPath('/directory/:placeholder/test', '/directory/placeholder/test'))
+  t.assert(!matchesPath('/directory/:placeholder', '/directory/test/test'))
+  t.assert(!matchesPath('/path/to/dir/:placeholder', '/path/to/dir'))
+  t.assert(matchesPath('/path/to/dir/:placeholder', '/path/to/dir/placeholder'))
+
+  /**
+   * (*) wildcard tests.
+   */
+  t.assert(matchesPath('/path/*/dir', '/path/to/dir'))
+  t.assert(matchesPath('/path/to/*/*/*', '/path/to/one/two/three'))
+  t.assert(matchesPath('/path/*/to/*/dir', '/path/placeholder/to/placeholder/dir'))
+  t.assert(!matchesPath('/path/*/to/*/dir', '/path/placeholder/to/placeholder'))
+  t.assert(matchesPath('/path/to/dir/*', '/path/to/dir'))
+  t.assert(!matchesPath('/*test', '/'))
+  t.assert(matchesPath('/*test', '/test'))
+  t.assert(matchesPath('/*test', '/otherTest'))
+
+  /**
+   * Trailing (*) wildcard matches recursive subdirs.
+   */
+  t.assert(matchesPath('/path/*', '/path/placeholder/to/placeholder/dir'))
+  t.assert(matchesPath('/path/to/*', '/path/to/oneDir'))
+  t.assert(matchesPath('/path/to/*', '/path/to/oneDir/twoDir/threeDir'))
+
+  /**
+   * Trailing wildcards match parent dir.
+   *
+   */
+  t.assert(matchesPath('/path/to/dir/*', '/path/to/dir'))
+  t.assert(matchesPath('/path/to/dir/*/:placeholder', '/path/to/dir/test'))
+
+  /**
+   * Mixed (*) and (:placeholder) wildcards.
+   */
+  t.assert(matchesPath('/path/*/to/:placeholder/:placeholder/*', '/path/placeholder/to/placeholder/dir/test'))
+  t.assert(matchesPath('/path/*/:placeholder', '/path/to/dir'))
+  t.assert(matchesPath('/path/:placeholder/:placeholder/*', '/path/to/dir/one/two/three'))
+  t.assert(matchesPath('/path/to/dir/*/:placeholder/test', '/path/to/dir/asterisk/placeholder/test'))
 })

src/utils/proxy.js

@@ -17,7 +17,7 @@ const toReadableStream = require('to-readable-stream')
 const { readFileAsync, fileExistsAsync, isFileAsync } = require('../lib/fs.js')
 
 const { createStreamPromise } = require('./create-stream-promise')
-const { parseHeadersFile, objectForPath } = require('./headers')
+const { parseHeadersFile, headersForPath } = require('./headers')
 const { NETLIFYDEVLOG, NETLIFYDEVWARN } = require('./logo')
 const { createRewriter } = require('./rules-proxy')
 const { onChanges } = require('./rules-proxy')
@@ -291,7 +291,7 @@ const initializeProxy = function (port, distDir, projectDir) {
       }
     }
     const requestURL = new URL(req.url, `http://${req.headers.host || 'localhost'}`)
-    const pathHeaderRules = objectForPath(headerRules, requestURL.pathname)
+    const pathHeaderRules = headersForPath(headerRules, requestURL.pathname)
     if (!isEmpty(pathHeaderRules)) {
       Object.entries(pathHeaderRules).forEach(([key, val]) => {
         res.setHeader(key, val)