parseConfigGypi fails on GYP files whose single-quoted strings contain double-quote characters (e.g. canvas, 'OS=="win"' conditions)

[vbjay]

Description

parseConfigGypi in lib/create-config-gypi.js converts single-quoted GYP strings to JSON by doing a bare global substitution:

config = config.replace(/'/g, '"')

This breaks whenever a single-quoted string itself contains double-quote characters, which is ubiquitous in GYP condition strings like:

['OS=="win"', { … }]

After the substitution that becomes:

["OS==""win"", { … }]

which is invalid JSON and causes JSON.parse to throw:

SyntaxError: Expected ',' or ']' after array element in JSON at position 31 (line 3 column 12)

Affected versions

Reproduced on v12.3.0 and v13.0.0 (latest). The three-step implementation of parseConfigGypi has been unchanged since the function was introduced.

Reproduction

const { parseConfigGypi } = require('node-gyp/lib/create-config-gypi')
// Any real-world binding.gyp that uses condition strings — e.g. canvas@2.11.2
const raw = require('fs').readFileSync('/path/to/canvas/binding.gyp', 'utf8')
parseConfigGypi(raw) // → SyntaxError

canvas@2.11.2 is a good, publicly available repro: install it with --ignore-scripts, then call parseConfigGypi on its binding.gyp.

Root cause

Step 3 of parseConfigGypi is a naive global ' → " replacement. It needs to be a proper tokenizer that converts single-quoted strings to double-quoted JSON strings while escaping any double-quote characters found inside those strings.

Suggested fix

Replace the global regex with a character-by-character pass:

function parseConfigGypi(raw) {
  // 1. Strip # comments
  let src = raw.replace(/#[^\n]*/g, '')
  // 2. Join Python-style multiline string continuations
  src = src.replace(/'\\?\n\s*'/g, '')

  // 3. Tokenise: convert single-quoted strings to double-quoted JSON strings,
  //    escaping any embedded double-quote characters.
  let out = ''
  let i = 0
  while (i < src.length) {
    const ch = src[i]
    if (ch === "'") {
      let s = '"'
      i++
      while (i < src.length && src[i] !== "'") {
        if (src[i] === '"') {
          s += '\\"'
        } else if (src[i] === '\\') {
          s += src[i] + (src[i + 1] || '')
          i++
        } else {
          s += src[i]
        }
        i++
      }
      s += '"'
      out += s
      i++ // skip closing `'`
    } else if (ch === '"') {
      // Copy already-valid double-quoted strings verbatim
      out += ch
      i++
      while (i < src.length && src[i] !== '"') {
        if (src[i] === '\\') { out += src[i] + (src[i + 1] || ''); i += 2 }
        else { out += src[i]; i++ }
      }
      out += src[i] || ''
      i++
    } else {
      out += ch
      i++
    }
  }

  // 4. Strip trailing commas (also invalid in JSON, common in GYP)
  out = out.replace(/,(\s*[}\]])/g, '$1')
  return JSON.parse(out)
}

[bitpshr]

Opened #3346 to fix this.

[legendecas]

In real world, config.gypi is actually a valid JSON, see: https://github.com/nodejs/node/blob/db3a8d866a8b71a295e2f0ea7176811c3ac918fb/configure.py#L2918-L2920

Do you encounter any issue with a real world reproduction with node-gyp command line, like node-gyp rebuild with a problematic binding.gyp?

[vbjay]

The context is I was trying to parse it to see the details in a security analysis task. So if there is a different way to get to parse the config without actually building, that would be great. This is tied to approve scripts functionality and getting a report of what lifecycle scripts do in mpm. I'd prefer to not run processes and instead use the parser available.

[legendecas]

parseConfigGypi is a private helper, not a general GYP parser. It only exists to read the config.gypi from the official Node.js release products. A general gyp file may contain command expansions like <!(cmd) or variable expansions <(var), or gyp file inclusions. Non of these are supported in parseConfigGypi.

node-gyp uses https://github.com/nodejs/gyp-next to evaluate general gyp files.

I'm inclined to close this as not-a-bug of node-gyp.

[vbjay]

I see thanks. Appreciate the pointer. I'll look there. I was digging into how do I reuse node gyp stuff somehow. Just missed the next package.