Review.txt

Code review for pyFreenet
=========================

bab, starting at 56fb60ee5b1d820a14f717639b66422e2fac9578
---------------------------------------------------------

### State

- All files in fcp/ are reviewed, I’m pretty sure that they are not evil. I did not check for all kinds of code errors, though, only for potentially malicious code.

- Scripts I checked in toplevel: fcpgenkey, fcpget, fcpinvertkey, fcpnames, fcpput, fcpredirect, fcp_to_mrtg_bridge.py, fcp_to_rrdtool_bridge.py, fcpxmlrpc.cgi, fproxyproxy, freesitemgr, pyNodeConfig, freedisk


### fcp/node.py

General note: Really nicely commented code.

#### 2013-02-07 00:42:22

Read the code completely. Disabled an unsafe exec.

#### 2013-02-07 01:11:11

Reviewed everything in fcp/ except for sitemgr.py, xmlobject.py and xmlrpc.py

#### 2013-02-13 23:48:24

- xmlobject.py looks good.
- xmlrpc.py looks good - and actually provides an XML-RPC interface to freenet!

#### 2013-02-15 00:34:28

- fixed sitemgr.py and reactivated namesite after creating a pseudopythonparser which can read the pythonesque config files without using exec.

- Scripts I checked in toplevel: fcpgenkey, fcpget, fcpinvertkey, fcpnames, fcpput, fcpredirect, fcp_to_mrtg_bridge.py, fcp_to_rrdtool_bridge.py, fcpxmlrpc.cgi, fproxyproxy, freesitemgr, pyNodeConfig

#### Notes

- quite a few global objects, but that’s ok
- the methods mostly use **kw, but the help text gives an overview of the useful keywords.
- seems to attempt compatibility to before python 2.3 (=ancient). See line 281.
- it might be useful to have a property for the ID which just spawns new ids
- FCPNode.shutdown() is misleading: it closes the connection but does not shutdown the node.
- epydoc documentation generation works
- [sha](http://docs.python.org/2/library/sha.html) has been deprecated in favor of [hashlib](http://docs.python.org/2/library/hashlib.html) since Python 2.5.

Do the tutorial:

- creating the fcp.node.FCPNode() works
- node.put(key, file=path) works

TODO: 

- doctests functions: 
    * readdir √
    * hashfile √
    * sha265dda √
    * uriIsPrivate √
    * parseTime √
    * base64encode - can’t really test this right now.
    * base64decode - can’t really test this right now.

- doctests node:
    * getSocketTimeout(self): √
    * setSocketTimeout(self, socketTimeout): √
    * getVerbosity(self): √
    * setVerbosity(self, verbosity): √

- namesite: I don’t know what these should do, so I can’t test them
  reliably. Could be a simple decentral DNS system. The code looks as
  if it should work, though.
    * namesiteInit(self, path):
    * namesiteLoad(self):
    * namesiteSave(self):
    * namesiteDelLocal(self, name):
    * namesiteDelRecord(self, localname, domain):
    * namesiteAddPeer(self, name, uri):
    * namesiteHasPeer(self, name):
    * namesiteGetPeer(self, name):
    * namesiteRemovePeer(self, name):
    * namesiteProcessUri(self, uri):




#### Check requests we can send to the node

(according to https://wiki.freenetproject.org/FCPv2 )

TODO: Check which are supported.

- ClientHello 

- ListPeer (since 1045)
- ListPeers
- ListPeerNotes
- AddPeer
- ModifyPeer
- ModifyPeerNote
- RemovePeer 

- GetNode
- GetConfig (since 1027)
- ModifyConfig (since 1027) 

- TestDDARequest (since 1027)
- TestDDAResponse (since 1027) 

- GenerateSSK
- ClientPut
- ClientPutDiskDir
- ClientPutComplexDir
- ClientGet 

- LoadPlugin (since 1227)
- ReloadPlugin (since 1227)
- RemovePlugin (since 1227) 

- GetPluginInfo (since 1075)
- FCPPluginMessage (since 1075) 

- SubscribeUSK
- UnsubscribeUSK (since 1209) 

- WatchGlobal
- GetRequestStatus
- ListPersistentRequests
- RemoveRequest
- ModifyPersistentRequest 

- Disconnect
- Shutdown 

- Void 

- ProbeRequest