linting refactoring communicator(utils) and reduced TOCTOU issus with online peer picking

src/communicator/onlinepeers/clearofflinepeer.py

@@ -1,9 +1,13 @@
-'''
+"""Onionr - Private P2P Communication.
-    Onionr - Private P2P Communication
 
-    clear offline peer in a communicator instance
+clear offline peer in a communicator instance
-'''
+"""
-'''
+from typing import TYPE_CHECKING
+
+import logger
+if TYPE_CHECKING:
+    from communicator import OnionrCommunicatorDaemon
+"""
     This program is free software: you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
     the Free Software Foundation, either version 3 of the License, or
@@ -16,14 +20,16 @@
 
     You should have received a copy of the GNU General Public License
     along with this program.  If not, see <https://www.gnu.org/licenses/>.
-'''
+"""
-import logger
+
-def clear_offline_peer(comm_inst):
+
-    '''Removes the longest offline peer to retry later'''
+def clear_offline_peer(comm_inst: 'OnionrCommunicatorDaemon'):
+    """Remove the longest offline peer to retry later."""
     try:
         removed = comm_inst.offlinePeers.pop(0)
     except IndexError:
         pass
     else:
-        logger.debug('Removed ' + removed + ' from offline list, will try them again.')
+        logger.debug('Removed ' + removed +
-    comm_inst.decrementThreadCount('clear_offline_peer')
+                     ' from offline list, will try them again.')
+    comm_inst.decrementThreadCount('clear_offline_peer')

src/communicator/onlinepeers/onlinepeers.py

@@ -1,11 +1,14 @@
-"""
+"""Onionr - Private P2P Communication.
-    Onionr - Private P2P Communication
 
-    get online peers in a communicator instance
+get online peers in a communicator instance
 """
 import time
+from typing import TYPE_CHECKING
+
 from etc import humanreadabletime
 import logger
+if TYPE_CHECKING:
+    from communicator import OnionrCommunicatorDaemon
 """
     This program is free software: you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
@@ -22,25 +25,25 @@ import logger
 """
 
 
-def get_online_peers(comm_inst):
+def get_online_peers(comm_inst: 'OnionrCommunicatorDaemon'):
-    """
+    """Manage the comm_inst.onlinePeers attribute list.
-        Manages the comm_inst.onlinePeers attribute list,
+
-        ]connects to more peers if we have none connected
+    Connect to more peers if we have none connected
     """
     config = comm_inst.config
     if config.get('general.offline_mode', False):
         comm_inst.decrementThreadCount('get_online_peers')
         return
     logger.debug('Refreshing peer pool...')
-    maxPeers = int(config.get('peers.max_connect', 10))
+    max_peers = int(config.get('peers.max_connect', 10))
-    needed = maxPeers - len(comm_inst.onlinePeers)
+    needed = max_peers - len(comm_inst.onlinePeers)
 
     last_seen = 'never'
     if not isinstance(comm_inst.lastNodeSeen, type(None)):
         last_seen = humanreadabletime.human_readable_time(
             comm_inst.lastNodeSeen)
 
-    for i in range(needed):
+    for _ in range(needed):
         if len(comm_inst.onlinePeers) == 0:
             comm_inst.connectNewPeer(useBootstrap=True)
         else:
@@ -50,8 +53,7 @@ def get_online_peers(comm_inst):
             break
     else:
         if len(comm_inst.onlinePeers) == 0:
-            logger.debug
+            logger.debug('Couldn\'t connect to any peers.' +
-            ('Couldn\'t connect to any peers.' +
              f' Last node seen {last_seen}  ago.')
         else:
             comm_inst.lastNodeSeen = time.time()

src/communicator/onlinepeers/pickonlinepeers.py

@@ -1,9 +1,12 @@
-'''
+"""
-    Onionr - Private P2P Communication
+Onionr - Private P2P Communication.
 
-    pick online peers in a communicator instance
+pick online peers in a communicator instance
-'''
+"""
-'''
+import secrets
+
+import onionrexceptions
+"""
     This program is free software: you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by
     the Free Software Foundation, either version 3 of the License, or
@@ -16,17 +19,22 @@
 
     You should have received a copy of the GNU General Public License
     along with this program.  If not, see <https://www.gnu.org/licenses/>.
-'''
+"""
-import secrets
+
+
 def pick_online_peer(comm_inst):
-    '''randomly picks peer from pool without bias (using secrets module)'''
+    """Randomly picks peer from pool without bias (using secrets module)."""
     ret_data = ''
+    peer_length = len(comm_inst.onlinePeers)
+    if peer_length <= 0:
+        raise onionrexceptions.OnlinePeerNeeded
+
     while True:
         peer_length = len(comm_inst.onlinePeers)
-        if peer_length <= 0:
+
-            break
         try:
-            # get a random online peer, securely. May get stuck in loop if network is lost or if all peers in pool magically disconnect at once
+            # Get a random online peer, securely.
+            # May get stuck in loop if network is lost or if all peers in pool magically disconnect at once
             ret_data = comm_inst.onlinePeers[secrets.randbelow(peer_length)]
         except IndexError:
             pass

src/communicatorutils/announcenode.py

@@ -25,6 +25,8 @@ from utils import gettransports
 from netcontroller import NetController
 from communicator import onlinepeers
 from coredb import keydb
+import onionrexceptions
+
 def announce_node(daemon):
     '''Announce our node to our peers'''
     ret_data = False
@@ -41,12 +43,17 @@ def announce_node(daemon):
                 peer = i
                 break
         else:
-            peer = onlinepeers.pick_online_peer(daemon)
+            try:
+                peer = onlinepeers.pick_online_peer(daemon)
+            except onionrexceptions.OnlinePeerNeeded:
+                peer = ""
 
-        for x in range(1):
+        for _ in range(1):
             try:
                 ourID = gettransports.get()[0]
-            except IndexError:
+                if not peer:
+                    raise onionrexceptions.OnlinePeerNeeded
+            except (IndexError, onionrexceptions.OnlinePeerNeeded):
                 break
 
             url = 'http://' + peer + '/announce'

src/communicatorutils/downloadblocks/__init__.py

@@ -43,8 +43,7 @@ def download_blocks_from_communicator(comm_inst: "OnionrCommunicatorDaemon"):
     # Iterate the block queue in the communicator
     for blockHash in list(comm_inst.blockQueue):
         count += 1
-        if len(comm_inst.onlinePeers) == 0:
+
-            break
         triedQueuePeers = [] # List of peers we've tried for a block
         try:
             blockPeers = list(comm_inst.blockQueue[blockHash])
@@ -62,9 +61,15 @@ def download_blocks_from_communicator(comm_inst: "OnionrCommunicatorDaemon"):
         if blockHash in comm_inst.currentDownloading:
             continue
 
+        if len(comm_inst.onlinePeers) == 0:
+            break
+
         comm_inst.currentDownloading.append(blockHash) # So we can avoid concurrent downloading in other threads of same block
         if len(blockPeers) == 0:
-            peerUsed = onlinepeers.pick_online_peer(comm_inst)
+            try:
+                peerUsed = onlinepeers.pick_online_peer(comm_inst)
+            except onionrexceptions.OnlinePeerNeeded:
+                continue
         else:
             blockPeers = onionrcrypto.cryptoutils.random_shuffle(blockPeers)
             peerUsed = blockPeers.pop(0)

src/communicatorutils/lookupadders.py

@@ -21,6 +21,7 @@ import logger
 from onionrutils import stringvalidators
 from communicator import peeraction, onlinepeers
 from utils import gettransports
+import onionrexceptions
 def lookup_new_peer_transports_with_communicator(comm_inst):
     logger.info('Looking up new addresses...')
     tryAmount = 1
@@ -32,8 +33,11 @@ def lookup_new_peer_transports_with_communicator(comm_inst):
         if len(newPeers) > 10000:
             # Don't get new peers if we have too many queued up
             break
-        peer = onlinepeers.pick_online_peer(comm_inst)
+        try:
-        newAdders = peeraction.peer_action(comm_inst, peer, action='pex')
+            peer = onlinepeers.pick_online_peer(comm_inst)
+            newAdders = peeraction.peer_action(comm_inst, peer, action='pex')
+        except onionrexceptions.OnlinePeerNeeded:
+            continue
         try:
             newPeers = newAdders.split(',')
         except AttributeError:

src/communicatorutils/lookupblocks.py

@@ -17,12 +17,15 @@
     You should have received a copy of the GNU General Public License
     along with this program.  If not, see <https://www.gnu.org/licenses/>.
 '''
+from gevent import time
+
 import logger, onionrproofs
 from onionrutils import stringvalidators, epoch
 from communicator import peeraction, onlinepeers
 from coredb import blockmetadb
 from utils import reconstructhash
 from onionrblocks import onionrblacklist
+import onionrexceptions
 blacklist = onionrblacklist.OnionrBlackList()
 def lookup_blocks_from_communicator(comm_inst):
     logger.info('Looking up new blocks')
@@ -43,7 +46,12 @@ def lookup_blocks_from_communicator(comm_inst):
         if comm_inst.storage_counter.is_full():
             logger.debug('Not looking up new blocks due to maximum amount of allowed disk space used')
             break
-        peer = onlinepeers.pick_online_peer(comm_inst) # select random online peer
+        try:
+            # select random online peer
+            peer = onlinepeers.pick_online_peer(comm_inst)
+        except onionrexceptions.OnlinePeerNeeded:
+            time.sleep(1)
+            continue
         # if we've already tried all the online peers this time around, stop
         if peer in triedPeers:
             if len(comm_inst.onlinePeers) == len(triedPeers):

src/communicatorutils/uploadblocks/__init__.py

@@ -45,8 +45,11 @@ def upload_blocks_from_communicator(comm_inst: OnionrCommunicatorDaemon):
                 comm_inst.decrementThreadCount(TIMER_NAME)
                 return
             session = session_manager.add_session(bl)
-            for i in range(min(len(comm_inst.onlinePeers), 6)):
+            for _ in range(min(len(comm_inst.onlinePeers), 6)):
-                peer = onlinepeers.pick_online_peer(comm_inst)
+                try:
+                    peer = onlinepeers.pick_online_peer(comm_inst)
+                except onionrexceptions.OnlinePeerNeeded:
+                    continue
                 try:
                     session.peer_exists[peer]
                     continue