Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Commander unsafely allows commands with duplicate names to be registered #1903

Closed
huntie opened this issue Jul 6, 2023 · 4 comments
Closed
Labels
semver: major Releasing requires a major version bump, not backwards compatible

Comments

@huntie
Copy link

huntie commented Jul 6, 2023

We ran into an issue recently in a project that depends on commander, which dynamically defines commands based on plugins. Commander's internal array data structure for commands, and the Array.find based lookup on name, meant that repeat calls to command() produced unexpected behaviour (the most recent call does not redeclare the command).

At minimum, I believe this should error.

Example

const { Command } = require('commander');
const program = new Command();
program.command('split').action((str, options) => console.log('1st'));
program.command('split').action((str, options) => console.log('2nd'));
program.parse();

Output will always be the first.

$ node example
Usage: example [options] [command]
Options:
  -h, --help      display help for command
Commands:
  split
  split
  help [command]  display help for command
$ node example split
1st

Expectation

Shouldn't allow duplicates and should throw an exception if a name already exists.

@shadowspawn
Copy link
Collaborator

shadowspawn commented Jul 6, 2023

Commander does not have tests for all possible mistakes/accidental-misuse by the "author", but willing to consider more as they come up. Mixing in plugins does make it possible for the issue to come up for the end-user rather than for the author as such.

The duplicate primary name does have a dubious outcome. I am less worried about aliases clashing, but could perhaps actually handle this and ignore clashing alias when command is added.

@aweebit
Copy link
Contributor

aweebit commented Aug 1, 2023

Opened #1924 with a fix.

@shadowspawn
Copy link
Collaborator

shadowspawn commented Nov 4, 2023

Reworked PR in #2059.

@shadowspawn shadowspawn added the pending release Merged into a branch for a future release, but not released yet label Nov 4, 2023
@shadowspawn shadowspawn removed the pending release Merged into a branch for a future release, but not released yet label Feb 3, 2024
@shadowspawn
Copy link
Collaborator

Released in Commander v12.0.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: major Releasing requires a major version bump, not backwards compatible
Projects
None yet
Development

No branches or pull requests

3 participants