achillean / shodan-python

The official Python library for Shodan
https://developer.shodan.io
Other
2.47k stars 552 forks source link

Add input validation to shodan convert command #205

Open rmhowe425 opened 9 months ago

rmhowe425 commented 9 months ago
rmhowe425 commented 9 months ago

@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 commented 9 months ago

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 commented 9 months ago

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

rmhowe425 commented 8 months ago

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