Skip to content

Commit 9947e44

Browse files
committed
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`.
1 parent 82d360b commit 9947e44

File tree

3 files changed

+30
-26
lines changed

3 files changed

+30
-26
lines changed

src/i2p.cpp

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <util/system.h>
2121

2222
#include <chrono>
23+
#include <memory>
2324
#include <stdexcept>
2425
#include <string>
2526

@@ -115,7 +116,8 @@ namespace sam {
115116
Session::Session(const fs::path& private_key_file,
116117
const CService& control_host,
117118
CThreadInterrupt* interrupt)
118-
: m_private_key_file(private_key_file), m_control_host(control_host), m_interrupt(interrupt)
119+
: m_private_key_file(private_key_file), m_control_host(control_host), m_interrupt(interrupt),
120+
m_control_sock(std::make_unique<Sock>(INVALID_SOCKET))
119121
{
120122
}
121123

@@ -145,15 +147,15 @@ bool Session::Accept(Connection& conn)
145147
try {
146148
while (!*m_interrupt) {
147149
Sock::Event occurred;
148-
conn.sock.Wait(MAX_WAIT_FOR_IO, Sock::RECV, &occurred);
150+
conn.sock->Wait(MAX_WAIT_FOR_IO, Sock::RECV, &occurred);
149151

150152
if ((occurred & Sock::RECV) == 0) {
151153
// Timeout, no incoming connections within MAX_WAIT_FOR_IO.
152154
continue;
153155
}
154156

155157
const std::string& peer_dest =
156-
conn.sock.RecvUntilTerminator('\n', MAX_WAIT_FOR_IO, *m_interrupt, MAX_MSG_SIZE);
158+
conn.sock->RecvUntilTerminator('\n', MAX_WAIT_FOR_IO, *m_interrupt, MAX_MSG_SIZE);
157159

158160
conn.peer = CService(DestB64ToAddr(peer_dest), Params().GetDefaultPort());
159161

@@ -171,7 +173,7 @@ bool Session::Connect(const CService& to, Connection& conn, bool& proxy_error)
171173
proxy_error = true;
172174

173175
std::string session_id;
174-
Sock sock;
176+
std::unique_ptr<Sock> sock;
175177
conn.peer = to;
176178

177179
try {
@@ -184,12 +186,12 @@ bool Session::Connect(const CService& to, Connection& conn, bool& proxy_error)
184186
}
185187

186188
const Reply& lookup_reply =
187-
SendRequestAndGetReply(sock, strprintf("NAMING LOOKUP NAME=%s", to.ToStringIP()));
189+
SendRequestAndGetReply(*sock, strprintf("NAMING LOOKUP NAME=%s", to.ToStringIP()));
188190

189191
const std::string& dest = lookup_reply.Get("VALUE");
190192

191193
const Reply& connect_reply = SendRequestAndGetReply(
192-
sock, strprintf("STREAM CONNECT ID=%s DESTINATION=%s SILENT=false", session_id, dest),
194+
*sock, strprintf("STREAM CONNECT ID=%s DESTINATION=%s SILENT=false", session_id, dest),
193195
false);
194196

195197
const std::string& result = connect_reply.Get("RESULT");
@@ -271,7 +273,7 @@ Session::Reply Session::SendRequestAndGetReply(const Sock& sock,
271273
return reply;
272274
}
273275

274-
Sock Session::Hello() const
276+
std::unique_ptr<Sock> Session::Hello() const
275277
{
276278
auto sock = CreateSock(m_control_host);
277279

@@ -285,15 +287,15 @@ Sock Session::Hello() const
285287

286288
SendRequestAndGetReply(*sock, "HELLO VERSION MIN=3.1 MAX=3.1");
287289

288-
return std::move(*sock);
290+
return sock;
289291
}
290292

291293
void Session::CheckControlSock()
292294
{
293295
LOCK(m_mutex);
294296

295297
std::string errmsg;
296-
if (!m_control_sock.IsConnected(errmsg)) {
298+
if (!m_control_sock->IsConnected(errmsg)) {
297299
Log("Control socket error: %s", errmsg);
298300
Disconnect();
299301
}
@@ -341,26 +343,26 @@ Binary Session::MyDestination() const
341343
void Session::CreateIfNotCreatedAlready()
342344
{
343345
std::string errmsg;
344-
if (m_control_sock.IsConnected(errmsg)) {
346+
if (m_control_sock->IsConnected(errmsg)) {
345347
return;
346348
}
347349

348350
Log("Creating SAM session with %s", m_control_host.ToString());
349351

350-
Sock sock = Hello();
352+
auto sock = Hello();
351353

352354
const auto& [read_ok, data] = ReadBinaryFile(m_private_key_file);
353355
if (read_ok) {
354356
m_private_key.assign(data.begin(), data.end());
355357
} else {
356-
GenerateAndSavePrivateKey(sock);
358+
GenerateAndSavePrivateKey(*sock);
357359
}
358360

359361
const std::string& session_id = GetRandHash().GetHex().substr(0, 10); // full is an overkill, too verbose in the logs
360362
const std::string& private_key_b64 = SwapBase64(EncodeBase64(m_private_key));
361363

362-
SendRequestAndGetReply(sock, strprintf("SESSION CREATE STYLE=STREAM ID=%s DESTINATION=%s",
363-
session_id, private_key_b64));
364+
SendRequestAndGetReply(*sock, strprintf("SESSION CREATE STYLE=STREAM ID=%s DESTINATION=%s",
365+
session_id, private_key_b64));
364366

365367
m_my_addr = CService(DestBinToAddr(MyDestination()), Params().GetDefaultPort());
366368
m_session_id = session_id;
@@ -370,12 +372,12 @@ void Session::CreateIfNotCreatedAlready()
370372
m_my_addr.ToString());
371373
}
372374

373-
Sock Session::StreamAccept()
375+
std::unique_ptr<Sock> Session::StreamAccept()
374376
{
375-
Sock sock = Hello();
377+
auto sock = Hello();
376378

377379
const Reply& reply = SendRequestAndGetReply(
378-
sock, strprintf("STREAM ACCEPT ID=%s SILENT=false", m_session_id), false);
380+
*sock, strprintf("STREAM ACCEPT ID=%s SILENT=false", m_session_id), false);
379381

380382
const std::string& result = reply.Get("RESULT");
381383

@@ -393,14 +395,14 @@ Sock Session::StreamAccept()
393395

394396
void Session::Disconnect()
395397
{
396-
if (m_control_sock.Get() != INVALID_SOCKET) {
398+
if (m_control_sock->Get() != INVALID_SOCKET) {
397399
if (m_session_id.empty()) {
398400
Log("Destroying incomplete session");
399401
} else {
400402
Log("Destroying session %s", m_session_id);
401403
}
402404
}
403-
m_control_sock.Reset();
405+
m_control_sock->Reset();
404406
m_session_id.clear();
405407
}
406408
} // namespace sam

src/i2p.h

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <threadinterrupt.h>
1313
#include <util/sock.h>
1414

15+
#include <memory>
1516
#include <optional>
1617
#include <string>
1718
#include <unordered_map>
@@ -29,7 +30,7 @@ using Binary = std::vector<uint8_t>;
2930
*/
3031
struct Connection {
3132
/** Connected socket. */
32-
Sock sock;
33+
std::unique_ptr<Sock> sock;
3334

3435
/** Our I2P address. */
3536
CService me;
@@ -166,7 +167,7 @@ class Session
166167
* @return a connected socket
167168
* @throws std::runtime_error if an error occurs
168169
*/
169-
Sock Hello() const EXCLUSIVE_LOCKS_REQUIRED(m_mutex);
170+
std::unique_ptr<Sock> Hello() const EXCLUSIVE_LOCKS_REQUIRED(m_mutex);
170171

171172
/**
172173
* Check the control socket for errors and possibly disconnect.
@@ -204,10 +205,11 @@ class Session
204205

205206
/**
206207
* Open a new connection to the SAM proxy and issue "STREAM ACCEPT" request using the existing
207-
* session id. Return the idle socket that is waiting for a peer to connect to us.
208+
* session id.
209+
* @return the idle socket that is waiting for a peer to connect to us
208210
* @throws std::runtime_error if an error occurs
209211
*/
210-
Sock StreamAccept() EXCLUSIVE_LOCKS_REQUIRED(m_mutex);
212+
std::unique_ptr<Sock> StreamAccept() EXCLUSIVE_LOCKS_REQUIRED(m_mutex);
211213

212214
/**
213215
* Destroy the session, closing the internally used sockets.
@@ -248,7 +250,7 @@ class Session
248250
* connections and make outgoing ones.
249251
* See https://geti2p.net/en/docs/api/samv3
250252
*/
251-
Sock m_control_sock GUARDED_BY(m_mutex);
253+
std::unique_ptr<Sock> m_control_sock GUARDED_BY(m_mutex);
252254

253255
/**
254256
* Our .b32.i2p address.

src/net.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
432432
i2p::Connection conn;
433433
if (m_i2p_sam_session->Connect(addrConnect, conn, proxyConnectionFailed)) {
434434
connected = true;
435-
sock = std::make_unique<Sock>(std::move(conn.sock));
435+
sock = std::move(conn.sock);
436436
addr_bind = CAddress{conn.me, NODE_NONE};
437437
}
438438
} else if (GetProxy(addrConnect.GetNetwork(), proxy)) {
@@ -2221,7 +2221,7 @@ void CConnman::ThreadI2PAcceptIncoming()
22212221
continue;
22222222
}
22232223

2224-
CreateNodeFromAcceptedSocket(conn.sock.Release(), NetPermissionFlags::PF_NONE,
2224+
CreateNodeFromAcceptedSocket(conn.sock->Release(), NetPermissionFlags::PF_NONE,
22252225
CAddress{conn.me, NODE_NONE}, CAddress{conn.peer, NODE_NONE});
22262226
}
22272227
}

0 commit comments

Comments
 (0)