test: Potential test errors, due to assumptions on socket data chunking (interface_http, ...) #36726

Summary: A test run with a forced reproducer, with changed socket data send chunking discovered issues with a few tests.

All networking code using sockets must not make assumption about the chunking of the incoming/outgoing data, and should be resilient against any chunking. Normally, the chunking of the sends of the writer dictate the chunking of the receives at the reader, but this is not guaranteed by IP/TCP, intermediate networking elements may change the packet structure. In test using sockets on the same host, this is not very likely, but still possible.

A test run was executed with a stress test reproducer. For every socket send, sending was changed to break up data in small chunks. This forcedly changed chunking of the data stream. This way code was tested for robustness in face of unexpected chunking of the data. The run is here: #36724 , the reproducer can be seen here .

A few test failed, which where analyzed.

On the positive note, NO NON-TEST ISSUES were identified.

The list of failed tests with the failure root cause:

Failed tests:

1. interface_http.py -- Failed in several CI runs. A wrong assumption in the test; it is implicitly assumed that an HTTP response arrives in a single recv. Being fixed, #36729 .

2. rpc_setban.py --v2transport Failed in two CI runs (1 2). The reason is more subtle, the tested use case can have two outcomes, depending on weather a full key (64 bytes) could be sent on a closing connection or not (ShouldReconnectV1()). The second case is very unlikely, but still can happen. The test is only prepared for the first outcome, with the reproduces the second can happen, causing it to fail. Up for discussion whether should be fixed or kept as-is.

3. p2p_v2_encrypted.py -- Failed once, a time-out occurred (on Windows). Also related to v1/v2 transport selection (ShouldReconnectV1()), but the reason is in the test framework (p2p.py, 358-361). The Python v1 peer disconnects after reading 4 bytes. If they are sent in separate syscalls, the sender node will not be able to send all 64 key bytes. This will cause ShouldReconnectV1() to return false, and the node will not reconnect. The test waits on reconnect. A potential fix is to change Python v1 parser to behave like bitcoind by moving the magic check after the full-header length check. Fix is attempted in #36730 .

A note regarding chance of occurrence. When a test makes an assumption about data chunking (1.), it can pass reliably in CI runs and local tests. However, this is still a latent bug, as there is no guarantee for correct execution. Data chunking can change between sender and receiver. So the fix is recommended, to make the test more resilient, but the issue is low priority.

PR #36729 raised to fix interface_http.

PR #36730 raised to fix p2p_v2_encrypted.py, with a small change to the test framework p2p v1 peer.