Re: [RFC][patch] Make timeout script-args consistent

[Daniel Miller <bonsaiviking () gmail com>]

Tom,

Thanks for catching this, should be fixed in r30656. I forgot about
--script-updatedb as a partial test for runtime errors before
committing, though I ran everything through luac -p to check for
syntax errors. The problem was that stdnse.parse_timespec wasn't meant
to handle nil input. I put an explicit nil check in it, and it should
work as expected now. Since this only affects cases that would throw
an error anyway, it shouldn't affect any other scripts.

The millisecond timeout changes are still waiting for Thursday, in
case anyone objects.

Dan

On Tue, Mar 5, 2013 at 9:47 PM, Tom Sellers <nmap () fadedcode net> wrote:
> On 3/5/2013 12:40 PM, David Fifield wrote:
> > On Tue, Mar 05, 2013 at 11:30:41AM -0600, Daniel Miller wrote:
> > > I noticed that several timeout-related script-args were documented
> > > as accepting a timespec as defined in the man page
> > > (http://nmap.org/book/man-performance.html), but in reality only
> > > accepted a plain number. Also, for some scripts this number was
> > > seconds, and for others, milliseconds. The attached patch attempts
> > > to standardize on the timespec method, as well as fix some
> > > documentation issues (missing nsedoc, incorrect argument names,
> > > etc.)
> > >
> > > The major objection I'd expect here is for scripts that calculate
> > > timeouts in seconds (os.time() - start < timeout) which may
> > > misbehave if a timeout less than 1 second is specified (e.g.
> > > --script-args myscript.timeout=100ms), but since the script did not
> > > alter any defaults, this should not affect most users.
> > >
> > > Are there any other objections to applying this patch? If there is
> > > no response by Thursday morning, or if there is significant positive
> > > response, I will commit it.
> >
> > I'm on board with this patch. You need to do the parts that change
> > behavior (milliseconds to seconds, like in broadcast-ping) as a separate
> > commit. All the parse_timespec changes should be refactoring that
> > doesn't change behavior for people already calling the scripts with
> > integer arguments. Anything that changes behavior, please put in a
> > separate commit.
>
> FYI, post commit I see the following when doing a fresh checkout and then
>
> sudo nmap --script-updatedb
>
> Starting Nmap 6.26SVN ( http://nmap.org ) at 2013-03-05 21:43 CST
> NSE: Updating rule database.
> NSE: Failed to load /usr/local/bin/../share/nmap/scripts//broadcast-bjnp-discover.nse:
> /usr/local/bin/../share/nmap/nselib/stdnse.lua:347: bad argument #1 to 'match' (string expected, got nil)
> stack traceback:
>         [C]: in function 'match'
>         /usr/local/bin/../share/nmap/nselib/stdnse.lua:347: in function 'parse_timespec'
>         ...l/bin/../share/nmap/scripts//broadcast-bjnp-discover.nse:44: in function 
> <...l/bin/../share/nmap/scripts//broadcast-bjnp-discover.nse:1>
> NSE: failed to initialize the script engine:
> /usr/local/bin/../share/nmap/nse_main.lua:547: could not load script
> stack traceback:
>         [C]: in function 'error'
>         /usr/local/bin/../share/nmap/nse_main.lua:547: in function 'new'
>         /usr/local/bin/../share/nmap/nse_main.lua:1249: in main chunk
>         [C]: in ?
>
> QUITTING!
>
> I see it running scans too, but the script DB update was the simplest test.
>
> Thanks much,
> Tom
_______________________________________________
Sent through the dev mailing list
http://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/