Skip to content

Commit 08a3c4e

Browse files
committed
refactor mapport to use test friendly pattern
1 parent ae12d2b commit 08a3c4e

File tree

4 files changed

+184
-9
lines changed

4 files changed

+184
-9
lines changed

src/mapport.cpp

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <random.h>
1818
#include <util/thread.h>
1919
#include <util/threadinterrupt.h>
20+
#include <mapport_hooks.h>
2021

2122
#ifdef USE_UPNP
2223
#include <cstddef> // workaround missing include in miniupnpc 2.3.3
@@ -35,6 +36,7 @@ static_assert(MINIUPNPC_API_VERSION >= 17, "miniUPnPc API version >= 17 assumed"
3536
#include <string>
3637
#include <thread>
3738

39+
3840
static CThreadInterrupt g_mapport_interrupt;
3941
static std::thread g_mapport_thread;
4042
static std::atomic_uint g_mapport_enabled_protos{MapPortProtoFlag::NONE};
@@ -79,7 +81,7 @@ static bool ProcessPCP()
7981
ret = false; // Set to true if any mapping succeeds.
8082

8183
// IPv4
82-
std::optional<CNetAddr> gateway4 = QueryDefaultGateway(NET_IPV4);
84+
std::optional<CNetAddr> gateway4 = mapport_hooks::QueryDefaultGatewayFn(NET_IPV4);
8385
if (!gateway4) {
8486
LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "portmap: Could not determine IPv4 default gateway\n");
8587
} else {
@@ -88,26 +90,26 @@ static bool ProcessPCP()
8890
// Open a port mapping on whatever local address we have toward the gateway.
8991
struct in_addr inaddr_any;
9092
inaddr_any.s_addr = htonl(INADDR_ANY);
91-
auto res = PCPRequestPortMap(pcp_nonce, *gateway4, CNetAddr(inaddr_any), private_port, requested_lifetime, g_mapport_interrupt);
93+
auto res = mapport_hooks::PCPRequestPortMapFn(pcp_nonce, *gateway4, CNetAddr(inaddr_any), private_port, requested_lifetime, g_mapport_interrupt);
9294
MappingError* pcp_err = std::get_if<MappingError>(&res);
9395
if (pcp_err && *pcp_err == MappingError::UNSUPP_VERSION) {
9496
LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "portmap: Got unsupported PCP version response, falling back to NAT-PMP\n");
95-
res = NATPMPRequestPortMap(*gateway4, private_port, requested_lifetime, g_mapport_interrupt);
97+
res = mapport_hooks::NATPMPRequestPortMapFn(*gateway4, private_port, requested_lifetime, g_mapport_interrupt);
9698
}
9799
handle_mapping(res);
98100
}
99101

100102
// IPv6
101-
std::optional<CNetAddr> gateway6 = QueryDefaultGateway(NET_IPV6);
103+
std::optional<CNetAddr> gateway6 = mapport_hooks::QueryDefaultGatewayFn(NET_IPV6);
102104
if (!gateway6) {
103105
LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "portmap: Could not determine IPv6 default gateway\n");
104106
} else {
105107
LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "portmap: gateway [IPv6]: %s\n", gateway6->ToStringAddr());
106108

107109
// Try to open pinholes for all routable local IPv6 addresses.
108-
for (const auto &addr: GetLocalAddresses()) {
110+
for (const auto &addr: mapport_hooks::GetLocalAddressesFn()) {
109111
if (!addr.IsRoutable() || !addr.IsIPv6()) continue;
110-
auto res = PCPRequestPortMap(pcp_nonce, *gateway6, addr, private_port, requested_lifetime, g_mapport_interrupt);
112+
auto res = mapport_hooks::PCPRequestPortMapFn(pcp_nonce, *gateway6, addr, private_port, requested_lifetime, g_mapport_interrupt);
111113
handle_mapping(res);
112114
}
113115
}

