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

Add input validation to shodan convert command #205

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rmhowe425
Copy link
Contributor

  • Closes out Add input validation to shodan convert command #204
  • Created validation.py where I stored 2 functions used as callback methods for the input file type and output format for the shodan convert command.
  • Functions used for input validation are passed in via the callback parameter for click.argument.

@rmhowe425
Copy link
Contributor Author

rmhowe425 commented Dec 3, 2023

@achillean If you're okay with this PR and my method for adding input validation, I would he happy to open a new issue & PR to add similar input validation for all other CLI arguments.

This could help clean up and simplify the actual implementation for each command.

@achillean
Copy link
Owner

The validation of the filename looks good but I'm not sure what the validation of the output parameters adds. click already checks that the output format is one of the values specified in type:

$ shodan convert test.txt nonsense
Usage: shodan convert [OPTIONS] <input file> <output format>
Try 'shodan convert -h' for help.

Error: Invalid value for '<output format>': 'nonsense' is not one of 'kml', 'csv', 'geo.json', 'images', 'xlsx'.

Also I'm not a fan of having to maintain the list of supported output formats in different locations. It's fairly rare that they get changed but it would be nicer if everything (help text, error text, mapping of output format to class etc.) was automatically generated. However, the primary hesitation for me in adding the callback for the output format is that I'm not sure why it needs to exist.

@rmhowe425
Copy link
Contributor Author

Ah yep I see what you mean! I'll remove the callback input validation for the output format 😄

@rmhowe425
Copy link
Contributor Author

@achillean I updated the implementation to only include filename input validation.

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