Javier,

    Here are some comments on the multiprocessing implementation:

        - Why are we removing the lock? "del self._plugin_lock"

        - Why are we removing "def grep_wrapper(self, fuzzableRequest,
response):" ? This changes the behavior of all grep plugins !

        - What about moving this outside from the _grep_result()
function and only creating this object once in xUrllib?
        811                 fuzz_req = createFuzzableRequestRaw(
        812                                 req.get_method(), url,
        813                                 req.get_data(), req.headers
        814                                 )

        - "[kb.kb.append(*infotuple) for infotuple in plugin_resp]"
means that all plugins need to return the result they want to store in
the kb, correct?

        - If the above is correct, is the kb shared between all
sub-processes? Can one grep plugin access the results of a previous
run? Can one plugin read output from another plugin that's running in
a different sub-process?

        - Does this message still make sense? The timeout might apply
to something different that the run of only one plugin?
        827                     msg = ('The grep plugins took more than %s
seconds to run. '
        828                     'This is too much, please review its source
code.' % timeout)
        829                     om.out.error(msg)

        - "831                  #TODO: FIX THIS" why don't we leave our
error handler (the one that lets the user put a summary, steps to
reproduce, etc.) worry about this?

        - Code needs cleanup: lots of multiline and single line
comments , print, etc.

        - multiprocess2.py is never referenced/imported

        - Are we sure that CPU_COUNT is the most performing number for
worker threads?

        - unittest is missing

    Other than that, all looks good! I think that the change from
writing to the kb inside the plugin to writing outsite from it makes
sense even at other levels and not only for multiprocessing, so go
ahead and lets change all the other grep plugins in this way.

    Once all changes are applied, lets measure performance!

Regards,
-- 
Andrés Riancho
Director of Web Security at Rapid7 LLC
Founder at Bonsai Information Security
Project Leader at w3af

------------------------------------------------------------------------------
All the data continuously generated in your IT infrastructure contains a
definitive record of customers, application performance, security
threats, fraudulent activity and more. Splunk takes this data and makes
sense of it. Business sense. IT sense. Common sense.
http://p.sf.net/sfu/splunk-d2dcopy1
_______________________________________________
W3af-develop mailing list
W3af-develop@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/w3af-develop

Reply via email to