mirrors/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2024-11-20 10:38:42 +01:00
Code Issues Projects Releases Wiki Activity

Merge #19951: net, test: CNetAddr scoped ipv6 test coverage, rename scopeId to m_scope_id

Browse Source 
f36887fa47 net: rename CNetAddr scopeId to m_scope_id, improve code doc (Jon Atack)
5cb5fd3005 test: add test coverage for CNetAddr ipv6 scoped addresses (Jon Atack)

Pull request description:

  Add some test coverage for the IPv6 scoped address feature in `netaddress`/`netbase` per https://tools.ietf.org/html/rfc4007, update the member name `scopeId` to `m_scope_id` per `doc/developer-notes.md`, and improve the code doc.

  ----

  Reviewers can manually verify the test with these steps:

  - [pull down the changes locally and check out this branch](https://jonatack.github.io/articles/how-to-review-pull-requests-in-bitcoin-core#pull-down-the-code-locally)

  - [build Bitcoin Core](https://jonatack.github.io/articles/how-to-compile-bitcoin-core-and-run-the-tests)

  - run the test: `cd src ; test/test_bitcoin -t net_tests/cnetaddr_basic -l all`

  <details><summary><em>you should see passing test output like this</em></summary><p>

  ```
  Running 1 test case...
  Entering test module "Bitcoin Core Test Suite"
  test/net_tests.cpp(85): Entering test suite "net_tests"
  test/net_tests.cpp(204): Entering test case "cnetaddr_basic"
  test/net_tests.cpp(210): info: check LookupHost("0.0.0.0", addr, false) has passed
  test/net_tests.cpp(211): info: check !addr.IsValid() has passed
  test/net_tests.cpp(212): info: check addr.IsIPv4() has passed
  test/net_tests.cpp(214): info: check addr.IsBindAny() has passed
  test/net_tests.cpp(215): info: check addr.ToString() == "0.0.0.0" has passed
  test/net_tests.cpp(218): info: check LookupHost("255.255.255.255", addr, false) has passed
  test/net_tests.cpp(219): info: check !addr.IsValid() has passed
  test/net_tests.cpp(220): info: check addr.IsIPv4() has passed
  test/net_tests.cpp(222): info: check !addr.IsBindAny() has passed
  test/net_tests.cpp(223): info: check addr.ToString() == "255.255.255.255" has passed
  test/net_tests.cpp(226): info: check LookupHost("12.34.56.78", addr, false) has passed
  test/net_tests.cpp(227): info: check addr.IsValid() has passed
  test/net_tests.cpp(228): info: check addr.IsIPv4() has passed
  test/net_tests.cpp(230): info: check !addr.IsBindAny() has passed
  test/net_tests.cpp(231): info: check addr.ToString() == "12.34.56.78" has passed
  test/net_tests.cpp(234): info: check LookupHost("::", addr, false) has passed
  test/net_tests.cpp(235): info: check !addr.IsValid() has passed
  test/net_tests.cpp(236): info: check addr.IsIPv6() has passed
  test/net_tests.cpp(238): info: check addr.IsBindAny() has passed
  test/net_tests.cpp(239): info: check addr.ToString() == "::" has passed
  test/net_tests.cpp(242): info: check LookupHost(ipv6_addr, addr, false) has passed
  test/net_tests.cpp(243): info: check addr.IsValid() has passed
  test/net_tests.cpp(244): info: check addr.IsIPv6() has passed
  test/net_tests.cpp(246): info: check !addr.IsBindAny() has passed
  test/net_tests.cpp(247): info: check addr.ToString() == ipv6_addr has passed
  test/net_tests.cpp(254): info: check LookupHost(scoped_addr, addr, false) has passed
  test/net_tests.cpp(255): info: check addr.IsValid() has passed
  test/net_tests.cpp(256): info: check addr.IsIPv6() has passed
  test/net_tests.cpp(257): info: check !addr.IsBindAny() has passed
  test/net_tests.cpp(258): info: check addr.ToString() == scoped_addr has passed
  test/net_tests.cpp(254): info: check LookupHost(scoped_addr, addr, false) has passed
  test/net_tests.cpp(255): info: check addr.IsValid() has passed
  test/net_tests.cpp(256): info: check addr.IsIPv6() has passed
  test/net_tests.cpp(257): info: check !addr.IsBindAny() has passed
  test/net_tests.cpp(258): info: check addr.ToString() == scoped_addr has passed
  test/net_tests.cpp(254): info: check LookupHost(scoped_addr, addr, false) has passed
  test/net_tests.cpp(255): info: check addr.IsValid() has passed
  test/net_tests.cpp(256): info: check addr.IsIPv6() has passed
  test/net_tests.cpp(257): info: check !addr.IsBindAny() has passed
  test/net_tests.cpp(258): info: check addr.ToString() == scoped_addr has passed
  test/net_tests.cpp(254): info: check LookupHost(scoped_addr, addr, false) has passed
  test/net_tests.cpp(255): info: check addr.IsValid() has passed
  test/net_tests.cpp(256): info: check addr.IsIPv6() has passed
  test/net_tests.cpp(257): info: check !addr.IsBindAny() has passed
  test/net_tests.cpp(258): info: check addr.ToString() == scoped_addr has passed
  test/net_tests.cpp(263): info: check !LookupHost(ipv6_addr + "%" + zone_id, addr, false) has passed
  test/net_tests.cpp(263): info: check !LookupHost(ipv6_addr + "%" + zone_id, addr, false) has passed
  test/net_tests.cpp(263): info: check !LookupHost(ipv6_addr + "%" + zone_id, addr, false) has passed
  test/net_tests.cpp(266): info: check LookupHost(ipv6_addr + "%0", addr, false) has passed
  test/net_tests.cpp(267): info: check addr.IsValid() has passed
  test/net_tests.cpp(268): info: check addr.IsIPv6() has passed
  test/net_tests.cpp(269): info: check !addr.IsBindAny() has passed
  test/net_tests.cpp(270): info: check addr.ToString() == ipv6_addr has passed
  test/net_tests.cpp(274): info: check addr.IsValid() has passed
  test/net_tests.cpp(275): info: check addr.IsTor() has passed
  test/net_tests.cpp(277): info: check !addr.IsBindAny() has passed
  test/net_tests.cpp(278): info: check addr.ToString() == "6hzph5hv6337r6p2.onion" has passed
  test/net_tests.cpp(282): info: check !addr.IsValid() has passed
  test/net_tests.cpp(283): info: check addr.IsInternal() has passed
  test/net_tests.cpp(285): info: check !addr.IsBindAny() has passed
  test/net_tests.cpp(286): info: check addr.ToString() == "esffpvrt3wpeaygy.internal" has passed
  test/net_tests.cpp(204): Leaving test case "cnetaddr_basic"; testing time: 30933us
  test/net_tests.cpp(85): Leaving test suite "net_tests"; testing time: 30975us
  Leaving test module "Bitcoin Core Test Suite"; testing time: 31169us

  *** No errors detected
  ```
  </p>/</details>

  - change this line in the code to break the feature:
  ```diff
  --- a/src/netaddress.cpp
  +++ b/src/netaddress.cpp
  @@ -716,7 +716,7 @@ bool CService::GetSockAddr(struct sockaddr* paddr, socklen_t *addrlen) const
           memset(paddrin6, 0, *addrlen);
           if (!GetIn6Addr(&paddrin6->sin6_addr))
               return false;
  -        paddrin6->sin6_scope_id = m_scope_id;
  +        // paddrin6->sin6_scope_id = m_scope_id;
  ```

  - rebuild, e.g. `make`

  - run the test: `test/test_bitcoin -t net_tests/cnetaddr_basic -l all`, verify that the added tests break

  <details><summary><em>you should see test output with a few failed tests like this</em></summary><p>

  ```
  Running 1 test case...
  Entering test module "Bitcoin Core Test Suite"
  test/net_tests.cpp(85): Entering test suite "net_tests"
  test/net_tests.cpp(204): Entering test case "cnetaddr_basic"
  test/net_tests.cpp(210): info: check LookupHost("0.0.0.0", addr, false) has passed
  test/net_tests.cpp(211): info: check !addr.IsValid() has passed
  test/net_tests.cpp(212): info: check addr.IsIPv4() has passed
  test/net_tests.cpp(214): info: check addr.IsBindAny() has passed
  test/net_tests.cpp(215): info: check addr.ToString() == "0.0.0.0" has passed
  test/net_tests.cpp(218): info: check LookupHost("255.255.255.255", addr, false) has passed
  test/net_tests.cpp(219): info: check !addr.IsValid() has passed
  test/net_tests.cpp(220): info: check addr.IsIPv4() has passed
  test/net_tests.cpp(222): info: check !addr.IsBindAny() has passed
  test/net_tests.cpp(223): info: check addr.ToString() == "255.255.255.255" has passed
  test/net_tests.cpp(226): info: check LookupHost("12.34.56.78", addr, false) has passed
  test/net_tests.cpp(227): info: check addr.IsValid() has passed
  test/net_tests.cpp(228): info: check addr.IsIPv4() has passed
  test/net_tests.cpp(230): info: check !addr.IsBindAny() has passed
  test/net_tests.cpp(231): info: check addr.ToString() == "12.34.56.78" has passed
  test/net_tests.cpp(234): info: check LookupHost("::", addr, false) has passed
  test/net_tests.cpp(235): info: check !addr.IsValid() has passed
  test/net_tests.cpp(236): info: check addr.IsIPv6() has passed
  test/net_tests.cpp(238): info: check addr.IsBindAny() has passed
  test/net_tests.cpp(239): info: check addr.ToString() == "::" has passed
  test/net_tests.cpp(242): info: check LookupHost(ipv6_addr, addr, false) has passed
  test/net_tests.cpp(243): info: check addr.IsValid() has passed
  test/net_tests.cpp(244): info: check addr.IsIPv6() has passed
  test/net_tests.cpp(246): info: check !addr.IsBindAny() has passed
  test/net_tests.cpp(247): info: check addr.ToString() == ipv6_addr has passed
  test/net_tests.cpp(254): info: check LookupHost(scoped_addr, addr, false) has passed
  test/net_tests.cpp(255): info: check addr.IsValid() has passed
  test/net_tests.cpp(256): info: check addr.IsIPv6() has passed
  test/net_tests.cpp(257): info: check !addr.IsBindAny() has passed
  test/net_tests.cpp(258): error: in "net_tests/cnetaddr_basic": check addr.ToString() == scoped_addr has failed [1122:3344:5566:7788:9900:aabb:ccdd:eeff != 1122:3344:5566:7788:9900:aabb:ccdd:eeff%1]
  test/net_tests.cpp(254): info: check LookupHost(scoped_addr, addr, false) has passed
  test/net_tests.cpp(255): info: check addr.IsValid() has passed
  test/net_tests.cpp(256): info: check addr.IsIPv6() has passed
  test/net_tests.cpp(257): info: check !addr.IsBindAny() has passed
  test/net_tests.cpp(258): error: in "net_tests/cnetaddr_basic": check addr.ToString() == scoped_addr has failed [1122:3344:5566:7788:9900:aabb:ccdd:eeff != 1122:3344:5566:7788:9900:aabb:ccdd:eeff%21]
  test/net_tests.cpp(254): info: check LookupHost(scoped_addr, addr, false) has passed
  test/net_tests.cpp(255): info: check addr.IsValid() has passed
  test/net_tests.cpp(256): info: check addr.IsIPv6() has passed
  test/net_tests.cpp(257): info: check !addr.IsBindAny() has passed
  test/net_tests.cpp(258): error: in "net_tests/cnetaddr_basic": check addr.ToString() == scoped_addr has failed [1122:3344:5566:7788:9900:aabb:ccdd:eeff != 1122:3344:5566:7788:9900:aabb:ccdd:eeff%19]
  test/net_tests.cpp(254): info: check LookupHost(scoped_addr, addr, false) has passed
  test/net_tests.cpp(255): info: check addr.IsValid() has passed
  test/net_tests.cpp(256): info: check addr.IsIPv6() has passed
  test/net_tests.cpp(257): info: check !addr.IsBindAny() has passed
  test/net_tests.cpp(258): error: in "net_tests/cnetaddr_basic": check addr.ToString() == scoped_addr has failed [1122:3344:5566:7788:9900:aabb:ccdd:eeff != 1122:3344:5566:7788:9900:aabb:ccdd:eeff%3]
  test/net_tests.cpp(263): info: check !LookupHost(ipv6_addr + "%" + zone_id, addr, false) has passed
  test/net_tests.cpp(263): info: check !LookupHost(ipv6_addr + "%" + zone_id, addr, false) has passed
  test/net_tests.cpp(263): info: check !LookupHost(ipv6_addr + "%" + zone_id, addr, false) has passed
  test/net_tests.cpp(266): info: check LookupHost(ipv6_addr + "%0", addr, false) has passed
  test/net_tests.cpp(267): info: check addr.IsValid() has passed
  test/net_tests.cpp(268): info: check addr.IsIPv6() has passed
  test/net_tests.cpp(269): info: check !addr.IsBindAny() has passed
  test/net_tests.cpp(270): info: check addr.ToString() == ipv6_addr has passed
  test/net_tests.cpp(274): info: check addr.IsValid() has passed
  test/net_tests.cpp(275): info: check addr.IsTor() has passed
  test/net_tests.cpp(277): info: check !addr.IsBindAny() has passed
  test/net_tests.cpp(278): info: check addr.ToString() == "6hzph5hv6337r6p2.onion" has passed
  test/net_tests.cpp(282): info: check !addr.IsValid() has passed
  test/net_tests.cpp(283): info: check addr.IsInternal() has passed
  test/net_tests.cpp(285): info: check !addr.IsBindAny() has passed
  test/net_tests.cpp(286): info: check addr.ToString() == "esffpvrt3wpeaygy.internal" has passed
  test/net_tests.cpp(204): Leaving test case "cnetaddr_basic"; testing time: 32316us
  test/net_tests.cpp(85): Leaving test suite "net_tests"; testing time: 32354us
  Leaving test module "Bitcoin Core Test Suite"; testing time: 32522us

  *** 4 failures are detected in the test module "Bitcoin Core Test Suite"
  ```
  </p></details>

  - leave your review here

ACKs for top commit:
  laanwj:
    ACK f36887fa47

Tree-SHA512: 8e77e512db130642be7d3a910735ca803a2afdb5a704f713f163f8b5a80be3077a2be6f0a3ca43d299731dcd2545ac35571f8c74e5250a72a48233c26f9a3ab5
This commit is contained in:
Wladimir J. van der Laan 2020-10-02 17:06:55 +02:00
commit d9935222d5
No known key found for this signature in database
GPG Key ID: 1E4AED62986CD25D
3 changed files with 27 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/netaddress.cpp
View File

@ -281,7 +281,7 @@ CNetAddr::CNetAddr(const struct in_addr& ipv4Addr)
CNetAddr::CNetAddr(const struct in6_addr& ipv6Addr, const uint32_t scope)
{
SetLegacyIPv6(Span<const uint8_t>(reinterpret_cast<const uint8_t*>(&ipv6Addr), sizeof(ipv6Addr)));
scopeId = scope;
m_scope_id = scope;
}
bool CNetAddr::IsBindAny() const
@ -918,7 +918,7 @@ bool CService::GetSockAddr(struct sockaddr* paddr, socklen_t *addrlen) const
memset(paddrin6, 0, *addrlen);
if (!GetIn6Addr(&paddrin6->sin6_addr))
return false;
paddrin6->sin6_scope_id = scopeId;
paddrin6->sin6_scope_id = m_scope_id;
paddrin6->sin6_family = AF_INET6;
paddrin6->sin6_port = htons(port);
return true;

8
src/netaddress.h
View File

@ -130,7 +130,11 @@ class CNetAddr
*/
Network m_net{NET_IPV6};
uint32_t scopeId{0}; // for scoped/link-local ipv6 addresses
/**
* Scope id if scoped/link-local IPV6 address.
* See https://tools.ietf.org/html/rfc4007
*/
uint32_t m_scope_id{0};
public:
CNetAddr();
@ -388,7 +392,7 @@ class CNetAddr
"Address too long: %u > %u", address_size, MAX_ADDRV2_SIZE));
}
scopeId = 0;
m_scope_id = 0;
if (SetNetFromBIP155Network(bip155_net, address_size)) {
m_addr.resize(address_size);

19
src/test/net_tests.cpp
View File

@ -246,6 +246,25 @@ BOOST_AUTO_TEST_CASE(cnetaddr_basic)
BOOST_CHECK(!addr.IsBindAny());
BOOST_CHECK_EQUAL(addr.ToString(), "1122:3344:5566:7788:9900:aabb:ccdd:eeff");
// IPv6, scoped/link-local. See https://tools.ietf.org/html/rfc4007
// We support non-negative decimal integers (uint32_t) as zone id indices.
// Test with a fairly-high value, e.g. 32, to avoid locally reserved ids.
const std::string link_local{"fe80::1"};
const std::string scoped_addr{link_local + "%32"};
BOOST_REQUIRE(LookupHost(scoped_addr, addr, false));
BOOST_REQUIRE(addr.IsValid());
BOOST_REQUIRE(addr.IsIPv6());
BOOST_CHECK(!addr.IsBindAny());
const std::string addr_str{addr.ToString()};
BOOST_CHECK(addr_str == scoped_addr || addr_str == "fe80:0:0:0:0:0:0:1");
// The fallback case "fe80:0:0:0:0:0:0:1" is needed for macOS 10.14/10.15 and (probably) later.
// Test that the delimiter "%" and default zone id of 0 can be omitted for the default scope.
BOOST_REQUIRE(LookupHost(link_local + "%0", addr, false));
BOOST_REQUIRE(addr.IsValid());
BOOST_REQUIRE(addr.IsIPv6());
BOOST_CHECK(!addr.IsBindAny());
BOOST_CHECK_EQUAL(addr.ToString(), link_local);
// TORv2
BOOST_REQUIRE(addr.SetSpecial("6hzph5hv6337r6p2.onion"));
BOOST_REQUIRE(addr.IsValid());