Skip to content

Commit

Permalink
Fix a bug where a socket connection to "bsv.aftrek.org" is successful… (
Browse files Browse the repository at this point in the history
#1132)

* Fix a bug where a socket connection to "bsv.aftrek.org" is successfully established but on protocol negotiation, the result is "None"

- This crashes the entire asyncio event loop with "TypeError: cannot unpack non-iterable NoneType object"
- Catch the raised TypeError to avoid crashing the asyncio event loop.

* Remove hardcoding of the blacklist attribute

- Servers will now be blacklisted if `DisconnectSessionError` is raised with blacklist=True

* Add unittest for various bad inputs to `protocol_tuple`

- Added type assertions to `_negotiate_protocol` handler to ensure bad responses from a misbehaving ElectrumX server are appropriately caught.
  • Loading branch information
AustEcon committed Aug 17, 2023
1 parent f275274 commit b7f2a91
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 4 deletions.
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,5 @@ Pipfile
Pipfile.lock

# local data directories
electrum_sv_data/regtest/*
electrum_sv_data/*
instance_*
7 changes: 5 additions & 2 deletions electrumsv/network.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ class DisconnectSessionError(Exception):

def __init__(self, reason, *, blacklist=False):
super().__init__(reason)
self.blacklist = False
self.blacklist = blacklist


class SVServerState:
Expand Down Expand Up @@ -461,11 +461,14 @@ async def _negotiate_protocol(self):
args = (PACKAGE_VERSION, [ version_string(PROTOCOL_MIN), version_string(PROTOCOL_MAX) ])
try:
server_string, protocol_string = await self.send_request(method, args)
assert isinstance(server_string, str)
assert isinstance(protocol_string, str)
self.logger.debug(f'server string: {server_string}')
self.logger.debug(f'negotiated protocol: {protocol_string}')
self.ptuple = protocol_tuple(protocol_string)
assert len(self.ptuple) in (2, 3)
assert PROTOCOL_MIN <= self.ptuple <= PROTOCOL_MAX
except (AssertionError, ValueError) as e:
except (AssertionError, TypeError, ValueError) as e:
raise DisconnectSessionError(f'{method} failed: {e}', blacklist=True)

async def _get_checkpoint_headers(self):
Expand Down
34 changes: 33 additions & 1 deletion electrumsv/tests/test_util.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import pytest
import unittest

from electrumsv.util import format_satoshis, get_identified_release_signers
from electrumsv.util import format_satoshis, get_identified_release_signers, protocol_tuple
from electrumsv.util.cache import LRUCache

from .conftest import get_tx_datacarrier_size, get_tx_small_size
Expand All @@ -12,6 +12,38 @@


class TestUtil(unittest.TestCase):
def test_protocol_tuple(self):
bad_input1 = ""
bad_input2 = "..."
bad_input3 = 100
bad_input4 = [1, 4, 2]
bad_input5 = 1.4
bad_input6 = (1, 4, 2)
bad_input7 = "letters"
bad_input8 = None

good_input1 = "1.4.2"
good_input2 = "1.4"
with pytest.raises(ValueError):
protocol_tuple(bad_input1)
with pytest.raises(ValueError):
protocol_tuple(bad_input2)
with pytest.raises(ValueError):
protocol_tuple(bad_input3)
with pytest.raises(ValueError):
protocol_tuple(bad_input4)
with pytest.raises(ValueError):
protocol_tuple(bad_input5)
with pytest.raises(ValueError):
protocol_tuple(bad_input6)
with pytest.raises(ValueError):
protocol_tuple(bad_input7)
with pytest.raises(ValueError):
protocol_tuple(bad_input8)

assert protocol_tuple(good_input1) == (1, 4, 2)
assert protocol_tuple(good_input2) == (1, 4)

def test_format_satoshis(self):
result = format_satoshis(1234)
expected = "0.00001234"
Expand Down

0 comments on commit b7f2a91

Please sign in to comment.