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

cmd/dev-cmd: improve handling of invalid input #6835

Merged
merged 6 commits into from Dec 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 29 additions & 13 deletions Library/Homebrew/cli/parser.rb
Expand Up @@ -34,6 +34,7 @@ def initialize(args = ARGV, &block)
@conflicts = []
@switch_sources = {}
@processed_options = []
@max_named_args = nil
@hide_from_man_page = false
instance_eval(&block)
post_initialize
Expand Down Expand Up @@ -139,6 +140,7 @@ def parse(cmdline_args = ARGV)
raise e
end
check_constraint_violations
check_named_args(remaining_args)
@args[:remaining] = remaining_args
@args.freeze_processed_options!(@processed_options)
Homebrew.args = @args
Expand Down Expand Up @@ -178,6 +180,10 @@ def formula_options
[]
end

def max_named(count)
@max_named_args = count
end

def hide_from_man_page!
@hide_from_man_page = true
end
Expand Down Expand Up @@ -269,6 +275,10 @@ def check_constraint_violations
check_constraints
end

def check_named_args(args)
raise NamedArgumentsError, @max_named_args if !@max_named_args.nil? && args.size > @max_named_args
end

def process_option(*args)
option, = @parser.make_switch(args)
@processed_options << [option.short.first, option.long.first, option.arg, option.desc.first]
Expand All @@ -277,14 +287,10 @@ def process_option(*args)

class OptionConstraintError < RuntimeError
def initialize(arg1, arg2, missing: false)
if !missing
message = <<~EOS
`#{arg1}` and `#{arg2}` should be passed together.
EOS
message = if !missing
"`#{arg1}` and `#{arg2}` should be passed together."
else
message = <<~EOS
`#{arg2}` cannot be passed without `#{arg1}`.
EOS
"`#{arg2}` cannot be passed without `#{arg1}`."
end
super message
end
Expand All @@ -294,17 +300,27 @@ class OptionConflictError < RuntimeError
def initialize(args)
args_list = args.map(&Formatter.public_method(:option))
.join(" and ")
super <<~EOS
Options #{args_list} are mutually exclusive.
EOS
super "Options #{args_list} are mutually exclusive."
end
end

class InvalidConstraintError < RuntimeError
def initialize(arg1, arg2)
super <<~EOS
`#{arg1}` and `#{arg2}` cannot be mutually exclusive and mutually dependent simultaneously.
EOS
super "`#{arg1}` and `#{arg2}` cannot be mutually exclusive and mutually dependent simultaneously."
end
end

class NamedArgumentsError < UsageError
def initialize(maximum)
message = case maximum
when 0
"This command does not take named arguments."
when 1
"This command does not take multiple named arguments."
else
"This command does not take more than #{maximum} named arguments."
end
super message
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cmd/--env.rb
Expand Up @@ -11,7 +11,7 @@ module Homebrew
def __env_args
Homebrew::CLI::Parser.new do
usage_banner <<~EOS
`--env` [<options>]
`--env` [<options>] [<formula>]

Summarise Homebrew's build environment as a plain list.

Expand Down
3 changes: 1 addition & 2 deletions Library/Homebrew/cmd/--version.rb
Expand Up @@ -13,14 +13,13 @@ def __version_args
Print the version numbers of Homebrew, Homebrew/homebrew-core and Homebrew/homebrew-cask
(if tapped) to standard output.
EOS
max_named 0
MikeMcQuaid marked this conversation as resolved.
Show resolved Hide resolved
end
end

def __version
__version_args.parse

odie "This command does not take arguments." if ARGV.any?

puts "Homebrew #{HOMEBREW_VERSION}"
puts "#{CoreTap.instance.full_name} #{CoreTap.instance.version_string}"
puts "#{Tap.default_cask_tap.full_name} #{Tap.default_cask_tap.version_string}" if Tap.default_cask_tap.installed?
Expand Down
5 changes: 2 additions & 3 deletions Library/Homebrew/cmd/analytics.rb
Expand Up @@ -19,14 +19,13 @@ def analytics_args
EOS
switch :verbose
switch :debug
max_named 1
end
end

def analytics
analytics_args.parse

raise UsageError if args.remaining.size > 1

case args.remaining.first
when nil, "state"
if Utils::Analytics.disabled?
Expand All @@ -42,7 +41,7 @@ def analytics
when "regenerate-uuid"
Utils::Analytics.regenerate_uuid!
else
raise UsageError
raise UsageError, "Unknown subcommand."
end
end
end
6 changes: 3 additions & 3 deletions Library/Homebrew/cmd/cat.rb
Expand Up @@ -12,17 +12,17 @@ def cat_args

Display the source of <formula>.
EOS
max_named 1
end
end

def cat
cat_args.parse
# do not "fix" this to support multiple arguments, the output would be
# unparsable, if the user wants to cat multiple formula they can call
# brew cat multiple times.
# unparsable; if the user wants to cat multiple formula they can call
# `brew cat` multiple times.
formulae = Homebrew.args.formulae
raise FormulaUnspecifiedError if formulae.empty?
raise "`brew cat` doesn't support multiple arguments" if args.remaining.size > 1

cd HOMEBREW_REPOSITORY
pager = if ENV["HOMEBREW_BAT"].nil?
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cmd/cleanup.rb
Expand Up @@ -21,7 +21,7 @@ def cleanup_args
description: "Show what would be removed, but do not actually remove anything."
switch "-s",
description: "Scrub the cache, including downloads for even the latest versions. "\
"Note downloads for any installed formula or cask will still not be deleted. "\
"Note downloads for any installed formulae or casks will still not be deleted. "\
"If you want to delete those too: `rm -rf \"$(brew --cache)\"`"
switch "--prune-prefix",
description: "Only prune the symlinks and directories from the prefix and remove no other files."
Expand Down
19 changes: 10 additions & 9 deletions Library/Homebrew/cmd/command.rb
Expand Up @@ -20,17 +20,18 @@ def command_args

def command
command_args.parse
abort "This command requires a command argument" if args.remaining.empty?

cmd = HOMEBREW_INTERNAL_COMMAND_ALIASES.fetch(args.remaining.first, args.remaining.first)
raise UsageError, "This command requires a command argument" if args.remaining.empty?

path = Commands.path(cmd)
args.remaining.each do |c|
cmd = HOMEBREW_INTERNAL_COMMAND_ALIASES.fetch(c, c)
path = Commands.path(cmd)
cmd_paths = PATH.new(ENV["PATH"]).append(Tap.cmd_directories) unless path
path ||= which("brew-#{cmd}", cmd_paths)
path ||= which("brew-#{cmd}.rb", cmd_paths)

cmd_paths = PATH.new(ENV["PATH"]).append(Tap.cmd_directories) unless path
path ||= which("brew-#{cmd}", cmd_paths)
path ||= which("brew-#{cmd}.rb", cmd_paths)

odie "Unknown command: #{cmd}" unless path
puts path
odie "Unknown command: #{cmd}" unless path
puts path
end
end
end
1 change: 1 addition & 0 deletions Library/Homebrew/cmd/commands.rb
Expand Up @@ -19,6 +19,7 @@ def commands_args
description: "Include aliases of internal commands."
switch :verbose
switch :debug
max_named 0
end
end

Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cmd/config.rb
Expand Up @@ -16,12 +16,12 @@ def config_args
EOS
switch :verbose
switch :debug
max_named 0
end
end

def config
config_args.parse
raise UsageError unless args.remaining.empty?

SystemConfig.dump_verbose_config
end
Expand Down
6 changes: 1 addition & 5 deletions Library/Homebrew/cmd/desc.rb
Expand Up @@ -40,11 +40,7 @@ def desc
search_type << :either if args.search
search_type << :name if args.name
search_type << :desc if args.description
if search_type.size > 1
odie "Pick one, and only one, of -s/--search, -n/--name, or -d/--description."
elsif search_type.present? && ARGV.named.empty?
odie "You must provide a search term."
end
odie "You must provide a search term." if search_type.present? && ARGV.named.empty?
MikeMcQuaid marked this conversation as resolved.
Show resolved Hide resolved

results = if search_type.empty?
raise FormulaUnspecifiedError if ARGV.named.empty?
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cmd/diy.rb
Expand Up @@ -21,6 +21,7 @@ def diy_args
description: "Explicitly set the <version> of the package being installed."
switch :verbose
switch :debug
max_named 0
end
end

Expand All @@ -47,7 +48,6 @@ def diy

def detect_version(path)
version = path.version.to_s

raise "Couldn't determine version, set it with --version=<version>" if version.empty?

version
Expand Down
3 changes: 2 additions & 1 deletion Library/Homebrew/cmd/doctor.rb
Expand Up @@ -18,7 +18,8 @@ def doctor_args
an issue; just ignore this.
EOS
switch "--list-checks",
description: "List all audit methods."
description: "List all audit methods, which can be run individually "\
"if provided as arguments."
switch "-D", "--audit-debug",
description: "Enable debugging and profiling of audit methods."
switch :verbose
Expand Down
1 change: 1 addition & 0 deletions Library/Homebrew/cmd/gist-logs.rb
Expand Up @@ -28,6 +28,7 @@ def gist_logs_args
"be accessible with its link."
switch :verbose
switch :debug
max_named 1
end
end

Expand Down
12 changes: 8 additions & 4 deletions Library/Homebrew/cmd/info.rb
Expand Up @@ -61,25 +61,29 @@ def info_args

def info
info_args.parse

if args.days.present?
raise UsageError, "days must be one of #{VALID_DAYS.join(", ")}" unless VALID_DAYS.include?(args.days)
raise UsageError, "--days must be one of #{VALID_DAYS.join(", ")}" unless VALID_DAYS.include?(args.days)
end

