fix: handle proper tokenization for --extra-params flag#2822
Open
valeriobelli wants to merge 1 commit into
Open
fix: handle proper tokenization for --extra-params flag#2822valeriobelli wants to merge 1 commit into
--extra-params flag#2822valeriobelli wants to merge 1 commit into
Conversation
`--extra-params` was split naively on spaces (`val.split(' ')`), so any
argument containing a legitimate space was broken apart and the quote
characters leaked through to xcodebuild/gradle, e.g.:
--extra-params='-destination "iOS Simulator" CODE_SIGNING_ALLOWED=NO'
--extra-params='EXCLUDED_ARCHS="arm64 i386"'
Add a shared `tokenize` utility in `cli-tools` and use it as the `parse`
callback for `--extra-params` in both the Apple command (buildOptions)
and the Android command (buildAndroid, which is also reused by
run-android).
`tokenize` reproduces the argument splitting a POSIX shell performs when
it builds the argv for a program, limited to the parts that make sense
for a value handed straight to a spawned tool:
- single and double quotes group values and are removed;
- a backslash escapes the next character on POSIX, while on Windows it
stays literal so native paths such as `C:\keys\app.jks` survive
without quoting;
- adjacent quoted and unquoted segments join into a single argument;
- an unterminated quote throws a clear CLIError.
There is deliberately no expansion phase: variables (`$FOO`), command
substitution (`$(...)`), globs and operators are passed through verbatim
so values such as `OTHER_LDFLAGS=$(inherited)` reach the tool unchanged
and are interpreted by xcodebuild/gradle themselves. The arguments are
spawned without a shell, so nothing is re-expanded downstream.
The xcodebuild build log now wraps arguments containing whitespace in
quotes so the printed command stays copy-pasteable.
valeriobelli
force-pushed
the
handle-proper-tokenization-for-extra-params
branch
from
July 25, 2026 15:49
5b552c9 to
7be8934
Compare
valeriobelli
marked this pull request as ready for review
July 25, 2026 15:54
valeriobelli
requested review from
cipolleschi,
cortinico and
thymikee
as code owners
July 25, 2026 15:54
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--extra-paramswas split naively on spaces (val.split(' ')), which broke any argument containing a legitimate space and leaked the quote characters through, e.g.:Replace the split with a shared, quote-aware
tokenizeutility incli-tools. It honors single/double quotes, joins adjacent quoted and unquoted segments into one token, and does no shell interpretation; values likeOTHER_LDFLAGS=$(inherited)and Windows Gradle paths (C:\keys\app.jks) reach the tool verbatim. Empty/whitespace-only arguments yield no arguments, and an unterminated quote throws a clear error.Wired into XcodeBuild (
buildOptions) and Gradle (buildAndroid, which is also reused byrun-androidcommand). The Xcode build log now quotes arguments containing whitespace so it stays copy-pasteable.Additionally, the apple-related
buildProjectfunction accounts for this change when printing the generatedxcodebuildcommand, enabling effective copy-and-paste.I've evaluated shell-quote (https://www.npmjs.com/package/shell-quote), which is also a transitive dependency of
@react-native-community/cli-tools.The reason why I considered
shell-quotenot fitting is that it structurally recognizes operators(;, |), command substitution ($(...)), and globs (*), returning them as{op}/{op:'glob'}objects, and this cannot be disabled by any option. Realisticxcodebuildvalues such asOTHER_LDFLAGS=$(inherited)are therefore mangled. By keeping the solution close to the current one, we require removal of verbatim quotes and field splitting, with the payload left untouched and close to the current splitting strategy. A minor bonus is that our utility throws a clearCLIErroron an unterminated quote, whereas shell-quote drops it silently.Another minor decision against
shell-quotewas about the recent CVE, although it impacted thequotefunction instead of theparsefunction we could have eventually used.We can still use
shell-quoteas an alternative strategy, since the dependency remains usable if the project later accepts post-processing its output and the operator caveat applies, but for this use case, a focused utility is simpler and more correct.Test Plan
Added tests in
packages/cli-tools/src/__tests__/tokenizeCommand.test.tsasserting some use cases.$ jest packages/cli-tools/src/__tests__/tokenize.test.ts PASS unit packages/cli-tools/src/__tests__/tokenize.test.ts tokenize ✓ splits space-separated xcodebuild arguments (1 ms) ✓ splits space-separated gradle properties ✓ keeps a double-quoted xcodebuild destination with spaces in one token ✓ keeps a single-quoted destination specifier with spaces in one token ✓ strips the surrounding quotes from a quoted path containing a space ✓ joins an unquoted build-setting name with its quoted, space-containing value ✓ collapses extra whitespace (spaces and tabs) between arguments ✓ passes xcodebuild build-setting syntax such as $(inherited) through verbatim ✓ preserves backslashes inside single quotes so Windows gradle paths survive ✓ throws on a lone unquoted apostrophe ✓ parses a quoted Gradle property value containing an apostrophe ✓ returns an empty array for empty input ✓ returns an empty array for whitespace-only input (1 ms) ✓ keeps an explicit empty quoted argument ✓ throws on an unterminated double quote ✓ throws on an unterminated single quote with POSIX backslash escaping (non-Windows) ✓ treats an unquoted backslash as an escape that preserves the next character ✓ escapes an apostrophe with a backslash outside quotes ✓ unescapes quotes inside double-quoted JSON values ✓ keeps backslashes literal inside double-quoted values on Windows ✓ keeps unquoted backslashes literal so native paths survive ✓ keeps backslashes literal inside double-quoted values Test Suites: 1 passed, 1 total Tests: 22 passed, 22 totalChecklist
react-nativecheckout (instructions).