src/mapport_hooks.h

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// Copyright (c) 2025 The Bitcoin Knots developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#pragma once
6+
7+
#include <common/netif.h>
8+
#include <common/pcp.h>
9+
#include <netaddress.h>
10+
#include <util/threadinterrupt.h>
11+
12+
#include <optional>
13+
#include <variant>
14+
#include <vector>
15+
16+
// Lightweight indirection hooks for mapport's network-dependent helpers.
17+
// These function-pointer hooks default to the real implementations in
18+
// production, and can be reassigned by unit tests at runtime to inject
19+
// deterministic behavior without performing any real network I/O.
20+
namespace mapport_hooks {
21+
using QueryDefaultGateway_t = std::optional<CNetAddr>(*)(Network);
22+
using PCPRequestPortMap_t = std::variant<MappingResult, MappingError>(*)(
23+
const PCPMappingNonce&, const CNetAddr&, const CNetAddr&, uint16_t, uint32_t, CThreadInterrupt&);
24+
using NATPMPRequestPortMap_t = std::variant<MappingResult, MappingError>(*)(
25+
const CNetAddr&, uint16_t, uint32_t, CThreadInterrupt&);
26+
using GetLocalAddresses_t = std::vector<CNetAddr>(*)();
27+
28+
// Defaults point to the real implementations. Tests may override these at runtime.
29+
inline QueryDefaultGateway_t QueryDefaultGatewayFn = QueryDefaultGateway;
30+
inline PCPRequestPortMap_t PCPRequestPortMapFn = [](const PCPMappingNonce& nonce, const CNetAddr& gateway,
31+
const CNetAddr& bind, uint16_t port, uint32_t lifetime,
32+
CThreadInterrupt& interrupt) {
33+
return PCPRequestPortMap(nonce, gateway, bind, port, lifetime, interrupt);
34+
};
35+
inline NATPMPRequestPortMap_t NATPMPRequestPortMapFn = [](const CNetAddr& gateway, uint16_t port,
36+
uint32_t lifetime, CThreadInterrupt& interrupt) {
37+
return NATPMPRequestPortMap(gateway, port, lifetime, interrupt);
38+
};
39+
inline GetLocalAddresses_t GetLocalAddressesFn = [](){ return GetLocalAddresses(); };
40+
} // namespace mapport_hooks

src/mapport_testing.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Copyright (c) 2025 The Bitcoin Knots developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#pragma once
6+
7+
// Backward-compatibility header. Prefer including <mapport_hooks.h> directly.
8+
// This header provides a namespace alias so existing includes continue to work
9+
// without referencing the term "testing" in production symbols.
10+
#include <mapport_hooks.h>
11+
12+
namespace mapport_testing = mapport_hooks;

src/test/mapport_tests.cpp

Lines changed: 124 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,50 @@
55
#include <boost/test/unit_test.hpp>
66

77
#include <mapport.h>
8+
#include <chrono>
9+
#include <thread>
10+
#include <test/util/setup_common.h>
11+
#include <netaddress.h>
12+
#include <netbase.h>
13+
#include <util/threadinterrupt.h>
14+
#include <mapport_hooks.h>
15+
16+
// Simple stub implementations matching the hook signatures (no network IO)
17+
static std::optional<CNetAddr> StubNoGateway(Network) { return std::nullopt; }
18+
static std::vector<CNetAddr> StubNoLocalAddrs() { return {}; }
19+
static std::variant<MappingResult, MappingError> StubPCPNoResources(
20+
const PCPMappingNonce&, const CNetAddr&, const CNetAddr&, uint16_t, uint32_t, CThreadInterrupt&)
21+
{
22+
return MappingError{MappingError::NO_RESOURCES};
23+
}
24+
static std::variant<MappingResult, MappingError> StubPMPNoResources(
25+
const CNetAddr&, uint16_t, uint32_t, CThreadInterrupt&)
26+
{
27+
return MappingError{MappingError::NO_RESOURCES};
28+
}
29+
30+
struct MapportStubGuard {
31+
// Save originals
32+
decltype(mapport_hooks::QueryDefaultGatewayFn) qdg_orig = mapport_hooks::QueryDefaultGatewayFn;
33+
decltype(mapport_hooks::PCPRequestPortMapFn) pcp_orig = mapport_hooks::PCPRequestPortMapFn;
34+
decltype(mapport_hooks::NATPMPRequestPortMapFn) pmp_orig = mapport_hooks::NATPMPRequestPortMapFn;
35+
decltype(mapport_hooks::GetLocalAddressesFn) gla_orig = mapport_hooks::GetLocalAddressesFn;
36+
~MapportStubGuard() {
37+
mapport_hooks::QueryDefaultGatewayFn = qdg_orig;
38+
mapport_hooks::PCPRequestPortMapFn = pcp_orig;
39+
mapport_hooks::NATPMPRequestPortMapFn = pmp_orig;
40+
mapport_hooks::GetLocalAddressesFn = gla_orig;
41+
}
42+
};
843

