Skip to content

Conversation

ptrgits
Copy link

@ptrgits ptrgits commented Aug 7, 2025

fix the problem, we should avoid constructing shell commands as strings with interpolated variables and instead use the execFileSync function, which takes the command and its arguments as separate parameters. This ensures that arguments are passed directly to the executable and are not interpreted by the shell, preventing issues with spaces or special characters in paths.

Specifically, for the line:

execSync(`${WASM_OPT} ${WASM_OPT_FLAGS} --enable-simd -o ${join(WASM_OUT, 'llhttp_simd.wasm')} ${join(WASM_OUT, 'llhttp_simd.wasm')}`, { stdio: 'inherit' })

We should:

  • Parse WASM_OPT_FLAGS into an array of arguments (if it is a string).
  • Build an array of arguments: [...WASM_OPT_FLAGS, '--enable-simd', '-o', join(WASM_OUT, 'llhttp_simd.wasm'), join(WASM_OUT, 'llhttp_simd.wasm')]
  • Use execFileSync(WASM_OPT, args, { stdio: 'inherit' }) instead of execSync.

We may need to add a helper function to split flags safely (e.g., using a simple split on spaces, or a more robust shell-words parser if available, but for this fix, a simple split is acceptable given the context).

Status

@mcollina
Copy link
Member

mcollina commented Aug 7, 2025

Great find! how did you spot this?

Copy link
Member

@metcoder95 metcoder95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, just linting

Copy link
Collaborator

@Ethan-Arrowood Ethan-Arrowood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm but needs update to pass linter

@Uzlopak
Copy link
Contributor

Uzlopak commented Aug 15, 2025

@nodejs/undici PTAL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants