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

Allow empty argument for --dane #526

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

oxzi
Copy link
Contributor

@oxzi oxzi commented Nov 14, 2024

The "--dane" option can be used both as a flag and with an argument. In its current implementation, it is even a special case for flags with variable numbers of arguments.

At an Icinga 2 ITL PR by GitHub user @peteeckel, an unexpected behavior was seen when calling check_ssl_cert with "--dane" followed by an empty argument1, as so:

$ ./check_ssl_cert --dane ""

If the empty argument was used, the --dane option was effectively useless. This is due to the argument counting/checking code, not expecting an empty second argument, setting DANE="", which disables it.

This change allows an empty second argument, which will then be swallowed. For the other options with variable numbers of arguments, this does not seem to apply.

Footnotes

  1. https://github.com/Icinga/icinga2/pull/10196#discussion_r1840123628

The "--dane" option can be used both as a flag and with an argument. In
its current implementation, it is even a special case for flags with
variable numbers of arguments.

At an Icinga 2 ITL PR by GitHub user @peteeckel, an unexpected behavior
was seen when calling check_ssl_cert with "--dane" followed by an empty
argument[0], as so:

$ ./check_ssl_cert --dane ""

If the empty argument was used, the --dane option was effectively
useless. This is due to the argument counting/checking code, not
expecting an empty second argument, setting DANE="", which disables it.

This change allows an empty second argument, which will then be
swallowed. For the other options with variable numbers of arguments,
this does not seem to apply.

[0]: Icinga/icinga2#10196 (comment)
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.

2 participants