Page MenuHomePhabricator

p2p: Refactor sock to add I2P unit test
ClosedPublic

Authored by PiRK on Feb 11 2022, 14:55.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABCde38a2d80d0f: p2p: Refactor sock to add I2P unit test
Summary

net: add connect() and getsockopt() wrappers to Sock

Extend the Sock class with wrappers to connect() and getsockopt().

This will make it possible to mock code which uses those.

https://github.com/bitcoin/bitcoin/pull/21387/commits/b5861100f85fef77b00f55dcdf01ffb4a2a112d8

net: change ConnectSocketDirectly() to take a Sock argument

Change ConnectSocketDirectly() to take a Sock argument instead of a
bare SOCKET. With this, use the Sock's (possibly mocked) methods
Connect(), Wait() and GetSockOpt() instead of calling the OS
functions directly.

https://github.com/bitcoin/bitcoin/pull/21387/commits/82d360b5a88d9057b6c09b61cd69e426c7a2412d

i2p: use pointers to Sock to accommodate mocking

Change the types of i2p::Connection::sock and
i2p::sam::Session::m_control_sock from Sock to
std::unique_ptr<Sock>.

Using pointers would allow us to sneak FuzzedSock instead of Sock
and have the methods of the former called.

After this change a test only needs to replace CreateSock() with
a function that returns FuzzedSock.

https://github.com/bitcoin/bitcoin/pull/21387/commits/9947e44de0cbd79e99d883443a9ac441d8c69713

test: add I2P test for a runaway SAM proxy

Add a regression test for https://github.com/bitcoin/bitcoin/pull/21407.

The test creates a socket that, upon read, returns some data, but never
the expected terminator \n, injects that socket into the I2P code and
expects i2p::sam::Session::Connect() to fail, printing a specific
error message to the log.

https://github.com/bitcoin/bitcoin/pull/21387/commits/40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5

This is a partial backport of core#21387, without the fuzzer changes.
Depends on D11037

Test Plan

ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PiRK requested review of this revision.Feb 11 2022, 14:55
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/netbase.cpp
681 ↗(On Diff #32350)

Not blocking on that, but https://github.com/bitcoin/bitcoin/pull/21444 is missing here

This revision is now accepted and ready to land.Feb 14 2022, 10:31
This revision was automatically updated to reflect the committed changes.