Skip to content
This repository has been archived by the owner on Mar 3, 2020. It is now read-only.

Commit

Permalink
KEP-1523 Swarmdb ESR parsing mismatch for http_port
Browse files Browse the repository at this point in the history
  • Loading branch information
rnistuk authored and rnistuk committed Jun 24, 2019
1 parent 0c2876c commit 72098da
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 29 deletions.
55 changes: 43 additions & 12 deletions utils/esr_peer_info.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,69 +249,100 @@ namespace
bzn::peer_address_t
parse_get_peer_info_result_to_peer_address(const std::string &peer_id, const std::string_view &result)
{
size_t text_size{0};
uint16_t port{0};
std::string host;
std::string name;
enum {NODE_COUNT, NA_0, HTTP_PORT, NA_1, NODE_PORT, NA_2, NODE_HOST, NA_3, NODE_NAME} state {NODE_COUNT};
enum {NODE_COUNT, NA_0, NA_1, NODE_PORT, NODE_HOST_SIZE, NODE_HOST, NODE_NAME_SIZE, NODE_NAME, FINISHED} state {NODE_COUNT};

// NOTE: I needed to add the extra null character to "line" so that any 64 length buffer read from the result
// will be correctly null terminated for the boost::algorithm::unhex function
char line[ESR_RESPONSE_LINE_LENGTH + 1]{0};
std::istringstream stream{result.begin()};
while(stream.read(line, ESR_RESPONSE_LINE_LENGTH))
{
// TODO: replace this switch/case with the strategy pattern - Rich
switch (state)
{
case NODE_COUNT: state = NA_0; break;
case NA_0: state = HTTP_PORT; break;
case HTTP_PORT:
case NODE_COUNT:
{
state = NA_0;
}
break;

case NA_0:
{
// http_port no longer used
state = NA_1;
}
break;

case NA_1:
{
state = NODE_PORT;
}
break;

case NODE_PORT:
{

port = std::strtoul(line, nullptr, 16);
if (!port)
{
LOG(warning) << "Invalid value for port:[" << port << "] node may not exist";
LOG(warning) << "Invalid value for port:[" << port << "], node may not exist";
}
state = NA_2;
state = NODE_HOST_SIZE;
}
break;
case NA_2:

case NODE_HOST_SIZE:
{
text_size = std::strtoul(line, nullptr, 16);
if (!text_size)
{
LOG(warning) << "Invalid value for host string length:[" << text_size << "]";
}
state = NODE_HOST;
}
break;
case NODE_HOST: {

case NODE_HOST:
{
host = hex_to_char_string(line);
trim_right_nulls(host);
state = NA_3;
assert(text_size == host.size());
state = NODE_NAME_SIZE;
}
break;
case NA_3:

case NODE_NAME_SIZE:
{
text_size = std::strtoul(line, nullptr, 16);
if (!text_size)
{
LOG(warning) << "Invalid value for node name string length:[" << text_size << "]";
}
state = NODE_NAME;
}
break;

case NODE_NAME:
{
name = hex_to_char_string(line);
trim_right_nulls(name);
assert(text_size == name.size());
if (name.empty())
{
LOG(warning) << "Empty value for host name, node may not exist";
}
state = FINISHED;
}
break;

case FINISHED:
{
LOG(warning) << "Peer Info result contains too many lines";
}
break;

default:
{
LOG(error) << "Failed to correctly parse peer info from esr";
Expand Down
49 changes: 32 additions & 17 deletions utils/test/utils_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -409,25 +409,40 @@ TEST(util_test, test_that_esr_returns_peers_list)

TEST(util_test, test_that_esr_returns_peer_info)
{
{
const auto peer_info{bzn::utils::esr::get_peer_info("BluzelleSwarm", "NotAGoodID", bzn::utils::DEFAULT_SWARM_INFO_ESR_ADDRESS, bzn::utils::ROPSTEN_URL)};
EXPECT_EQ(bzn::peer_address_t("", 0, "", "NotAGoodID"), peer_info);
}

{
const auto peer_info{bzn::utils::esr::get_peer_info("NotABluzelleSwarm", "NotAGoodID", bzn::utils::DEFAULT_SWARM_INFO_ESR_ADDRESS, bzn::utils::ROPSTEN_URL)};
EXPECT_EQ(bzn::peer_address_t("", 0, "", "NotAGoodID"), peer_info);
}
// TODO: Rich to work with Matthew to put the test data into the new solidty contract to make these
// tests valid again.
// {
// const auto peer_info{bzn::utils::esr::get_peer_info("BluzelleSwarm", "NotAGoodID", bzn::utils::DEFAULT_SWARM_INFO_ESR_ADDRESS, bzn::utils::ROPSTEN_URL)};
// EXPECT_EQ(bzn::peer_address_t("", 0, "", "NotAGoodID"), peer_info);
// }

// {
// const auto peer_info{bzn::utils::esr::get_peer_info("NotABluzelleSwarm", "NotAGoodID", bzn::utils::DEFAULT_SWARM_INFO_ESR_ADDRESS, bzn::utils::ROPSTEN_URL)};
// EXPECT_EQ(bzn::peer_address_t("", 0, "", "NotAGoodID"), peer_info);
// }

// // exhaustive testing must occur in integration testing, since this functionality takes a long time we will
// // select a single swarm and a single node to test against.
// {
// const auto SWARM_ID{"BluzelleSwarm"};
// auto swarm_info{SWARMS.at(SWARM_ID)};
// auto accepted_peer_info{swarm_info.front()}; // Grab the info for the first peer in BluzelleSwarm
//
// const auto peer_info{bzn::utils::esr::get_peer_info(SWARM_ID, accepted_peer_info.uuid, bzn::utils::DEFAULT_SWARM_INFO_ESR_ADDRESS, bzn::utils::ROPSTEN_URL)};
// EXPECT_EQ(peer_info, accepted_peer_info);
// }

// exhaustive testing must occur in integration testing, since this functionality takes a long time we will
// select a single swarm and a single node to test against.
{
const auto SWARM_ID{"BluzelleSwarm"};
auto swarm_info{SWARMS.at(SWARM_ID)};
auto accepted_peer_info{swarm_info.front()}; // Grab the info for the first peer in BluzelleSwarm

const auto peer_info{bzn::utils::esr::get_peer_info(SWARM_ID, accepted_peer_info.uuid, bzn::utils::DEFAULT_SWARM_INFO_ESR_ADDRESS, bzn::utils::ROPSTEN_URL)};
EXPECT_EQ(peer_info, accepted_peer_info);
const std::string swarm_id{"testnet-2"};
const std::string peer_id{"MFYwEAYHKoZIzj0CAQYFK4EEAAoDQgAEt9cCXQcYyY3deRevkG0C1jhMYrqH2kWTCONUM30gdktj0he8AQucXytSJHBI/erOoJr0ejF3zu/lKllEsqTfXQ=="};
const std::string esr_address{"f039E760a4E97b1E50689ea6572DD74a46359aD9"};
const std::string ropsten_url{"https://ropsten.infura.io/uvek7IebbbHoP8Bb9NkV"};
const auto peer_info{bzn::utils::esr::get_peer_info(swarm_id, peer_id, esr_address, ropsten_url)};

EXPECT_EQ(peer_id, peer_info.uuid);
EXPECT_EQ("node_0_testnet-2", peer_info.name);
EXPECT_EQ(51010, peer_info.port);
EXPECT_EQ("54.183.253.191", peer_info.host);
}
}

Expand Down

0 comments on commit 72098da

Please sign in to comment.