add completion for nmap

[kellerben]

Description

This is a completion for the well-known nmap tool.

TODOs:

• Changes to fish usage are reflected in user documentation/manpages.
• Tests have been added for regressions fixed
• User-visible changes noted in CHANGELOG.md

[kellerben]

can someone explain the failed test? Is there something I can do?

[exploide]

Great, nmap completions were also on my todo list 👍

can someone explain the failed test?

This project is experimenting with GitHub Actions for CI but it does not work well at the moment. Ignore it for now.

I have a few comments regarding your completion:

You use __fish_complete_user_at_hosts five times, but nmap does not take username@hostname arguments but just hostname. So you want to use __fish_print_hostnames instead.

Whenever you need to supply a file argument, you use __fish_complete_path. This is for certain circumstances where one wants to restrict the paths and might extend it with a description. This is not needed here, since an arbitrary file can be chosen for nmap's options. That means you can just use -F when you want to force file completion.

For the --script option, multiple comma-separated arguments can be passed. In #6833 it was discussed how that could work. Using __fish_complete_list could work: complete -c nmap -l script -r -a "(__fish_complete_list , __fish_complete_nmap_script)" -d 'script'. This would need to be adjusted to also work with the categories.

[kellerben]

@exploide: Thanks for the recommendations, I implemented everything you suggested.
I recognized, that I do not need the -F switch as the file completion is done by default anyway…

[kellerben]

I just added the completion for ncat which is part of nmap
I hope it is ok, that they are in the same pull request…

[exploide]

Looks good overall. When you only want to complete non-file arguments, use -x instead of -r. It seems to be mostly correct so you probably already know. But at least for exclude it is currently -r which seems odd. Haven't checked each and every option in detail.

I just added the completion for ncat

We already have netcat completions in nc.fish. This contains mostly the same completions you have in ncat.fish (feel free to submit improvements there).

If you prefer to use the ncat command and like to have completions for it, it is sufficient to have complete -c ncat -w nc in ncat.fish. Though it's up to the fish maintainers if they would like to include that. But it's not uncommon to have wrapping aliases.

[kellerben]

There are dozens of netcat-implementations and all of them are different. The typical ones are nc.openbsd or nc.traditional which are symlinked to nc. However, one often finds Busybox to be symlinked to nc or ncat symlinked to nc…
Having said that, I think its better to do it the other way round, writing one completion for every single tool and then having one nc.fish file, which tries to guess which nc-implementation is used currently (to choose the correct one). Currently, the completion of nc completes already to many options on my systems… What I wrote in the ncat.fish is a direct copy from the ncat-manpage which is from the nmap-project, therefore this should be correct for ncat (and not entirely if you use it for nc).

Of course, I see your point and one could try to extract all common nc-completions which is included in all others and they make their extensions. However, as there are so many nc-implementations, I think one will last with a really small common set…

[kellerben]

Looks good overall. When you only want to complete non-file arguments, use -x instead of -r. It seems to be mostly correct so you probably already know. But at least for exclude it is currently -r which seems odd. Haven't checked each and every option in detail.

This was a bug… I corrected it.
There are several options which only take numbers or hex or other formats. Should one specify a completion argument here? (like -d [0-9])

[exploide]

There are several options which only take numbers or hex or other formats. Should one specify a completion argument here?

If it is a small finite number of possible arguments, yes, you could do something like -x -a (seq 10). But concerning more complex forms, I'm not aware of a good way. If a very short hint of what is expected fits into the description string, that would be fine. For real argument hints, there is an open issue #2201.

There are dozens of netcat-implementations and all of them are different.

Yes, I share your observation. Though, the current nc.fish conforms to the ncat version of netcat. I updated it just a couple of days ago. Probably you have the old completion file installed since the new one is not part of a release, yet.

The idea was, that ncat is a modern and well-maintained version of netcat that is probably part of most distributions today (I might be wrong?!). Hence nc.fish covers ncat as of today. When someone complains it's not fitting their version of nc, we could incorporate the additional options and dispatch that based on the output of nc --version.

This is similar to what you propose with:

writing one completion for every single tool and then having one nc.fish file, which tries to guess which nc-implementation is used currently

Just that I would dispatch everything within nc.fish and you would include more distinct files and nc.fish just wraps the right one.

Both would work. Actually I start to like your idea. But one problem could be that sometimes there is just a nc and no nc.something. Or if it is just a symlink. Which command would nc.fish wrap in this case?

[kellerben]

Or if it is just a symlink. Which command would nc.fish wrap in this case?
basename (realpath (which nc)) should work…?

[exploide]

Nah, I was talking about the completion file associated to some command that is wrapped in this case.

Anyway, let's do it as you proposed. I like your idea more. When the cases I mentioned occur, we can still incorporate that version of nc in nc.fish.

I only have minor review comments on your ncat.fish. Compared with the current version you have that cool feature with openssl ciphers, that's good.

For the execution option -e, better allow file completion so one can complete something like /bin/bash.

When an argument is required, explicitly use -x or -r because otherwise the option will appear twice in the completion list, e.g. as --allow and --allow=. I noticed that for --allow and --deny.

[exploide]

Then, you can change the existing nc.fish to have this content:

# There a several different implementations of netcat.
# Let's use the version string to tell them apart
# and load to right set of completions.

set -l netcat (nc --version 2>&1 | string collect)

# Nmap Ncat
string match -q -e -- Ncat "$netcat" && complete -c nc -w ncat

Further implementations of netcat can easily be added in the future :) And I think we are done.

[kellerben]

I think using nc --version is no good solution… neither nc.traditional nor nc.openbsd nor busybox have a --version switch… therefore I think my proposed solution is better, you may want to look at it…

[exploide]

neither nc.traditional nor nc.openbsd nor busybox have a --version switch

I did not imagine it's that bad...

therefore I think my proposed solution is better, you may want to look at it

Looks great. I think that should work most of the time :) Thanks!

[kellerben]

I have another improvements. The first is just the replacement of sed with the internal string replace. Please review the third commit. There I cache the nmap-script results in a global variable to have a faster completion (after the first tab…)

[exploide]

I think caching these is a good idea. But I'm not sure whether it is expected behaviour for a completion script to set global variables. I would leave that question open for a fish maintainer.

[zanchey]

The problem with caching is then invalidation - if you have a long-running session that nmap gets upgraded underneath, you end up with incomplete lists in some sessions and that can cause confusion. We do it elsewhere it is kicked off in the background so as not to block the completions, and then regenerated every 5 minutes - see share/functions/__fish_print_packages.fish. In this case I wouldn't bother.

[kellerben]

The problem with caching is then invalidation - if you have a long-running session that nmap gets upgraded underneath, you end up with incomplete lists in some sessions and that can cause confusion.

ok, I added a timestamp, it gets renewed every 5 minutes

We do it elsewhere it is kicked off in the background so as not to block the completions

I think we do not need that much overhead here. The completion needs only 500ms on my computer, so the caching is somewhat a luxury problem ;)

[krobelus]

Merged as 0a40a6d and de9f4cb, thanks!
I changed the logic a bit because my system doesn't have the nc.openbsd command name, even though nc is the OpenBSD version; completions are only autoloaded for valid executables. It should still work for you.

[kellerben]

I think the file nc.traditional should be named nc.traditional.fish

[zanchey]

Quite right, fixed in 021679b