if args.category.present?
if ARGV.named.present? && !VALID_FORMULA_CATEGORIES.include?(args.category)
raise UsageError, "category must be one of #{VALID_FORMULA_CATEGORIES.join(", ")} when querying formulae"
raise UsageError, "--category must be one of #{VALID_FORMULA_CATEGORIES.join(", ")} when querying formulae"
end

unless VALID_CATEGORIES.include?(args.category)
raise UsageError, "category must be one of #{VALID_CATEGORIES.join(", ")}"
raise UsageError, "--category must be one of #{VALID_CATEGORIES.join(", ")}"
end
end

if args.json
raise UsageError, "invalid JSON version: #{args.json}" unless ["v1", true].include? args.json
raise UsageError, "Invalid JSON version: #{args.json}" unless ["v1", true].include? args.json
raise UsageError, "This command's option requires a formula argument" if ARGV.named.empty?
Copy link
Member

Choose a reason for hiding this comment

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

Could/should this be a FormulaUnspecifiedError?

Copy link
Member Author

Choose a reason for hiding this comment

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

I made these a UsageError with a custom message stating that a formula argument is required only for these specific flags, rather than the command in general, as is implied in the wording of FormulaUnspecifiedError.

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha 👍


print_json
elsif args.github?
raise UsageError, "This command's option requires a formula argument" if ARGV.named.empty?

exec_browser(*Homebrew.args.formulae.map { |f| github_info(f) })
else
print_info
Expand Down
1 change: 1 addition & 0 deletions Library/Homebrew/cmd/install.rb
Expand Up @@ -101,6 +101,7 @@ def install
end

install_args.parse

raise FormulaUnspecifiedError if args.remaining.empty?

if args.ignore_dependencies?
Expand Down
1 change: 1 addition & 0 deletions Library/Homebrew/cmd/leaves.rb
Expand Up @@ -15,6 +15,7 @@ def leaves_args
List installed formulae that are not dependencies of another installed formula.
EOS
switch :debug
max_named 0
end
end

Expand Down
5 changes: 4 additions & 1 deletion Library/Homebrew/cmd/link.rb
Expand Up @@ -48,7 +48,10 @@ def link
else
keg.name
end
puts "To relink: brew unlink #{keg.name} && brew link #{name_and_flag}"
puts <<~EOS
To relink:
brew unlink #{keg.name} && brew link #{name_and_flag}
EOS
next
end

Expand Down
1 change: 1 addition & 0 deletions Library/Homebrew/cmd/log.rb
Expand Up @@ -22,6 +22,7 @@ def log_args
description: "Print only one line per commit."
flag "-1", "--max-count",
description: "Print only one or a specified number of commits."
max_named 1
end
end

Expand Down
1 change: 1 addition & 0 deletions Library/Homebrew/cmd/missing.rb
Expand Up @@ -27,6 +27,7 @@ def missing_args

def missing
missing_args.parse

return unless HOMEBREW_CELLAR.exist?

ff = if ARGV.named.empty?
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/cmd/outdated.rb
Expand Up @@ -10,7 +10,7 @@ module Homebrew
def outdated_args
Homebrew::CLI::Parser.new do
usage_banner <<~EOS
`outdated` [<options>]
`outdated` [<options>] [<formula>]

List installed formulae that have an updated version available. By default, version
information is displayed in interactive shells, and suppressed otherwise.
Expand Down Expand Up @@ -41,7 +41,7 @@ def outdated
ARGV.resolved_formulae
end
if args.json
raise UsageError, "invalid JSON version: #{args.json}" unless ["v1", true].include? args.json
raise UsageError, "Invalid JSON version: #{args.json}" unless ["v1", true].include? args.json

outdated = print_outdated_json(formulae)
else
Expand Down
2 changes: 2 additions & 0 deletions Library/Homebrew/cmd/postinstall.rb
Expand Up @@ -23,6 +23,8 @@ def postinstall_args
def postinstall
postinstall_args.parse

raise KegUnspecifiedError if args.remaining.empty?

ARGV.resolved_formulae.each do |f|
ohai "Postinstalling #{f}"
fi = FormulaInstaller.new(f)
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cmd/readall.rb
Expand Up @@ -14,7 +14,7 @@ def readall_args
Import all formulae from the specified <tap>, or from all installed taps if none is provided.
This can be useful for debugging issues across all formulae when making
significant changes to `formula.rb`, testing the performance of loading
all formulae or to determine if any current formulae have Ruby issues.
all formulae or checking if any current formulae have Ruby issues.
EOS
switch "--aliases",
description: "Verify any alias symlinks in each tap."
Expand Down
2 changes: 2 additions & 0 deletions Library/Homebrew/cmd/reinstall.rb
Expand Up @@ -47,6 +47,8 @@ def reinstall_args
def reinstall
reinstall_args.parse

raise FormulaUnspecifiedError if args.remaining.empty?

FormulaInstaller.prevent_build_flags unless DevelopmentTools.installed?

Install.perform_preinstall_checks
Expand Down