Skip to content

Fix crash in Option.attributeName() on doubled/trailing hyphen in option flags - #2643

Closed
heykav wants to merge 1 commit into
tj:masterfrom
heykav:fix/option-attributename-empty-segment
Closed

heykav wants to merge 1 commit into
tj:masterfrom
heykav:fix/option-attributename-empty-segment

Conversation

@heykav

@heykav heykav commented Oct 3, 2026

Copy link
Copy Markdown

Summary

  • .option('--foo--bar <val>') (an easy typo of --foo-bar, e.g. --dry--run) currently crashes with an uncaught TypeError: Cannot read properties of undefined (reading 'toUpperCase') instead of being handled by commander.
  • Root cause: camelcase() in lib/option.js indexes word[0] after str.split('-'), which is undefined for the empty segment produced by a doubled or trailing hyphen. splitOptionFlags() already rejects malformed dash prefixes (e.g. ---triple) but not an embedded double hyphen, so this input reaches camelcase() unvalidated.
  • The crash is a raw TypeError, not a CommanderError, so it also bypasses exitOverride().
  • Fix: use word.charAt(0) instead of word[0], which returns '' for an empty segment, so the segment is skipped instead of crashing (--foo--bar gives attribute name fooBar).

Test plan

  • Added tests/option.attributeName.test.js covering doubled hyphen, trailing hyphen, and negated-option doubled-hyphen cases. All 3 fail without the fix and pass with it.
  • node --test: 1375 passing, 0 failing, 1 skipped (previously 1372 passing).
  • prettier --check clean on the changed files.

🤖 Generated with Claude Code

camelcase() in lib/option.js assumed every segment produced by
str.split('-') is non-empty, indexing word[0] directly. An option flags
string with a doubled hyphen (an easy typo, e.g. '--dry--run' instead of
'--dry-run') or a trailing hyphen produces an empty segment, so word[0]
is undefined and .toUpperCase() throws a raw TypeError.

This crash happens synchronously inside .option()/.addOption(), so it is
not a CommanderError and is not caught by exitOverride().

Use word.charAt(0), which returns '' for an empty segment, so the empty
segment is skipped ('--foo--bar' maps to attribute name 'fooBar').

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@shadowspawn

Copy link
Copy Markdown
Collaborator

This looks like a drive-by AI contribution.

This may get looked at and used when the issue is prioritised. It has already been reported in automated looking PRs twice: #2532 #2535

The human maintainer does not have time to review all AI heavy PRs that are opened.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants