Skip to content

Commit

Permalink
Fix bug #5547
Browse files Browse the repository at this point in the history
  • Loading branch information
NathanFreeman committed Nov 24, 2024
1 parent 0f8f2be commit 9105da7
Show file tree
Hide file tree
Showing 12 changed files with 107 additions and 81 deletions.
16 changes: 1 addition & 15 deletions core-tests/src/network/dns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,23 +74,9 @@ TEST(dns, cancel) {
}

TEST(dns, getaddrinfo) {
char buf[1024] = {};
swoole::network::GetaddrinfoRequest req = {};
req.hostname = "www.baidu.com";
req.family = AF_INET;
req.socktype = SOCK_STREAM;
req.protocol = 0;
req.service = nullptr;
req.result = buf;
swoole::GetaddrinfoRequest req("www.baidu.com", AF_INET, SOCK_STREAM, 0, "");
ASSERT_EQ(swoole::network::getaddrinfo(&req), 0);
ASSERT_GT(req.count, 0);

vector<string> ip_list;
req.parse_result(ip_list);

for (auto &ip : ip_list) {
ASSERT_TRUE(swoole::network::Address::verify_ip(AF_INET, ip));
}
}

TEST(dns, load_resolv_conf) {
Expand Down
36 changes: 31 additions & 5 deletions include/swoole_async.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,17 @@ enum AsyncFlag {
SW_AIO_EOF = 1u << 2,
};

struct AsyncRequest {
virtual ~AsyncRequest() = default;

Check warning on line 38 in include/swoole_async.h

View check run for this annotation

Codecov / codecov/patch

include/swoole_async.h#L38

Added line #L38 was not covered by tests
};

struct AsyncEvent {
size_t task_id;
uint8_t canceled;
/**
* input & output
*/
void *data;
std::shared_ptr<AsyncRequest> data;
/**
* output
*/
Expand All @@ -60,22 +64,44 @@ struct AsyncEvent {
}
};

struct GethostbynameRequest {
const char *name;
struct GethostbynameRequest : public AsyncRequest {
std::string name;
int family;
char *addr;
size_t addr_len;

GethostbynameRequest(const char *_name, int _family) : name(_name), family(_family) {
GethostbynameRequest(std::string _name, int _family) : name(std::move(_name)), family(_family) {
addr_len = _family == AF_INET6 ? INET6_ADDRSTRLEN : INET_ADDRSTRLEN;
addr = new char[addr_len];
}

~GethostbynameRequest() {
~GethostbynameRequest() override {
delete[] addr;
}
};

struct GetaddrinfoRequest : public AsyncRequest {
std::string hostname;
std::string service;
int family;
int socktype;
int protocol;
int error;
std::vector<struct sockaddr_in6> results;
int count;
void parse_result(std::vector<std::string> &retval);
GetaddrinfoRequest(std::string _hostname, int _family, int _socktype, int _protocol, std::string _service)
: hostname(std::move(_hostname)),
service(std::move(_service)) {
family =_family;
socktype =_socktype;
protocol =_protocol;
count = 0;
error = 0;

Check warning on line 100 in include/swoole_async.h

View check run for this annotation

Codecov / codecov/patch

include/swoole_async.h#L93-L100

Added lines #L93 - L100 were not covered by tests
}
~GetaddrinfoRequest() override = default;

Check warning on line 102 in include/swoole_async.h

View check run for this annotation

Codecov / codecov/patch

include/swoole_async.h#L102

Added line #L102 was not covered by tests
};

class AsyncThreads {
public:
bool schedule = false;
Expand Down
15 changes: 2 additions & 13 deletions include/swoole_socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,20 +64,9 @@ enum {
};

namespace swoole {
namespace network {
struct GetaddrinfoRequest;

struct GetaddrinfoRequest {
const char *hostname;
const char *service;
int family;
int socktype;
int protocol;
int error;
void *result;
int count;

void parse_result(std::vector<std::string> &retval);
};
namespace network {

struct SendfileTask {
off_t offset;
Expand Down
24 changes: 7 additions & 17 deletions src/coroutine/system.cc
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ ssize_t System::write_file(const char *file, char *buf, size_t length, bool lock

std::string gethostbyname_impl_with_async(const std::string &hostname, int domain, double timeout) {
AsyncEvent ev{};
GethostbynameRequest dns_request(hostname.c_str(), domain);
ev.data = &dns_request;
auto req = new GethostbynameRequest(hostname, domain);
ev.data = std::shared_ptr<AsyncRequest>(req);
ev.retval = 1;

coroutine::async(async::handler_gethostbyname, ev, timeout);
Expand All @@ -150,7 +150,7 @@ std::string gethostbyname_impl_with_async(const std::string &hostname, int domai
swoole_set_last_error(ev.error);
return "";
} else {
std::string addr(dns_request.addr);
std::string addr(req->addr);
return addr;
}
}
Expand Down Expand Up @@ -199,30 +199,20 @@ std::vector<std::string> System::getaddrinfo(
assert(family == AF_INET || family == AF_INET6);

AsyncEvent ev{};
network::GetaddrinfoRequest req{};

ev.data = &req;

struct sockaddr_in6 result_buffer[SW_DNS_HOST_BUFFER_SIZE];

req.hostname = hostname.c_str();
req.family = family;
req.socktype = socktype;
req.protocol = protocol;
req.service = service.empty() ? nullptr : service.c_str();
req.result = result_buffer;
auto req = new GetaddrinfoRequest(hostname, family, socktype, protocol, service);
ev.data = std::shared_ptr<AsyncRequest>(req);

coroutine::async(async::handler_getaddrinfo, ev, timeout);

std::vector<std::string> retval;

if (ev.retval == -1 || req.error != 0) {
if (ev.retval == -1 || req->error != 0) {
if (ev.error == SW_ERROR_AIO_TIMEOUT) {
ev.error = SW_ERROR_DNSLOOKUP_RESOLVE_TIMEOUT;
}
swoole_set_last_error(ev.error);
} else {
req.parse_result(retval);
req->parse_result(retval);
}

return retval;
Expand Down
14 changes: 4 additions & 10 deletions src/network/client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -615,14 +615,13 @@ static int Client_tcp_connect_async(Client *cli, const char *host, int port, dou

if (cli->wait_dns) {
AsyncEvent ev{};
auto dns_request = new GethostbynameRequest(cli->server_host, cli->_sock_domain);
ev.data = dns_request;
auto req = new GethostbynameRequest(cli->server_host, cli->_sock_domain);
ev.data = std::shared_ptr<AsyncRequest>(req);;
ev.object = cli;
ev.handler = async::handler_gethostbyname;
ev.callback = Client_onResolveCompleted;

if (swoole::async::dispatch(&ev) == nullptr) {
delete dns_request;
return SW_ERR;
} else {
return SW_OK;
Expand Down Expand Up @@ -1112,17 +1111,13 @@ static void Client_onTimeout(Timer *timer, TimerNode *tnode) {
}

static void Client_onResolveCompleted(AsyncEvent *event) {
auto dns_request = (GethostbynameRequest *) event->data;
if (event->canceled) {
delete dns_request;
return;
}
GethostbynameRequest *req = dynamic_cast<GethostbynameRequest *>(event->data.get());

Client *cli = (Client *) event->object;
cli->wait_dns = 0;

if (event->error == 0) {
Client_tcp_connect_async(cli, dns_request->addr, cli->server_port, cli->timeout, 1);
Client_tcp_connect_async(cli, req->addr, cli->server_port, cli->timeout, 1);
} else {
swoole_set_last_error(SW_ERROR_DNSLOOKUP_RESOLVE_FAILED);
cli->socket->removed = 1;
Expand All @@ -1131,7 +1126,6 @@ static void Client_onResolveCompleted(AsyncEvent *event) {
cli->onError(cli);
}
}
delete dns_request;
}

static int Client_onWrite(Reactor *reactor, Event *event) {
Expand Down
29 changes: 16 additions & 13 deletions src/network/dns.cc
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ std::vector<std::string> dns_lookup_impl_with_socket(const char *domain, int fam
temp = &packet[steps];
j = 0;
while (*temp != 0) {
if ((uchar)(*temp) == 0xc0) {
if ((uchar) (*temp) == 0xc0) {
++temp;
temp = &packet[(uint8_t) *temp];
} else {
Expand Down Expand Up @@ -373,7 +373,7 @@ std::vector<std::string> dns_lookup_impl_with_socket(const char *domain, int fam
temp = &packet[steps];
j = 0;
while (*temp != 0) {
if ((uchar)(*temp) == 0xc0) {
if ((uchar) (*temp) == 0xc0) {
++temp;
temp = &packet[(uint8_t) *temp];
} else {
Expand Down Expand Up @@ -767,36 +767,40 @@ int getaddrinfo(GetaddrinfoRequest *req) {
hints.ai_socktype = req->socktype;
hints.ai_protocol = req->protocol;

int ret = ::getaddrinfo(req->hostname, req->service, &hints, &result);
int ret = ::getaddrinfo(req->hostname.c_str(), req->service.c_str(), &hints, &result);
if (ret != 0) {
req->error = ret;
return SW_ERR;
}

void *buffer = req->result;
int i = 0;
for (ptr = result; ptr != nullptr; ptr = ptr->ai_next) {
for (ptr = result; ptr != nullptr; ptr = ptr->ai_next, i++) {
}
req->count = SW_MIN(i, SW_DNS_HOST_BUFFER_SIZE);
req->results.resize(req->count);

for (ptr = result, i = 0; ptr != nullptr; ptr = ptr->ai_next, i++) {
switch (ptr->ai_family) {
case AF_INET:
memcpy((char *) buffer + (i * sizeof(struct sockaddr_in)), ptr->ai_addr, sizeof(struct sockaddr_in));
memcpy(&req->results[i], ptr->ai_addr, sizeof(struct sockaddr_in));
break;
case AF_INET6:
memcpy((char *) buffer + (i * sizeof(struct sockaddr_in6)), ptr->ai_addr, sizeof(struct sockaddr_in6));
memcpy(&req->results[i], ptr->ai_addr, sizeof(struct sockaddr_in6));

Check warning on line 788 in src/network/dns.cc

View check run for this annotation

Codecov / codecov/patch

src/network/dns.cc#L788

Added line #L788 was not covered by tests
break;
default:
swoole_warning("unknown socket family[%d]", ptr->ai_family);
break;
}
i++;
if (i == SW_DNS_HOST_BUFFER_SIZE) {
break;
}
}
::freeaddrinfo(result);
req->error = 0;
req->count = i;

return SW_OK;
}
} // namespace network

void GetaddrinfoRequest::parse_result(std::vector<std::string> &retval) {
struct sockaddr_in *addr_v4;
Expand All @@ -805,18 +809,17 @@ void GetaddrinfoRequest::parse_result(std::vector<std::string> &retval) {
char tmp[INET6_ADDRSTRLEN];
const char *r;

for (int i = 0; i < count; i++) {
for (auto &addr : results) {
if (family == AF_INET) {
addr_v4 = (struct sockaddr_in *) ((char *) result + (i * sizeof(struct sockaddr_in)));
addr_v4 = (struct sockaddr_in *) &addr;
r = inet_ntop(AF_INET, (const void *) &addr_v4->sin_addr, tmp, sizeof(tmp));
} else {
addr_v6 = (struct sockaddr_in6 *) ((char *) result + (i * sizeof(struct sockaddr_in6)));
addr_v6 = (struct sockaddr_in6 *) &addr;

Check warning on line 817 in src/network/dns.cc

View check run for this annotation

Codecov / codecov/patch

src/network/dns.cc#L817

Added line #L817 was not covered by tests
r = inet_ntop(AF_INET6, (const void *) &addr_v6->sin6_addr, tmp, sizeof(tmp));
}
if (r) {
retval.push_back(tmp);
}
}
}
} // namespace network
} // namespace swoole
10 changes: 5 additions & 5 deletions src/os/base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,14 @@ namespace async {

void handler_gethostbyname(AsyncEvent *event) {
char addr[INET6_ADDRSTRLEN];
auto request = (GethostbynameRequest *) event->data;
int ret = network::gethostbyname(request->family, request->name, addr);
sw_memset_zero(request->addr, request->addr_len);
auto req = dynamic_cast<GethostbynameRequest *>(event->data.get());
int ret = network::gethostbyname(req->family, req->name.c_str(), addr);
sw_memset_zero(req->addr, req->addr_len);

if (ret < 0) {
event->error = SW_ERROR_DNSLOOKUP_RESOLVE_FAILED;
} else {
if (inet_ntop(request->family, addr, request->addr, request->addr_len) == nullptr) {
if (inet_ntop(req->family, addr, req->addr, req->addr_len) == nullptr) {
ret = -1;
event->error = SW_ERROR_BAD_IPV6_ADDRESS;
} else {
Expand All @@ -104,7 +104,7 @@ void handler_gethostbyname(AsyncEvent *event) {
}

void handler_getaddrinfo(AsyncEvent *event) {
network::GetaddrinfoRequest *req = (network::GetaddrinfoRequest *) event->data;
auto req = dynamic_cast<GetaddrinfoRequest *>(event->data.get());
event->retval = network::getaddrinfo(req);
event->error = req->error;
}
Expand Down
8 changes: 8 additions & 0 deletions tests/include/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -852,3 +852,11 @@ function build_ftp_url(string $path = ''): string
{
return 'ftp://' . FTP_USER . ':' . FTP_PASS . '@' . FTP_HOST . ':' . FTP_PORT . '/' . $path;
}

function mkdir_if_not_exists(string $string): void
{
if (!is_dir($string)) {
mkdir($string, 0777, true);
}
}

2 changes: 1 addition & 1 deletion tests/include/skipif.inc
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function swoole_color(string $content, int $color): string
function skip(string $reason, bool $is_skip = true, int $color = SWOOLE_COLOR_YELLOW)
{
if ($is_skip) {
exit('skip ' . swoole_color($reason, $color));
exit('skip ' . swoole_color($reason, $color) . "\n");
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/swoole_coroutine_system/getaddrinfo.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ skip_if_offline();
--FILE--
<?php
require __DIR__ . '/../include/bootstrap.php';
go(function () {
Co\run(function () {
$ip_list = Swoole\Coroutine\System::getaddrinfo('www.baidu.com', AF_INET);
Assert::assert(!empty($ip_list) and is_array($ip_list));
foreach ($ip_list as $ip) {
Expand Down
30 changes: 30 additions & 0 deletions tests/swoole_coroutine_system/getaddrinfo_timeout.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
--TEST--
swoole_coroutine_system: getaddrinfo timeout
--SKIPIF--
<?php require __DIR__ . '/../include/skipif.inc';
skip_if_not_root();
skip_if_in_ci();
?>
--FILE--
<?php
require __DIR__ . '/../include/bootstrap.php';
$tmpdir = '/tmp/' . uniqid();
mkdir_if_not_exists($tmpdir . '/etc');
chroot($tmpdir);
if (!is_file('/etc/resolv.conf')) {
file_put_contents('/etc/resolv.conf', "nameserver 192.168.8.8\noptions timeout:1 retry:1\n");
}
Co\run(static function () {
$res = Swoole\Coroutine\System::getaddrinfo(
domain: 'swoole-non-existent-domain',
protocol: 0,
service: '',
timeout: 0.5,
);
Assert::false($res);
Assert::eq(swoole_last_error(), SWOOLE_ERROR_CO_TIMEDOUT);
echo "DONE\n";
});
?>
--EXPECT--
DONE
2 changes: 1 addition & 1 deletion tests/swoole_http_server/bug_5186.phpt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
--TEST--
swoole_http_server: 在使用hyperf3的时候遇到Context::parse_multipart_data()的提示
swoole_http_server: GitHub issue #5186
--SKIPIF--
<?php require __DIR__ . '/../include/skipif.inc'; ?>
--FILE--
Expand Down

0 comments on commit 9105da7

Please sign in to comment.