Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

Commit

Permalink
fix: weak constraint of ipv4 and incorrect code logic in hostname_fro…
Browse files Browse the repository at this point in the history 
…m_ip (#406)
  • Loading branch information
@Smityz
Smityz authored and neverchanje committed Mar 29, 2020
1 parent fbe41c5 commit 6fbf040
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 56 deletions.
41 changes: 22 additions & 19 deletions include/dsn/tool-api/rpc_address.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,11 @@
*/
#pragma once

#include <unordered_map>
#include <unordered_set>
#include <vector>
#include <cstring>
#include <string>
#include <cstdlib>
#include <cstdint>

#include <arpa/inet.h>
#include <thrift/protocol/TProtocol.h>
#include <dsn/utility/string_conv.h>

typedef enum dsn_host_type_t {
HOST_TYPE_INVALID = 0,
Expand Down Expand Up @@ -105,25 +102,31 @@ class rpc_address

std::string to_std_string() const { return std::string(to_string()); }

// This function is used for validating the format of ipv4 like "192.168.0.1:12345"
// Due to historical legacy, we also consider "localhost:8080" is in a valid format
// IP address without port like "127.0.0.1" is invalid here
bool from_string_ipv4(const char *s)
{
set_invalid();
std::string str = std::string(s);
auto pos = str.find_last_of(':');
if (pos == std::string::npos)
std::string ip_port = std::string(s);
auto pos = ip_port.find_last_of(':');
if (pos == std::string::npos) {
return false;
}
std::string ip = ip_port.substr(0, pos);
std::string port = ip_port.substr(pos + 1);
// check port
unsigned int port_num;
if (!dsn::internal::buf2unsigned(port, port_num) || port_num > UINT16_MAX) {
return false;
else {
auto host = str.substr(0, pos);
auto port_str = str.substr(pos + 1);
char *p = nullptr;
long port = ::strtol(port_str.data(), &p, 10);
if (*p != 0) // bad string
return false;
if (port <= 0 || port > UINT16_MAX) // out of range
return false;
assign_ipv4(host.c_str(), (uint16_t)port);
}
// check localhost & IP
uint32_t ip_addr;
if (ip == "localhost" || inet_pton(AF_INET, ip.c_str(), &ip_addr)) {
assign_ipv4(ip.c_str(), (uint16_t)port_num);
return true;
}
return false;
}

uint64_t &value() { return _addr.value; }
Expand Down
4 changes: 2 additions & 2 deletions src/core/core/utils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,5 +190,5 @@ bool list_hostname_from_ip_port(const char *ip_port_list, std::string *hostname_
*hostname_result_list = result.str();
return all_ok;
}
}
}
} // namespace utils
} // namespace dsn
98 changes: 63 additions & 35 deletions src/core/tests/hostname_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,76 +10,104 @@
namespace dsn {
namespace replication {

TEST(ip_to_hostname, ipv4_validate)
{
rpc_address rpc_test_ipv4;
struct ip_test
{
std::string ip;
bool result;
} tests[] = {{"127.0.0.1:8080", true},
{"172.16.254.1:1234", true},
{"172.16.254.1:222222", false},
{"172.16.254.1", false},
{"2222,123,33,1:8080", false},
{"123.456.789.1:8080", false},
{"001.223.110.002:8080", false},
{"172.16.254.1.8080", false},
{"172.16.254.1:8080.", false},
{"127.0.0.11:123!", false},
{"127.0.0.11:123", true},
{"localhost:34601", true},
{"localhost:3460100022212312312213", false},
{"localhost:-12", false},
{"localhost:1@2", false}};

for (auto test : tests) {
ASSERT_EQ(rpc_test_ipv4.from_string_ipv4(test.ip.c_str()), test.result);
}
}

TEST(ip_to_hostname, localhost)
{
std::string hostname_result;

const std::string success_ip = "127.0.0.1";
const std::string valid_ip = "127.0.0.1";
const std::string expected_hostname = "localhost";

const std::string success_ip_port = "127.0.0.1:23010";
const std::string valid_ip_port = "127.0.0.1:23010";
const std::string expected_hostname_port = "localhost:23010";

const std::string failed_ip = "123.456.789.111";
const std::string failed_ip_port = "123.456.789.111:23010";

const std::string success_ip_list = "127.0.0.1,127.0.0.1,127.0.0.1";
const std::string valid_ip_list = "127.0.0.1,127.0.0.1,127.0.0.1";
const std::string expected_hostname_list = "localhost,localhost,localhost";

const std::string success_ip_port_list = "127.0.0.1:8080,127.0.0.1:8080,127.0.0.1:8080";
const std::string valid_ip_port_list = "127.0.0.1:8080,127.0.0.1:8080,127.0.0.1:8080";
const std::string expected_hostname_port_list = "localhost:8080,localhost:8080,localhost:8080";

rpc_address rpc_example_success, rpc_example_failed;
rpc_example_success.assign_ipv4(success_ip.c_str(), 23010);
rpc_example_failed.assign_ipv4(failed_ip.c_str(), 23010);
rpc_address rpc_example_valid;
rpc_example_valid.assign_ipv4(valid_ip.c_str(), 23010);

// static bool hostname(const rpc_address &address,std::string *hostname_result);
ASSERT_TRUE(dsn::utils::hostname(rpc_example_success, &hostname_result));
ASSERT_STREQ(expected_hostname_port.c_str(), hostname_result.c_str());

ASSERT_FALSE(dsn::utils::hostname(rpc_example_failed, &hostname_result));
ASSERT_TRUE(dsn::utils::hostname(rpc_example_valid, &hostname_result));
ASSERT_EQ(expected_hostname_port, hostname_result);

// static bool hostname_from_ip(uint32_t ip, std::string* hostname_result);
ASSERT_TRUE(dsn::utils::hostname_from_ip(htonl(rpc_example_success.ip()), &hostname_result));
ASSERT_STREQ(expected_hostname.c_str(), hostname_result.c_str());

ASSERT_FALSE(dsn::utils::hostname_from_ip(htonl(rpc_example_failed.ip()), &hostname_result));
ASSERT_TRUE(dsn::utils::hostname_from_ip(htonl(rpc_example_valid.ip()), &hostname_result));
ASSERT_EQ(expected_hostname, hostname_result);

// static bool hostname_from_ip(const char *ip,std::string *hostname_result);
ASSERT_TRUE(dsn::utils::hostname_from_ip(success_ip.c_str(), &hostname_result));
ASSERT_STREQ(expected_hostname.c_str(), hostname_result.c_str());

ASSERT_FALSE(dsn::utils::hostname_from_ip(failed_ip.c_str(), &hostname_result));
ASSERT_STREQ(failed_ip.c_str(), hostname_result.c_str());
ASSERT_TRUE(dsn::utils::hostname_from_ip(valid_ip.c_str(), &hostname_result));
ASSERT_EQ(expected_hostname, hostname_result);

// static bool hostname_from_ip_port(const char *ip_port,std::string *hostname_result);
ASSERT_TRUE(dsn::utils::hostname_from_ip_port(success_ip_port.c_str(), &hostname_result));
ASSERT_STREQ(expected_hostname_port.c_str(), hostname_result.c_str());

ASSERT_FALSE(dsn::utils::hostname_from_ip_port(failed_ip_port.c_str(), &hostname_result));
ASSERT_STREQ(failed_ip_port.c_str(), hostname_result.c_str());
ASSERT_TRUE(dsn::utils::hostname_from_ip_port(valid_ip_port.c_str(), &hostname_result));
ASSERT_EQ(expected_hostname_port, hostname_result);

// static bool list_hostname_from_ip(const char *ip_port_list,std::string
// *hostname_result_list);
ASSERT_TRUE(dsn::utils::list_hostname_from_ip(success_ip_list.c_str(), &hostname_result));
ASSERT_STREQ(expected_hostname_list.c_str(), hostname_result.c_str());
ASSERT_TRUE(dsn::utils::list_hostname_from_ip(valid_ip_list.c_str(), &hostname_result));
ASSERT_EQ(expected_hostname_list, hostname_result);

ASSERT_FALSE(dsn::utils::list_hostname_from_ip("127.0.0.1,127.0.0.23323,111127.0.0.3",
&hostname_result));
ASSERT_STREQ("localhost,127.0.0.23323,111127.0.0.3", hostname_result.c_str());
ASSERT_EQ("localhost,127.0.0.23323,111127.0.0.3", hostname_result);

ASSERT_FALSE(dsn::utils::list_hostname_from_ip("123.456.789.111,127.0.0.1", &hostname_result));
ASSERT_STREQ("123.456.789.111,localhost", hostname_result.c_str());
ASSERT_EQ("123.456.789.111,localhost", hostname_result);

// static bool list_hostname_from_ip_port(const char *ip_port_list,std::string
// *hostname_result_list);
ASSERT_TRUE(
dsn::utils::list_hostname_from_ip_port(success_ip_port_list.c_str(), &hostname_result));
ASSERT_STREQ(expected_hostname_port_list.c_str(), hostname_result.c_str());
dsn::utils::list_hostname_from_ip_port(valid_ip_port_list.c_str(), &hostname_result));
ASSERT_EQ(expected_hostname_port_list, hostname_result);

ASSERT_FALSE(dsn::utils::list_hostname_from_ip_port(
"127.0.3333.1:23456,1127.0.0.2:22233,127.0.0.1:8080", &hostname_result));
ASSERT_STREQ("127.0.3333.1:23456,1127.0.0.2:22233,localhost:8080", hostname_result.c_str());
ASSERT_EQ("127.0.3333.1:23456,1127.0.0.2:22233,localhost:8080", hostname_result);
}

TEST(ip_to_hostname, invalid_ip)
{

std::string hostname_result;
const std::string invalid_ip = "123.456.789.111";
const std::string invalid_ip_port = "123.456.789.111:23010";

ASSERT_FALSE(dsn::utils::hostname_from_ip(invalid_ip.c_str(), &hostname_result));
ASSERT_EQ(invalid_ip, hostname_result);

ASSERT_FALSE(dsn::utils::hostname_from_ip_port(invalid_ip_port.c_str(), &hostname_result));
ASSERT_EQ(invalid_ip_port, hostname_result);
}

} // namespace replication
Expand Down

0 comments on commit 6fbf040

Please sign in to comment.