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