search

certego / fw1-loggrabber

FW1-Loggrabber is a command-line tool to grab logfiles from remote Checkpoint devices using OPSEC LEA (Log Export API)
GNU General Public License v2.0
52 stars 35 forks source link

Provide ability to specify starting record number for ng offline mode. #46

Closed jvm3487 closed 6 years ago

jvm3487 commented 6 years ago

We have a requirement to be able to explicitly specify a starting record number from the command line when working with an ng firewall in offline mode similar to the below PR. I was wondering if a change similar to this would also be acceptable to the main fw1-loggrabber repository.

@adepasquale please let me know your thoughts if you get a chance, and thank you for your time.

jvm3487 commented 6 years ago

Hi @ManofWax and @adepasquale ,

Not to rush, but I wasn't sure if you saw this since it was opened it at the same time as my bugfix PR (https://github.com/certego/fw1-loggrabber/pull/47). I was hoping to get your thoughts on this, and thank you for your consideration.

sevdog commented 6 years ago

Hi @jvm3487, we think that your idea is fine.

However there should be a clear error/warning message when --loc is negative number, at least to inform users that if that parameter is not a positive integer it will be considered as 0.

Also I will prefer avoid using atoi with "untrusted" (aka: cli provided) input, cause its behaviour is undefined when input is not a string which represents an integer (see this SO thread, this article and this post on Mozilla blog ).

jvm3487 commented 6 years ago

@sevdog Thank you very much for your feedback! I have made changes based off your suggestions and added them to the PR. Please let me know if you have any other suggestions.

jvm3487 commented 6 years ago

Thanks @sevdog !