Repository: ethereum/web3.py
Pull request #3537 (2024-11-22 23:04:47)

bugfix: include new line delimiter in messages sent over IPC
### What was wrong?

- ``IPCProvider`` and ``AsyncIPCProvider`` should use a delimiter to separate messages when writing to the socket.

### How was it fixed?

- Add `b"\n"` at the end of the encoded message when sending messages via IPC. Tests passed before because Geth accounts for this. However, when testing against Nethermind, all calls will hang without these changes.
- Add tests to make sure we don't accidentally remove this at some point.

### Todo:

- [x] Clean up commit history
- [x] Add entry to the [release notes](https://github.com/ethereum/web3.py/blob/main/newsfragments/README.md)

#### Cute Animal Picture

```
        ,--._______,-.
       ,','  ,    .  ,_`-.
      / /  ,' , _` ``. |  )       `-..
     (,';'""`/ '"`-._ ` \/ ______    \\
       : ,o.-`- ,o.  )\` -'      `---.))
       : , d8b ^-.   '|   `.      `    `.
       |/ __:_     `. |  ,  `       `    \
       | ( ,-.`-.    ;'  ;   `       :    ;
       | |  ,   `.      /     ;      :    \
       ;-'`:::._,`.__),'             :     ;
      / ,  `-   `--                  ;     |
     /  \                   `       ,      |
    (    `     :              :    ,\      |
     \   `.    :     :        :  ,'  \    :
      \    `|-- `     \ ,'    ,-'     :-.-';
      :     |`--.______;     |        :    :
       :    /           |    |         |   \
       |    ;           ;    ;        /     ;
     _/--' |           :`-- /         \_:_:_|
   ,',','  |           |___ \           

```


fselmo: > LGTM! Approving, but is there a good way to test this?

Yes! What thinking to add at least a simple test before merging. I will do this before I merge to make sure the messages we send end in `b"\n"`.

Patch:
diff --git a/newsfragments/3537.bugfix.rst b/newsfragments/3537.bugfix.rst
new file mode 100644
index 00000000..6f8474c7
--- /dev/null
+++ b/newsfragments/3537.bugfix.rst
@@ -0,0 +1,1 @@
+Include an end-of-line delimiter when sending messages via IPC with the ``IPCProvider`` and ``AsyncIPCProvider``.
diff --git a/web3/providers/ipc.py b/web3/providers/ipc.py
index 9ea72157..349cb1a4 100644
--- a/web3/providers/ipc.py
+++ b/web3/providers/ipc.py
@@ -164,7 +164,7 @@ class IPCProvider(JSONBaseProvider):
     def _make_request(self, request: bytes) -> RPCResponse:
         with self._lock, self._socket as sock:
             try:
-                sock.sendall(request)
+                sock.sendall(request + b"\n")
             except BrokenPipeError:
                 # one extra attempt, then give up
                 sock = self._socket.reset()
diff --git a/web3/providers/persistent/async_ipc.py b/web3/providers/persistent/async_ipc.py
index 6b09cae0..d9a18628 100644
--- a/web3/providers/persistent/async_ipc.py
+++ b/web3/providers/persistent/async_ipc.py
@@ -116,7 +116,7 @@ class AsyncIPCProvider(PersistentConnectionProvider):
 
     async def _socket_send(self, request_data: bytes) -> None:
         try:
-            self._writer.write(request_data)
+            self._writer.write(request_data + b"\n")
             await self._writer.drain()
         except OSError as e:
             # Broken pipe


Test patch:
diff --git a/tests/core/providers/test_async_ipc_provider.py b/tests/core/providers/test_async_ipc_provider.py
index 10a7bbbd..845d1403 100644
--- a/tests/core/providers/test_async_ipc_provider.py
+++ b/tests/core/providers/test_async_ipc_provider.py
@@ -8,6 +8,10 @@ from threading import (
     Thread,
 )
 import time
+from unittest.mock import (
+    AsyncMock,
+    Mock,
+)
 
 from websockets import (
     ConnectionClosed,
@@ -43,7 +47,7 @@ ETH_SUBSCRIBE_RESPONSE = {
 
 TWENTY_MB = 20 * 1024 * 1024
 SIZED_MSG_START = b'{"id": 0, "jsonrpc": "2.0", "result": "'
-SIZED_MSG_END = b'"}\n' b""
+SIZED_MSG_END = b'"}\n'
 
 
 @pytest.fixture
@@ -100,9 +104,9 @@ def ipc_server_fixture(simple_ipc_server):
 @pytest.fixture
 def serve_empty_result(ipc_server_fixture):
     def response_fn(connection):
-        connection.sendall(b'{"id": 0, "result": {}')
+        connection.sendall(b'{"id": 0, "result": {}}')
         time.sleep(0.1)
-        connection.sendall(b"}\n")
+        connection.sendall(b"\n")
 
     yield from ipc_server_fixture(response_fn)
 
@@ -342,3 +346,20 @@ async def test_async_ipc_read_buffer_limit_is_configurable(
             len(response["result"])
             == TWENTY_MB - len(SIZED_MSG_START) - len(SIZED_MSG_END) + 1024
         )
+
+
+@pytest.mark.asyncio
+async def test_async_ipc_provider_write_messages_end_with_new_line_delimiter(
+    simple_ipc_server,
+    jsonrpc_ipc_pipe_path,
+):
+    async with AsyncWeb3(AsyncIPCProvider(pathlib.Path(jsonrpc_ipc_pipe_path))) as w3:
+        w3.provider._writer.write = Mock()
+        w3.provider._reader.readline = AsyncMock(
+            return_value=b'{"id": 0, "result": {}}\n'
+        )
+
+        await w3.provider.make_request("method", [])
+
+        request_data = b'{"jsonrpc": "2.0", "method": "method", "params": [], "id": 0}'
+        w3.provider._writer.write.assert_called_with(request_data + b"\n")
diff --git a/tests/core/providers/test_ipc_provider.py b/tests/core/providers/test_ipc_provider.py
index 14cb26c6..4e465594 100644
--- a/tests/core/providers/test_ipc_provider.py
+++ b/tests/core/providers/test_ipc_provider.py
@@ -9,6 +9,7 @@ from threading import (
 )
 import time
 from unittest.mock import (
+    Mock,
     patch,
 )
 
@@ -185,3 +186,14 @@ def test_web3_auto_gethdev(request_mocker):
 
     assert "extraData" not in block
     assert block.proofOfAuthorityData == b"\xff" * 33
+
+
+def test_ipc_provider_write_messages_end_with_new_line_delimiter(jsonrpc_ipc_pipe_path):
+    provider = IPCProvider(pathlib.Path(jsonrpc_ipc_pipe_path), timeout=3)
+    provider._socket.sock = Mock()
+    provider._socket.sock.recv.return_value = b'{"id":1, "result": {}}\n'
+
+    provider.make_request("method", [])
+
+    request_data = b'{"jsonrpc": "2.0", "method": "method", "params": [], "id": 0}'
+    provider._socket.sock.sendall.assert_called_with(request_data + b"\n")