[Twisted-Python] feedback on next version of Fireshark using twisted framework....

[Stephan]

thanks for the feed back Jasper! More feedback welcome.
Yes the code def. needs to be cleaned up before public release, and I will
make more use of python libs for path concatenation.

The feedback I'm specifically looking for is anything related to how I'm
using twisted and process management.
The reason I have everything in there from timing out, to killing a
process, is because I'm dealing with malicious websites, so bad things
happen and so I need to take extreme measures to for resource monitoring.

Any suggestions on how I can do things better with the service in terms of
how i'm using Twisted is welcome! Jasper provided great comments and I'll
be working to make those changes.

Thanks,
Stephan

On Thu, May 3, 2012 at 11:12 AM, Jasper St. Pierre <jstpierre at mecheye.net>wrote:

> You seem to have some indentation issues. You have a random assortment
> of Python style issues (some lines end with a semicolon). You use
> 'foo' + '/' + 'bar' to generate paths. While not evil (I think all
> major platforms will support that and do the translation on their
> own), you're better off using os.path.join. The "~/.mozilla/firefox"
> won't work on certain platforms (and you could just use
> os.path.expanduser if you don't want to support those platforms).
>
> Your process spawning code is ugly, and could be open to injection if
> not handled properly. Do it the better way:
>
>    reactor.spawnProcess(self.processProtocol, 'xfvb-run',
> ['xvfv-run', '--auto-servernum', 'firefox', '-p', profile_id], env =
> os.environ, usePTY=1)
>
> Don't do 'foo == True'. Please use new-style classes (inherit from
> 'object'). I don't know why the timeout stuff makes me a bit scared,
> but it does. I don't really think timeouts are necessary here.
>
> I don't think you should be killing a process with os.kill. I'm quite
> sure whatever you're trying to do has a better Twisted idiom.
>
> Google for PEP 8, ignore the ones that don't make sense (like foo_case
> instead of camelCase in Twisted code), and try to follow the rest.
> When using Twisted code, always look for things in Twisted that might
> combat the standard library, and try to use Twisted's whenever
> possible.
>
> On Thu, May 3, 2012 at 11:56 AM, Stephan <schenette at gmail.com> wrote:
> >
> > Hi I have an open source project that is pretty well-known in the
> security
> > community. http://www.fireshark.org
> > it's a service wrapper that runs a headless version of firefox to visit
> > malicious sites to store telemetry data.
> >
> > The old service used PERL for threading (very painful), so a while ago I
> > changed everything to python using
> > the twisted framework. the service's job is to launch a choose a firefox
> > profile, launch the process and
> > handle closes and crashes approprietly.
> >
> > the launch of version 2.5 is next week but I sure could use some
> feedback on
> > the service code before releasing it.
> > Keep in mind I haven't finishing commenting for public release.
> >
> > here are the files for the new service:
> >
> > 1)
> > fireshark.py http://pastebin.mozilla.org/1614427
> >
> > this is the main service. opens up a port to listen on, and uses the
> > following two classes to manage profiles and launch
> > a firefox process.
> >
> > 2)
> > firesharkprofilemanager.py http://pastebin.mozilla.org/1614428
> >
> > this is the firefox profile manager, it manages what firefox profiles are
> > available.
> >
> > 3)
> > firefoxprocess.py http://pastebin.mozilla.org/1614431
> >
> > this is the firefox process class, to check if  the launched process has
> > been killed as well, as check if the logging from
> > the internal plugin (another part of the fireshark project) has completed
> > logging.
> >
> > Thanks in advance! Twisted has been a great Framework!
> > Stephan
> >
> > _______________________________________________
> > Twisted-Python mailing list
> > Twisted-Python at twistedmatrix.com
> > http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
> >
>
>
>
> --
>   Jasper
>
> _______________________________________________
> Twisted-Python mailing list
> Twisted-Python at twistedmatrix.com
> http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20120503/f473e823/attachment.html>