Skip to content

Commit

Permalink
test: fix creation of std::string objects with \0s
Browse files Browse the repository at this point in the history
Summary:
A string literal `"abc"` contains a terminating `\0`, so that is 4
bytes. There is no need to write `"abc\0"` unless two terminating
`\0`s are necessary.

`std::string` objects do not internally contain a terminating `\0`, so
`std::string("abc")` creates a string with size 3 and is the same as
`std::string("abc", 3)`.

In `"\01"` the `01` part is interpreted as one number (1) and that is
the same as `"\1"` which is a string like `{1, 0}` whereas `"\0z"` is a
string like `{0, 'z', 0}`. To create a string like `{0, '1', 0}` one
must use `"\0" "1"`.

Adjust the tests accordingly.

This is a backport of [[bitcoin/bitcoin#20000 | core#20000]]

Test Plan: `ninja check`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D10808
  • Loading branch information
vasild authored and PiRK committed Jan 11, 2022
1 parent 734651c commit 952efc1
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 54 deletions.
23 changes: 15 additions & 8 deletions src/test/base32_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
#include <test/util/setup_common.h>

#include <boost/test/unit_test.hpp>
#include <string>

using namespace std::literals;

BOOST_FIXTURE_TEST_SUITE(base32_tests, BasicTestingSetup)

Expand All @@ -29,14 +32,18 @@ BOOST_AUTO_TEST_CASE(base32_testvectors) {

// Decoding strings with embedded NUL characters should fail
bool failure;
(void)DecodeBase32(std::string("invalid", 7), &failure);
BOOST_CHECK_EQUAL(failure, true);
(void)DecodeBase32(std::string("AWSX3VPP", 8), &failure);
BOOST_CHECK_EQUAL(failure, false);
(void)DecodeBase32(std::string("AWSX3VPP\0invalid", 16), &failure);
BOOST_CHECK_EQUAL(failure, true);
(void)DecodeBase32(std::string("AWSX3VPPinvalid", 15), &failure);
BOOST_CHECK_EQUAL(failure, true);
// correct size, invalid due to \0
(void)DecodeBase32("invalid\0"s, &failure);
BOOST_CHECK(failure);
// valid
(void)DecodeBase32("AWSX3VPP"s, &failure);
BOOST_CHECK(!failure);
// correct size, invalid due to \0
(void)DecodeBase32("AWSX3VPP\0invalid"s, &failure);
BOOST_CHECK(failure);
// invalid size
(void)DecodeBase32("AWSX3VPPinvalid"s, &failure);
BOOST_CHECK(failure);
}

BOOST_AUTO_TEST_SUITE_END()
29 changes: 15 additions & 14 deletions src/test/base58_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
#include <univalue.h>

#include <boost/test/unit_test.hpp>
#include <string>

using namespace std::literals;

BOOST_FIXTURE_TEST_SUITE(base58_tests, BasicTestingSetup)

Expand Down Expand Up @@ -61,14 +64,14 @@ BOOST_AUTO_TEST_CASE(base58_DecodeBase58) {
strTest);
}

BOOST_CHECK(!DecodeBase58("invalid", result, 100));
BOOST_CHECK(!DecodeBase58(std::string("invalid"), result, 100));
BOOST_CHECK(!DecodeBase58(std::string("\0invalid", 8), result, 100));
BOOST_CHECK(!DecodeBase58("invalid"s, result, 100));
BOOST_CHECK(!DecodeBase58("invalid\0"s, result, 100));
BOOST_CHECK(!DecodeBase58("\0invalid"s, result, 100));

BOOST_CHECK(DecodeBase58(std::string("good", 4), result, 100));
BOOST_CHECK(!DecodeBase58(std::string("bad0IOl", 7), result, 100));
BOOST_CHECK(!DecodeBase58(std::string("goodbad0IOl", 11), result, 100));
BOOST_CHECK(!DecodeBase58(std::string("good\0bad0IOl", 12), result, 100));
BOOST_CHECK(DecodeBase58("good"s, result, 100));
BOOST_CHECK(!DecodeBase58("bad0IOl"s, result, 100));
BOOST_CHECK(!DecodeBase58("goodbad0IOl"s, result, 100));
BOOST_CHECK(!DecodeBase58("good\0bad0IOl"s, result, 100));

// check that DecodeBase58 skips whitespace, but still fails with unexpected
// non-whitespace at the end.
Expand All @@ -78,14 +81,12 @@ BOOST_AUTO_TEST_CASE(base58_DecodeBase58) {
BOOST_CHECK_EQUAL_COLLECTIONS(result.begin(), result.end(),
expected.begin(), expected.end());

BOOST_CHECK(DecodeBase58Check(std::string("3vQB7B6MrGQZaxCuFg4oh", 21),
result, 100));
BOOST_CHECK(!DecodeBase58Check(std::string("3vQB7B6MrGQZaxCuFg4oi", 21),
result, 100));
BOOST_CHECK(!DecodeBase58Check(std::string("3vQB7B6MrGQZaxCuFg4oh0IOl", 25),
BOOST_CHECK(DecodeBase58Check("3vQB7B6MrGQZaxCuFg4oh"s, result, 100));
BOOST_CHECK(!DecodeBase58Check("3vQB7B6MrGQZaxCuFg4oi"s, result, 100));
BOOST_CHECK(!DecodeBase58Check("3vQB7B6MrGQZaxCuFg4oh0IOl"s, result, 100));
BOOST_CHECK(!DecodeBase58Check("3vQB7B6MrGQZaxCuFg4oh\0"
"0IOl"s,
result, 100));
BOOST_CHECK(!DecodeBase58Check(
std::string("3vQB7B6MrGQZaxCuFg4oh\00IOl", 26), result, 100));
}

BOOST_AUTO_TEST_CASE(base58_random_encode_decode) {
Expand Down
19 changes: 11 additions & 8 deletions src/test/base64_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
#include <test/util/setup_common.h>

#include <boost/test/unit_test.hpp>
#include <string>

using namespace std::literals;

BOOST_FIXTURE_TEST_SUITE(base64_tests, BasicTestingSetup)

Expand All @@ -24,14 +27,14 @@ BOOST_AUTO_TEST_CASE(base64_testvectors) {

// Decoding strings with embedded NUL characters should fail
bool failure;
(void)DecodeBase64(std::string("invalid", 7), &failure);
BOOST_CHECK_EQUAL(failure, true);
(void)DecodeBase64(std::string("nQB/pZw=", 8), &failure);
BOOST_CHECK_EQUAL(failure, false);
(void)DecodeBase64(std::string("nQB/pZw=\0invalid", 16), &failure);
BOOST_CHECK_EQUAL(failure, true);
(void)DecodeBase64(std::string("nQB/pZw=invalid", 15), &failure);
BOOST_CHECK_EQUAL(failure, true);
(void)DecodeBase64("invalid\0"s, &failure);
BOOST_CHECK(failure);
(void)DecodeBase64("nQB/pZw="s, &failure);
BOOST_CHECK(!failure);
(void)DecodeBase64("nQB/pZw=\0invalid"s, &failure);
BOOST_CHECK(failure);
(void)DecodeBase64("nQB/pZw=invalid\0"s, &failure);
BOOST_CHECK(failure);
}

BOOST_AUTO_TEST_SUITE_END()
7 changes: 4 additions & 3 deletions src/test/net_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
#include <numeric>
#include <string>

using namespace std::literals;

class CAddrManSerializationMock : public CAddrMan {
public:
virtual void Serialize(CDataStream &s) const = 0;
Expand Down Expand Up @@ -112,9 +114,8 @@ BOOST_AUTO_TEST_CASE(caddrdb_read) {
BOOST_CHECK(Lookup("250.7.1.1", addr1, 8333, false));
BOOST_CHECK(Lookup("250.7.2.2", addr2, 9999, false));
BOOST_CHECK(Lookup("250.7.3.3", addr3, 9999, false));
BOOST_CHECK(Lookup(std::string("250.7.3.3", 9), addr3, 9999, false));
BOOST_CHECK(
!Lookup(std::string("250.7.3.3\0example.com", 21), addr3, 9999, false));
BOOST_CHECK(Lookup("250.7.3.3"s, addr3, 9999, false));
BOOST_CHECK(!Lookup("250.7.3.3\0example.com"s, addr3, 9999, false));

// Add three addresses to new table.
CService source;
Expand Down
32 changes: 14 additions & 18 deletions src/test/netbase_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

#include <string>

using namespace std::literals;

BOOST_FIXTURE_TEST_SUITE(netbase_tests, BasicTestingSetup)

static CNetAddr ResolveIP(const std::string &ip) {
Expand Down Expand Up @@ -510,26 +512,20 @@ BOOST_AUTO_TEST_CASE(netpermissions_test) {
BOOST_AUTO_TEST_CASE(
netbase_dont_resolve_strings_with_embedded_nul_characters) {
CNetAddr addr;
BOOST_CHECK(LookupHost(std::string("127.0.0.1", 9), addr, false));
BOOST_CHECK(!LookupHost(std::string("127.0.0.1\0", 10), addr, false));
BOOST_CHECK(
!LookupHost(std::string("127.0.0.1\0example.com", 21), addr, false));
BOOST_CHECK(
!LookupHost(std::string("127.0.0.1\0example.com\0", 22), addr, false));
BOOST_CHECK(LookupHost("127.0.0.1"s, addr, false));
BOOST_CHECK(!LookupHost("127.0.0.1\0"s, addr, false));
BOOST_CHECK(!LookupHost("127.0.0.1\0example.com"s, addr, false));
BOOST_CHECK(!LookupHost("127.0.0.1\0example.com\0"s, addr, false));
CSubNet ret;
BOOST_CHECK(LookupSubNet(std::string("1.2.3.0/24", 10), ret));
BOOST_CHECK(!LookupSubNet(std::string("1.2.3.0/24\0", 11), ret));
BOOST_CHECK(!LookupSubNet(std::string("1.2.3.0/24\0example.com", 22), ret));
BOOST_CHECK(
!LookupSubNet(std::string("1.2.3.0/24\0example.com\0", 23), ret));
BOOST_CHECK(LookupSubNet("1.2.3.0/24"s, ret));
BOOST_CHECK(!LookupSubNet("1.2.3.0/24\0"s, ret));
BOOST_CHECK(!LookupSubNet("1.2.3.0/24\0example.com"s, ret));
BOOST_CHECK(!LookupSubNet("1.2.3.0/24\0example.com\0"s, ret));
// We only do subnetting for IPv4 and IPv6
BOOST_CHECK(!LookupSubNet(std::string("5wyqrzbvrdsumnok.onion", 22), ret));
BOOST_CHECK(
!LookupSubNet(std::string("5wyqrzbvrdsumnok.onion\0", 23), ret));
BOOST_CHECK(!LookupSubNet(
std::string("5wyqrzbvrdsumnok.onion\0example.com", 34), ret));
BOOST_CHECK(!LookupSubNet(
std::string("5wyqrzbvrdsumnok.onion\0example.com\0", 35), ret));
BOOST_CHECK(!LookupSubNet("5wyqrzbvrdsumnok.onion"s, ret));
BOOST_CHECK(!LookupSubNet("5wyqrzbvrdsumnok.onion\0"s, ret));
BOOST_CHECK(!LookupSubNet("5wyqrzbvrdsumnok.onion\0example.com"s, ret));
BOOST_CHECK(!LookupSubNet("5wyqrzbvrdsumnok.onion\0example.com\0"s, ret));
}

// Since CNetAddr (un)ser is tested separately in net_tests.cpp here we only
Expand Down
11 changes: 8 additions & 3 deletions src/test/util_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

#include <array>
#include <cstdint>
#include <cstring>
#include <optional>
#include <utility>
#ifndef WIN32
Expand All @@ -39,6 +40,8 @@
#include <thread>
#include <vector>

using namespace std::literals;

/* defined in logging.cpp */
namespace BCLog {
std::string LogEscapeMessage(const std::string &str);
Expand Down Expand Up @@ -1492,9 +1495,11 @@ BOOST_AUTO_TEST_CASE(util_ParseMoney) {
BOOST_CHECK(!ParseMoney("-1", ret));

// Parsing strings with embedded NUL characters should fail
BOOST_CHECK(!ParseMoney(std::string("\0-1", 3), ret));
BOOST_CHECK(!ParseMoney(std::string("\01", 2), ret));
BOOST_CHECK(!ParseMoney(std::string("1\0", 2), ret));
BOOST_CHECK(!ParseMoney("\0-1"s, ret));
BOOST_CHECK(!ParseMoney("\0"
"1"s,
ret));
BOOST_CHECK(!ParseMoney("1\0"s, ret));
}

BOOST_AUTO_TEST_CASE(util_IsHex) {
Expand Down

0 comments on commit 952efc1

Please sign in to comment.