944
// These tests intentionally avoid enabling any mapping protocol in order to
10-
// exercise the control flow in mapport.cpp without starting background threads
11-
// or performing any network operations.
45+
// exercise the control flow in mapport.cpp without performing any real
46+
// network operations. We rely on the fact that UPnP support is not compiled
47+
// in this build (USE_UPNP undefined). Enabling UPnP will still start the
48+
// mapport thread, but it won't attempt any UPnP work; we immediately
49+
// interrupt to keep the test fast and deterministic.
1250

13-
BOOST_AUTO_TEST_SUITE(mapport_tests)
51+
BOOST_FIXTURE_TEST_SUITE(mapport_tests, BasicTestingSetup)
1452

1553
BOOST_AUTO_TEST_CASE(start_stop_no_protocols)
1654
{
@@ -26,4 +64,87 @@ BOOST_AUTO_TEST_CASE(start_stop_no_protocols)
2664
StopMapPort();
2765
}
2866

67+
BOOST_AUTO_TEST_CASE(start_thread_with_upnp_only_then_interrupt)
68+
{
69+
// Start the background thread by enabling only UPnP (no actual UPnP code
70+
// will run in this build). Immediately interrupt and stop it to exercise
71+
// ThreadMapPort(), StartThreadMapPort(), InterruptMapPort(), and StopMapPort().
72+
StartMapPort(true, false);
73+
74+
// Give the thread a tiny slice to start. Not strictly necessary, but helps
75+
// stabilize coverage across machines.
76+
std::this_thread::sleep_for(std::chrono::milliseconds(1));
77+
78+
InterruptMapPort();
79+
StopMapPort();
80+
81+
// Calling stop again should be a no-op.
82+
StopMapPort();
83+
}
84+
85+
BOOST_AUTO_TEST_CASE(repeated_start_calls_are_idempotent)
86+
{
87+
// Start once with UPnP only, then start again. The second call should not
88+
// start a second thread. Interrupt/stop will terminate the single thread.
89+
StartMapPort(true, false);
90+
StartMapPort(true, false);
91+
92+
InterruptMapPort();
93+
StopMapPort();
94+
}
95+
96+
BOOST_AUTO_TEST_CASE(toggle_enable_disable_sequences)
97+
{
98+
// Sequence of toggles to exercise DispatchMapPort paths where
99+
// current==NONE and enabled toggles between NONE and non-NONE.
100+
StartMapPort(false, false); // No thread should be started.
101+
StartMapPort(true, false); // Start thread (UPnP-only path).
102+
103+
// Disable again while thread may be running. The thread will only exit
104+
// after interrupt/stop.
105+
StartMapPort(false, false);
106+
InterruptMapPort();
107+
StopMapPort();
108+
109+
// Final sanity: calls are safe repeatedly.
110+
InterruptMapPort();
111+
StopMapPort();
112+
}
113+
114+
BOOST_AUTO_TEST_CASE(start_with_pcp_only_then_interrupt)
115+
{
116+
// Avoid any real network. Force no default gateways and no local addresses.
117+
MapportStubGuard guard;
118+
mapport_hooks::QueryDefaultGatewayFn = &StubNoGateway;
119+
mapport_hooks::GetLocalAddressesFn = &StubNoLocalAddrs;
120+
mapport_hooks::PCPRequestPortMapFn = &StubPCPNoResources;
121+
mapport_hooks::NATPMPRequestPortMapFn = &StubPMPNoResources;
122+
123+
StartMapPort(false, true);
124+
std::this_thread::sleep_for(std::chrono::milliseconds(1));
125+
InterruptMapPort();
126+
StopMapPort();
127+
}
128+
129+
BOOST_AUTO_TEST_CASE(enabling_another_protocol_does_not_switch)
130+
{
131+
// Avoid network: no gateways so PCP does nothing.
132+
MapportStubGuard guard;
133+
mapport_hooks::QueryDefaultGatewayFn = [](Network){ return std::optional<CNetAddr>{}; };
134+
mapport_hooks::GetLocalAddressesFn = [](){ return std::vector<CNetAddr>{}; };
135+
136+
// Start with PCP only, then enable UPnP in addition. According to
137+
// DispatchMapPort(), enabling another protocol does not switch away from
138+
// the currently used one; the dispatch should early-return.
139+
StartMapPort(false, true); // Start PCP path.
140+
std::this_thread::sleep_for(std::chrono::milliseconds(1));
141+
142+
// Enable UPnP while PCP is active.
143+
StartMapPort(true, true);
144+
145+
// Clean up.
146+
InterruptMapPort();
147+
StopMapPort();
148+
}
149+
29150
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)