Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.1]Fix bug #5547 #5586

Merged
merged 1 commit into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
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 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 @@
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 @@
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 @@
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 @@
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
Loading