Re: jsonrpc library and bitcoinrpc-info

[Patrik Karlsson <patrik () cqure net>]

On Jul 26, 2011, at 1:16 AM, Fyodor wrote:

> On Mon, Jul 25, 2011 at 11:03:23AM +0200, Patrik Karlsson wrote:
> > Should we commit this script or not? Any thoughts?
> > I think it could be useful and don't mind committing it.
>
> Thanks for stepping up to the plate!  The SoC mentors (David, Henri,
> Patrick, and me) have been pretty slammed with all of the SoC code, so
> the NSE review queue has grown longer than usual.  So if you can help
> shepherd this one into the tree, that will be great!  And of course
> thanks to Toni for writing it in the first place.  I just took a quick
> look (the library and script are very short) and have a few
> suggestions:
>
> o As Toni mentioned, the jsonrpc library (as simple as it is) still
>  should have NSEDoc for the public functions, etc.

I agree.

> o Regarding "@args creds.global", won't NSEDoc already catch and print
>  that since the script is requiring the creds library?  I don't know if
>  mentioning it explicitly will cause any problems.

This one is not as straight forward. While the creds library offers this functionality and processes the argument, few 
or no script make use of it.
So while including the library will populate a table with the credentials, it's up to the script to make use of them.
The script could be using the cred library for other functionality, such as the standardized output of discovered 
credentials (the brute scripts).
I don't know the best way to handle this, but wouldn't it make sense to document the argument in those scripts that use 
it instead of the library?

> o This is arguably too verbose for a default script.  Admittedly it
>  only produces output if a bitcoin server is found and if it has the
>  credentials.  But still, it would be nice to shorten this.  I don't
>  even know what many of the variables in @output mean, and many are
>  blank.  Perhaps the output can be shortened by only printing the
>  most important values and/or printing more than one per line where
>  that makes sense.  For the values which are worth printing, it would
>  be nice if the NSEDoc would describe them and/or provide links for
>  more information.

I think some of the parameters could be displayed only if run in verbose mode.
I know to little about the parameters to know which could be removed.

> If you guys can get this script whipped up into the shape that you are
> happy with, please do commit it.
>
> Cheers,
> Fyodor


Cheers,
Patrik

--
Patrik Karlsson
http://www.cqure.net
http://www.twitter.com/nevdull77

_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://seclists.org/nmap-dev/