Re: [NSE] [patch] Big changes to http-enum.nse

[Ron <ron () skullsecurity net>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hey,

I've made the changes you requested, including Patrick's recommendation. But beyond them, I've done some more work on 
the HTTP library. 

I'd like to get your opinions of revision 20720 of my branch. I made some interface changes to the http library, mostly 
by making functions local that probably should have been local (and that aren't being used). I also removed a couple 
functions that I don't think are useful, and replaced the header documentation. 

While writing the header documentation, I realized that the pipeline() code could use some cleanup. There are redundant 
arguments, and arguments that I'd argue are missing. My next task is going to be to clean up that set of functions, 
document all the public functions better, then call it good to go. 

My goal is to go through the different nselibs and do something similar -- cleanup and document the interface as best 
as I can. Having good documentation makes a world of difference to new developers, and I like writing documentation. 

Ron

On Tue, 19 Oct 2010 22:30:05 -0700 David Fifield <david () bamsoftware com> wrote:
> There are some whitespace inconsistencies in the patch:
>
>      body = options.content
> -       elseif request_method_needs_content_length(method) then
> -               body = ""
> +  elseif request_method_needs_content_length(method) then
> +    body = ""
>    end
>
> Make sure that whitespace is consistent throughout any functions you
> edited.
>
> Let's use the term "method" instead of "verb," especially in http.lua.
>
> In response_contains, I think I see what you're doing:
>
> +  result, _, m[1], m[2], m[3], m[4], m[5], m[6], m[7], m[8], m[9] =
> string.find(case(response['status-line']), pattern)
>
> You can do it more robustly with this: (maybe someone else knows an
> even better way)
>
>   local function matches(...) return table.remove(arg, 1),
> table.remove(arg, 1), arg end result, _, m =
> matches(string.find(case(response['status-line']), pattern))
>
> The parse_url function has already been written as
> http://nmap.org/nsedoc/lib/url#parse; can you use that instead?
>
> Finally, I'm getting an error when a page can't be retrieved:
>
> nmap-http$ ./nmap --datadir . --script=http-enum www.bamsoftware.com
> -d -p80 NSE: HTTP: Page was '200 OK', it exists! (/images/)
> NSE: HTTP: HTTP request failed (is the host still up?)
> NSE: 'http-enum' (thread: 0x9bf68b8) against 69.164.193.231:80 threw
> an error! ./nselib/http.lua:1864: attempt to index local
> 'response' (a nil value) stack traceback:
>         ./nselib/http.lua:1864: in function 'response_contains'
>         ./scripts/http-enum.nse:385: in function
> <./scripts/http-enum.nse:305> (tail call): ?
>
> Once these things are addressed you're good to commit.
>
> David Fifield
> _______________________________________________
> Sent through the nmap-dev mailing list
> http://cgi.insecure.org/mailman/listinfo/nmap-dev
> Archived at http://seclists.org/nmap-dev/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iEYEARECAAYFAkzBF0cACgkQ2t2zxlt4g/TJpQCcDg9hnla2W4ajSczUADEIxdi9
XYcAoMPcMElgHTIDA3SzS1SA+eg6OijS
=LE0k
-----END PGP SIGNATURE-----
_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://seclists.org/nmap-dev/